Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2 SMS Validation #112

Open
8 tasks
k-macmillan opened this issue Dec 30, 2024 · 2 comments
Open
8 tasks

v2 SMS Validation #112

k-macmillan opened this issue Dec 30, 2024 · 2 comments
Labels
Dev Reviewed Reviewed by Tech Lead Notify Board trigger PM Reviewed Reviewed by Product Manager QA Reviewed Reviewed by Quality Assurance QA Issue requires QA collaboration

Comments

@k-macmillan
Copy link
Member

k-macmillan commented Dec 30, 2024

User Story - Business Need

Validate /legacy/v2/notifications/sms in the same manner notification-api validates them. Evaluating which follow-on tasks should be "chained" is not necessary.

  • Sync with Kyle when ticket is picked up.
  • Ticket is understood, and QA has been contacted (if the ticket has a QA label).

User Story(ies)

As a VA Notify client
I want any upgrades to be seamless for me
So that I don't have to change for any VA Notify changes

Additional Info and Resources

Starts with post_notification in the notification-api.

  • Validate (skip = not doing, defer = doing later)
    1. Template is of the right channel type: sms and is active
    2. Personalization matches template personalization (permissive, needs them if there, extra is ignored)
    3. defer: template is part of the service - will come when we have proper JWT auth and request state
    4. defer: rate limiting - will come later (only applies to dev/staging)
    5. defer: daily message limiting - will come later (only applies to dev/staging)
    6. skip: service has permission to send this channel of notification
    7. skip: template is active - no longer applicable
    8. skip: scheduled for - unused
    9. skip: sms sender default - all services have a default sms sender now

Acceptance Criteria

  • SOLID principles
  • route entrypoint calls a validate method
  • All non-skipped/deffered methods validations in the Additional Info have methods written
  • Other than identifying what to validate, no notification-api code is reused
  • All validations involving the database are done against the notification-api read database
  • This work is added to the sprint review slide deck (key win bullet point and demo slide)

QA Considerations

Endpoint log parity or identification of what will change so we can notify our clients (when we get far enough along to switch).

Potential Dependencies

Out of Scope

  • Evaluating chain tasks.
  • Sending to Celery/SQS.
@k-macmillan k-macmillan added Dev Reviewed Reviewed by Tech Lead Notify Board trigger QA Issue requires QA collaboration labels Dec 30, 2024
@cris-oddball
Copy link
Contributor

@k-macmillan potentially dumb question, but does ENP have permissions to read from the notification-api database? Or do we need a ticket for that (upon which this ticket is dependent)?

@cris-oddball
Copy link
Contributor

Nevermind, I'll add #114 to dependencies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dev Reviewed Reviewed by Tech Lead Notify Board trigger PM Reviewed Reviewed by Product Manager QA Reviewed Reviewed by Quality Assurance QA Issue requires QA collaboration
Projects
None yet
Development

No branches or pull requests

4 participants