Three improvements to the email-alert-api test helpers #1132
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.
Firstly, adding
content_id
to the subscriber lists URL helper.The way the test helpers handle subscriber lists is kind of weird. We pass all the parameters---both the request and response ones undifferentiated---into a single
has_subscriber_list
method, which then uses an allowlist to pluck out the parameters which should go in the request URL, and uses the others to construct the response. This is pretty strange, the only benefit I can see is that some parameters you'd otherwise have to specify twice, but the cost is that we have this list here which has to be kept up to date. I think it would be better to remove the need for this entirely, but that can be a later change.Secondly, adding
sender_message_id
andgovuk_request_id
to the bulk-unsubscribe bad-request helper.These were left off because the only time the endpoint raises a bad request response is if you don't specify these. But that makes it kind of difficult to actually use in the email-alert-service tests: I could blank out the
govuk_request_id
easily enough, but I can't blank out thesender_message_id
- so in practice this helper can't actually be used. So I've added those parameters in.Thirdly, adding both with- and without-message versions of the bulk-unsubscribe helpers.
This is useful because, in cases where we don't care about what the message actually is (like the above case when we're testing a 422 response is gracefully handled), we can use the without-message variants.