Support delivering submissions by multiple methods#2218
Merged
Conversation
All submission jobs that were queued with a `Submission` as the argument will have now been processed, so we can remove the backwards compatibility.
We're going to allow submissions to be delivered by multiple delivery methods. Whilst we don't currently need to consume the delivery_method it is useful to identify which method the delivery corresponds to now there can be multiple `immediate` deliveries. Also store the `formats` on the delivery. If we store this at the time we create the delivery, it means we don't need to look at the delivery_configurations on the form again to determine which formats to send. We can also log it for deliveries to help with debugging. Allow this to be null, as it won't be set for deliveries that were created before this change and so we'll need to fallback on using the form's submission_format stored on the Submission in these cases.
Tests for the FormSubmissionService when the form has different submission_format existed because previously the formats were configured by having different submission_types - `email_with_csv`, `email_with_json`, `s3_with_json`. We test sending submissions with different formats in the specs for the AwsSesSubmissionService and S3SubmissionService, so these extra tests were unnecessary and were not testing anything.
When we delete submissions, we send a metric to CloudWatch with the delivery status of the submission as a dimension so we can tell how many submissions we're deleting that haven't been successfully delivered. This was assuming there was only one delivery per submission, but there can now be multiple. Calculate the status from all the deliveries for this metric so we can report if any of the deliveries were pending or failed.
stephencdaly
force-pushed
the
use-delivery-configurations
branch
from
July 16, 2026 18:44
96f8918 to
92a8b17
Compare
The `Submission.sent?` method is used by the e2e tests to check if a submission has been delivered. For the e2e tests, we'll only ever have one delivery - but to make this method more correct, check if all deliveries have been sent. This will allow us to remove the `Submission#single_submission_delivery` method.
stephencdaly
force-pushed
the
use-delivery-configurations
branch
4 times, most recently
from
July 17, 2026 09:32
4a662f8 to
1d312f6
Compare
To prepare for being able to send submissions by multiple delivery methods, remove this method as a submission can now have multiple `immediate` deliveries. The method is only used in tests now, so update those tests to just get the first delivery.
Form documents now include a `delivery_configurations` field, which is an array of delivery configuration objects. We will use these to determine how to deliver form submissions to form processors, replacing the `submisssion_type`, `submission_formats`, `send_daily_submission_batch` and `send_weekly_submission_batch` attributes.
Iterate over all delivery_configurations for the form, and create a Delivery and enqueue the appropriate job to deliver it. Set the delivery_method and formats on the Delivery record so we can use these when delivering the submission rather than using the submission_type and submission_format which will no longer be accurate when there are multiple delivery_configurations for a form. Stop logging the submission type and formats on the "form_submission" log event. We already log the Job class name that is enqueued for deliveries and can log the formats when the job is performed.
If a delivery job fails to enqueue, and we have already enqueued a job for a different delivery method for the same form submission, we cannot delete the submission. Instead, mark the delivery that failed to enqueue as failed and capture an error message to Sentry for support to investigate. This will need to be resolved by running a rake task to retry the failed delivery.
Add logging attributes for the delivery method and formats so that all log lines for the jobs to send submissions to S3 and SES will include these fields.
For daily and weekly batch deliveries, the delivery_method can only ever be "email" and the format can only be "csv" for now. Hardcode these values rather than reading off the DeliveryConfiguration for the form. If we later support sending batch deliveries via other methods or formats, we can update this code to read from the DeliveryConfiguration.
We are now storing the formats on the Delivery model, as we support multiple delivery methods per form, which can each have different formats. The formats are populated from the delivery_configurations for the form. Use the formats from the Delivery model to decide which files to create for email and S3 submissions. Deliveries that were created before the change to use the delivery_configurations will have the `formats` set to nil. In this case, we need to fall back to using the `submission_format` from the Form stored against the submission.
Form#submission_type is no longer used, so we can remove the attribute from the Form model. We still need to use submission_format as a fallback for historic Submissions if we need to retry delivering them, so just remove any unnecessary usages and add a comment.
stephencdaly
force-pushed
the
use-delivery-configurations
branch
from
July 17, 2026 10:19
1d312f6 to
ef3233c
Compare
Contributor
|
🎉 A review copy of this PR has been deployed! It is made of up two components It may take 5 minutes or so for the application to be fully deployed and working. If it still isn't ready For the sign in details and more information, see the review apps wiki page. |
stephencdaly
marked this pull request as ready for review
July 17, 2026 10:56
thomasiles
approved these changes
Jul 17, 2026
thomasiles
left a comment
thomasiles
left a comment
Contributor
There was a problem hiding this comment.
I'm not super familiar with this part of the code so i might not be able to spot things missing.
This is comprehensive, well tested and covers the transition between the old and new 🏅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this pull request solve?
Trello card: https://trello.com/c/WMnjLlJV
Use the
delivery_configurationson the form to determine which delivery methods are enabled, and deliver submissions using each method.I've tested on dev that this is still working as expected for email and S3 submissions.
I've tested locally that submissions are delivered by both S3 and email when both are enabled (I haven't added a rake task to enable this on dev yet)
Things to consider when reviewing