Update check_and_queue_callback Logic for a callback url

[k-macmillan]

User Story - Business Need

As part of the work to enhance client experience and reduce opportunities for silent failures, we are enabling a callback_url as part of the notification request. If there is a callback_url in the Notification object it will supersede any service-level callback.

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

User Story(ies)

As a Service using VA Notify I want to be able to specify callbacks at the request-level So that we have more agency

Additional Info and Resources

The check_and_queue_callback_task method was only built with service callbacks in mind.

Acceptance Criteria

• [x] TDD
• [x] Existing service-level callback logic moved to a new method, untouched
• [x] New callback_url based logic is in a separate method
• [x] Logic in check_and_queue_callback_task calls one method or the other
• [x] New method collects data using the create_delivery_status_callback_data_v3 method
• [x] Signature is retrieved with generate_callback_signature method
• [x] 400, 401, and 403 responses need to be logged as error so we can surface those with our clients.
• [x] Update callback docs for request level callbacks
  • [x] make note that we currently do not offer SQS callback
• [x] This work is added to the sprint review slide deck (key win bullet point and demo slide)

QA Considerations

Edge cases such as invalid URLs or clients rejecting the request due to validation.

Potential Dependencies

[kbelikova-oddball]

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

[EvanParish]

I've started writing tests for this and since the blocking ticket is now merged, I am starting implementation work.

[EvanParish]

I made some good progress today, but I have a few questions I'll have to run by Kyle / team tomorrow before I can finish this work.

[EvanParish]

I did some testing yesterday and it's looking promising so far! There were a couple things I wanted to look into today before opening the PR for review, so I'm just wrapping that up.

[cris-oddball]

QA PASSED

• [x] Regression in Perf passes

• [x] Service callbacks still function as before if a notification is sent without a callback.

  

  
  
  

• [x] Notification callback w/ service callback results in a logged message, 'Callback successfully sent for notification', only the notification callback is issued, not the service callback, and includes the header 'x-enp-signature' "id": "83613921-56ad-4d11-947d-0470ff19d10f",

  

No logs seen for the service callback for this notification.

• [x] Notification callback w/o service callback results in a logged message, 'Callback successfully sent for notification' and includes the header 'x-enp-signature' "id": "49af13bb-44a9-4728-8b5a-16472d100d61",

  

  
  
  

• [x] Notification callback that returns 4xx that is not a 429 is not retryable w/ log 'sending callback for notification' and 'will not retry' (note there are a lot of 4xx errors, pick a few) Using pipedream service for these: 400 "id": "52dd4e52-e8ff-4e90-a8bb-c9c8a8e263f0" 401 "id": "916170eb-8582-4ac5-92df-e1eba8ec8d84", 404 "id": "8a83d276-7a00-4162-9f31-13da585c0376",

  

also the following for each

• [x] Notification callback that returns 429 is retryable w/ log 'Retryable error sending callback for notification' "id": "d4ac43be-fb78-4dfd-9f14-9699bcdec35e",

  

  '
  
  

• [x] Notification callback that returns 5xx is retryable w/ log 'Retryable error sending callback for notification' "id": "e8467a74-2784-430b-9f35-b70e79109440",