department-of-veterans-affairs / notification-api

Notification API
MIT License
16 stars 9 forks source link

V3 - Add Provider Logic for Email and SMS #1505

Closed mjones-oddball closed 1 month ago

mjones-oddball commented 12 months ago

User Story - Business Need

We want to ensure V3 has feature parity with V2. This ticket should ensure that the appropriate provider is chosen based on system configurations.

User Story(ies)

As a notification platform I want the ability to send notifications from multiple providers So that I can support different digital notification channels and VA business needs

Additional Info and Resources

SES and Pinpoint are the default providers on all services. However, some services like VEText send through multiple providers and, as a result, we use template configs to specify provider.

The provider is determined in the following order:

  1. template level
  2. service level
  3. priority provider logic (active, notification_type, priority #)

Engineering Checklist

Please note - it is critical we finish this work after the new year. Remaining V3 work should not be a code overhaul. Any refactoring requests must be approved by Product and Tech Lead.

Acceptance Criteria

Givens

Cases

Email - /v3/notifications/email

SES Confirm you receive an email in the following cases:

SMS - /v3/notifications/sms

Use a valid sender for the tests below. If you try to send sms using a Twilio sender but provider is Pinpoint (or vice versa) this will fail. That logic is not altered as part of the v3 work.

Twilio Confirm you receive a text in the following cases:

Pinpoint Confirm you receive a text in the following cases:

QA Considerations

Out of Scope

If something requires overhaul of V2 functionality to make it work for V3

kalbfled commented 9 months ago

@mjones-oddball @k-macmillan The "existing logic" checks 2 feature flags that I doubt are necessary anymore. Do we need to keep that? If not, I could probably replace the somewhat overly complicated "cient_to_use" function with a single database query.

mjones-oddball commented 9 months ago

@kalbfled what are the feature flags? I would need to know more about it before making a judgment call. CC @k-macmillan

kalbfled commented 9 months ago
  1. PROVIDER_STRATEGIES_ENABLED
  2. TEMPLATE_SERVICE_PROVIDERS_ENABLED
mjones-oddball commented 9 months ago

@k-macmillan interested to hear your thoughts on if we need to keep the feature flagged or not. I can't think of a reason off the top of my head.

k-macmillan commented 1 month ago

Will re-address with va-enp-api.