Legacy v2 SMS Pydantic Models

[k-macmillan]

User Story - Business Need

We need Pydantic models representing the notification-api /v2/notification/sms endpoint. This will help us prepare to wrap notification-api with va-enp-api.

• Ticket is understood, and QA has been contacted (if the ticket has a QA label).

User Story(ies)

As a VA Notify dev
I want Pydantic models for all routes
So that our API is fast and consistent

Additional Info and Resources

• QA repo has some/most of this work done
• post_sms_request is in notification-api app/v2/notifications/notification_schemas.py

Acceptance Criteria

• Models follow the style guide
• Request model
• Response model
• Same POST endpoint fields as notification-api for /v2/notifications/sms
• Same response fields on success
  Note: Leaving this AC unchecked due to inability to properly test response fields due to hardcoded response. Testing dependent on DB for proper responses.
• This work is added to the sprint review slide deck (key win bullet point and demo slide)

QA Considerations

QA Validation is deferred to another ticket to be created.

Potential Dependencies

Dev Testing

Unit tests added to cover valid and invalid data

Tested locally with the following POST payloads and verifying a response of 201 Created
Response data is currently hardcoded and not included in testing
Basic validation of required optional fields but proper validation and error responses to be addressed in later ticket.

/v2/notifications/sms
/legacy/v2/notifications/sms

Required fields (template_id and phone_number)

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "phone_number": "+18005550101" }

Required fields (template_id and recipient_identifier)

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "recipient_identifier": { "id_type": "ICN", "id_value": "some-id-value" } }

Required fields (template_id, phone_number, and recipient_identifier)

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "phone_number": "+18005550101", "recipient_identifier": { "id_type": "ICN", "id_value": "some-id-value" } }

Optional personalisation fields (includes email specific options since the model is shared)

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "phone_number": "+18005550101", "personalisation": { "additionalProp1": "string", "additionalProp2": "string", "ListProp1": [ "Item1", 1, 1.0 ] } }

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "phone_number": "+18005550101", "personalisation": { "additionalProp1": "string", "additionalProp2": "string", "file_obj": { "file": "U29tZSByYW5kb20gZmlsZSBkYXRh", "filename": "string", "sending_method": "attach" } } }

Optional fields

{ "billing_code": "12345", "callback_url": "https://example.com/", "personalisation": { "additionalProp1": "string", "additionalProp2": 1, "additionalProp3": 1.0 }, "reference": "an-external-id", "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "phone_number": "+18005550101", "sms_sender_id": "4f44ffc8-1ff8-4832-b1af-0b615691b6ea", "scheduled_for": "2024-03-15T10:00:00Z", "email_reply_to_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1" }

scheduled_for validation is out of scope

Validation failures (400 Error: Bad Request)

1. template_id should be UUID

{ "template_id": "foo", "phone_number": "+18005550101" }

2. phone_number shold be E164 phone_number (minimal validation)

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "phone_number": "+1800550101" }

3. recipient_identifier missing required id_type

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "recipient_identifier": { "id_value": "some-id-value" } }

4. recipient_identifier missing required id_value

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "recipient_identifier": { "id_type": "ICN" } }

5. recipient_identifier invalid id_type

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "recipient_identifier": { "id_type": "FOO", "id_value": "some-id-value" } }

6. http callback url

{ "callback_url": "http://example.com/", "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "phone_number": "+18005550101" }

7. missing required file object fields

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "phone_number": "+18005550101", "personalisation": { "file_obj": { "filename": "string", "sending_method": "attach" } } }

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "phone_number": "+18005550101", "personalisation": { "file_obj": { "file": "U29tZSByYW5kb20gZmlsZSBkYXRh", "sending_method": "attach" } } }

8. invalid attachment type

{ "template_id": "a71400e3-b2f8-4bd1-91c0-27f9ca7106a1", "phone_number": "+18005550101", "personalisation": { "file_obj": { "file": "U29tZSByYW5kb20gZmlsZSBkYXRh", "filename": "string", "sending_method": "foo" } } }

Out of Scope

Fixing inconsistencies in the notification-api on the POST request. Any inconsistencies will be brought to va-enp-api because this is a "legacy" route. notification-api requests can fail in many ways. We are not going to replicate the responses in this ticket.

Any before/after request validation regarding phone number or recipient (follow-on ticket)

QA Automation will be handled in a follow-on ticket

[ChrisJohnson-CDJ]

Had an initial conversation with QA and followed up with questions.
Reviewing Pydantic docs, /v2/notifications/sms swagger docs, and app/v2/notifications/notification_schemas.py

[ChrisJohnson-CDJ]

Ran ENP unit tests on temporary models to verify.
Worked through comparisons of the existing notification-api v2 schema compared to temporary ENP models.
Clarified ambiguous descriptions for phone_number and recipient_identifier, anyOf vs oneOf, defering to code definition vs. description to maintain 1:1 equivalency.

[ChrisJohnson-CDJ]

https://docs.pydantic.dev/latest/migration/#required-optional-and-nullable-fields
https://docs.pydantic.dev/latest/concepts/fields/#the-annotated-pattern

For the anyOf constraint on recipient_id and phone_number, Pydantic doesn't have a direct/clean equivalent.
Best work-around looks like a combination of allowing/default of None and using a model_validator.
Existing temporary model already implements this.

Adding field_validator to callback_url to restrict to https (per notification-api v2 schema)

[ChrisJohnson-CDJ]

mypy seems to have problems with (among other things) the use of aliasing i.e. personalisation aliased with personalization

mypy check failed
tests/app/legacy/v2/notifications/test_rest.py:188: error: Missing named argument "personalization" for "V2PostSmsRequestModel" [call-arg]
Found 1 error in 1 file (checked 56 source files)

pydantic/pydantic#6713 (comment)

Update: The solution is that although the first parameter of Field is 'default', you have to specify it by name vs. position for mypy to see it.

personalisation: dict[str, str | int | float] | None = Field(default=None, alias='personalization')

[ChrisJohnson-CDJ]

Checking the unittest coverage and met with Evan to discuss local Postman testing, including generation of JWT token

[ChrisJohnson-CDJ]

Discussed current changes and possible testing methodology with QA

Potential issues to be discussed with team and/or addressed on Monday

1. personalization alias on ENP presents a problem if clients utilize it and then get flipped back to notify-api. QA recommends dropping alias
2. personalisation structure is different for sms vs. email so they shouldn't share a base model. email personalisation can contain lists and objects for attachments (file, calendar).
3. validation error messages need to match existing notify-api (out of scope)
4. proposing that the hard coded responses in ENP be temporarily replaced with data for testing. This would be clearly commented in the code with an additional todo tag for visibility. (discuss with team, likely out of scope)

[ChrisJohnson-CDJ]

Discussed testing with Cris and given that validation is to be addressed in #112, testing for now can just return a 400 if validation fails.

[cris-oddball]

NOTE: QA Validation is deferred to another ticket to be created.

[ChrisJohnson-CDJ]

Made changes based on review suggestions. Simplified UUID and HTTPS validation.