Add returned letter callback celery tasks #4369
Conversation
app/celery/tasks.py
Outdated
| # queue callback task only if the service_callback_api exists | ||
| if service_callback_api := get_service_returned_letter_callback_api_for_service(service_id=data.service_id): | ||
| reported_date = get_returned_letter_reported_at_date_by_notification_id_dao(data.id) | ||
| returned_letter_data = create_returned_letter_callback_data(reported_date, service_callback_api) |
There was a problem hiding this comment.
I haven't tried running any of this code, but spotted that create_returned_letter_callback_data has three required arguments and yet is being called with only two here
There was a problem hiding this comment.
Good catch. I will update the function call.
There was a problem hiding this comment.
We should also make sure this would get caught by the tests
app/celery/service_callback_tasks.py
Outdated
| returned_letter_data = fetch_returned_letter_data(service_id, reported_date) | ||
|
|
||
| data = { | ||
| "notification_id": str(returned_letter_data[0]["notification_id"]), |
There was a problem hiding this comment.
I might have got this wrong, but from looking at the code it appears that this create_returned_letter_callback_data function is called once per returned letter, gets all the returned letters for the relevant service and yet only ever returns the first one. So if a service had 5 returned letters, its callback endpoint would be hit 5 times but every time they would receive details of the first letter.
I have tried to test this locally to see if this is the case, but am getting SQLAlchemy errors from this function which I haven't had a chance to dig into. This needs some thorough local testing to check everything is working as expected.
There was a problem hiding this comment.
@klssmith Yes, you are right, only details for the first letter is being retrieved. I will address that and also update the test cases as the two bugs you noticed should have been picked up by unit tests.
|
I am having a rethink of my approach in this PR and will be overhauling most of it. |
This helper function initiates returned letters callback for services that have the feature enabled.
This method retrieves the data for the returned letter callback. It mirrors the information provided in the returned letter report.
BlessedDev
force-pushed
the
add_returned_letter_callback_celery_tasks
branch
from
25c0131 to
15d3530
Compare
This PR adds the celery tasks needed for the returned letter callback feature as described in this card.