department-of-veterans-affairs / notification-api

Notification API
MIT License
16 stars 9 forks source link

Add Callback URL on email/sms POST Requests #2014

Closed k-macmillan closed 1 month ago

k-macmillan commented 2 months ago

User Story - Business Need

We want to ensure the new callback_url ends up in the database for that notification.

User Story(ies)

As a VA Notify developer I want callback_url to be available everywhere the Notification object is available So that we can use it when appropriate

Additional Info and Resources

Acceptance Criteria

QA Considerations

Potential Dependencies

npmartin-oddball commented 2 months ago

Hey team! Please add your planning poker estimate with Zenhub @coreycarvalho @cris-oddball @EvanParish @k-macmillan @kalbfled @MackHalliday @mchlwellman

MackHalliday commented 1 month ago

NOTES from QA call with @k-macmillan

MackHalliday commented 1 month ago

Today

Tomorrow

MackHalliday commented 1 month ago

Unit Testing

DEV Testing

MackHalliday commented 1 month ago

Note - check edge case for length of URL. Should throw ValidationError instead of DataError

MackHalliday commented 1 month ago

Discovered that updating the schema for the https_url to have a max length 255 has an unintended side affect of limiting the max length of the url on the ServiceCallback table.

Should the url be constrained on the ServiceCallback table be constrained?

kalbfled commented 1 month ago

@k-macmillan Mack and I discussed this. The schema https_url is used to create and update rows in the ServiceCallback table. It shouldn't be part of this ticket.

cris-oddball commented 1 month ago

PR approved and merged, along with the QA PR that validates this work.

Sending up to Perf, new regression will vet this, then can close ticket.

cris-oddball commented 1 month ago

Passed valdiation