API can accept VA identifiers

[QaysarA]

Value Statement:

As a consumer of VANotify
I want to send a notification using a recipient's VA identifiers
So that I don't have to find the recipient's contact details myself

Example

POST /v2/notifications/email
{
  "template_id": "foo",
  "va_identifier": {
    "id_type": "VA_PROFILE_ID", # changed from `type` to `id_type` because `type` is a json schema keyword
    "value": "bar"
  }
  # note the conspicuous lack, the eerie absence, the tantalising nonexistence of the key "email_address"
  # and the unexpected appearance of a new friendly object: "va_identifier"
}

Acceptance Criteria

• 1. Valid sms requests
  GIVEN a service and template have been configured on VANotify
  WHEN I make a request to send an sms notification
  AND I don't specify phone_number
  AND I do specify va_identifier
  THEN the API does not reject the request as invalid

• 2. Valid email requests
  GIVEN a service and template have been configured on VANotify
  WHEN I make a request to send an email notification
  AND I don't specify email_address
  AND I do specify va_identifier
  THEN the API does not reject the request as invalid

• 3. Invalid requests
  GIVEN a service and template have been configured on VANotify
  WHEN I make a request to send a notification
  AND I don't specify contact information
  AND I don't specify va_identifier
  OR I specify va_identifier, but it does not contain both type and value
  THEN the API rejects the request as invalid

• 4. Specific identifier_types
  GIVEN a service and template have been configured on VANotify
  WHEN I make a request to send a notification
  AND I specify va_identifier
  AND va_identifier.type is not one of: PID, ICN, VA_PROFILE_ID
  THEN the API rejects the request as invalid

• 5. Persisting identifiers
  GIVEN a service and template have been configured on VANotify
  WHEN I make a request to send a notification
  AND I specify va_identifier
  THEN the notification_id, va_identifier_type, and va_identifier_value should be persisted in a new table, called notification_identifiers recipient_identifiers

• 6. Don't send it just yet
  GIVEN a service and template have been configured on VANotify
  WHEN I make a request to send a notification
  AND I don't specify contact information
  AND I specify va_identifier
  THEN the notification is not sent - notification delivery will come in a later story

• 7. VA_PROFILE_ID
  GIVEN a service and template have been configured on VANotify
  WHEN I make a request to send a notification
  AND I don't specify contact information
  AND I specify va_identifier
  AND va_identifier.type is VA_PROFILE_ID
  THEN a Celery task lookup_contact_info should be enqueued
  AND that task, upon execution, should log some information about the notification, and a message indicating that we should lookup contact information

• 8. Other VA identifiers
  GIVEN a service and template have been configured on VANotify
  WHEN I make a request to send a notification
  AND I don't specify contact information
  AND I specify va_identifier
  AND va_identifier.type is not VA_PROFILE_ID
  THEN a Celery task lookup_va_profile_id should be enqueued
  AND that task, upon execution, should log some information about the notification, and a message indicating that we should lookup VA Profile ID

• 9. Contact info AND identifiers
  GIVEN a service and template have been configured on VANotify
  WHEN I make a request to send a notification
  AND I specify contact information
  AND I specify va_identifier
  THEN the notification is sent as normal
  AND the identifiers are persisted
  AND no tasks to lookup contact information are enqueued (because we've already got the contact information! hooray)

Checklist:

• In notification_schemas.py, change API to accept va_identifier, as described above
• In post_notifications.py, persist those new parameters if provided
  • Migration to create new table recipient_identifiers
• Add schema for RecipientIdentifiersHistory, which should be linked to NotificationsHistory
  • Migration to create new table recipient_identifiers_history
  • When notification is moved to notifications_history, the matching recipient identifiers should move to history table -> Because recipient information is not stored in NotificationHistory, we will not carry over RecipientIdentifiers information into a history table.
• Migration + model change to make the to column in Notification nullable. Since that's where we store the recipient, and we won't have contact information yet
• Add new Celery tasks, which just log a nice friendly message for now
• In post_notifications.py, enqueue the appropriate new Celery task if the new parameters have been provided
• Documentation Updates:
  • Update OpenAPI documentation
  • Update lookup contact info sequence diagram
  • Add to postman collection
  • Add a test to the user flows - somewhat dependent on Run User Flows in VAEC #228, but we can run them manually until then
• Other:
  • Scheduled Notifcations?
  • Simulated Notifications?

Assumptions:

Additional Info/Resources:

• See this ADR for context and design decisions
• See this sequence diagram for a draft of the process

Out of Scope:

• The tasks lookup_vet360_id and lookup_contact_info don't need to anything for now - they should just log, so we know they're running
• Validating the basic format of each identifier will be done in a separate card Add recipient identifiers validation to the API #191

Open Questions:

• What should be the name for the new table, which stores VA identifiers? - notification_identifiers, why not
• What values of identifier_type should the API accept? - seemingly: PID, ICN, VA_PROFILE_ID
• List of documentation that needs to be updated - just openapi and postman collection
• Confirm reject if more than one identifier is sent - not possible given the API design
• Confirm if email and identifiers provided, then the email will take precedence. Both values must be stored in the table - added AC9 - if a consumer gives us contact info AND identifiers, we just send the notification, but persist everything they sent us
• Include updating User Flows (need to validate and then delete OOS) - hell yes
• we have special behaviour for VA_PROFILE_ID - if the consumer provides that, we can go straight to VA Profile, without going to MPI. is this the case for any other identifiers?

[marisahoenig]

If we want to move models into a directory and have each be it's own file, these are some instructions. Basically, we would need to change where we call db.init_app() (in __ init __.py) to instead be called from application.py.

To note: if you try creating a directory "models," the app thinks you're trying to reference the models.py, so it has to be a different name. I would rather separate this into its own card to change over several of the models to a directory since I'm not sure if there will be other issues to fix once those changes are made.

[marisahoenig]

merged to master! 🤞

[marisahoenig]

things to finish off before Review:

• Add 501 response to openapi documentation
• Test in VAEC dev; make sure logs correct message for queue

when in QA:

• test in staging with feature toggle off
• turn on feature flag in staging and test

[marisahoenig]

While mobbing, we tested in VAEC dev and staging. With feature toggle on in dev and feature toggle off in staging, we saw the expected behavior and logs. We still need to test in VAEC staging now that we turned the toggle on, but I'm moving this to review.

[jsmithVA]

Two potential issues discovered during testing:

• Error message when attempting to send a notification with no contact info and no identifier is not descriptive/informative. @marisahoenig to determine how difficult it would be to update the message. It will either be updated under this card or a new card depending on effort.

• Celery logs for the triggered tasks show no request id. Is this something we need?

[marisahoenig]

@jsmithVA For the celery logs, these were just filler messages to be changed later. I think including the request-id, if wanted, can be included in the card that actually queries VA Profile/MPI.

[jsmithVA]

Link to test plan document:here

[jsmithVA]

Error message issue will be handled under issue 258