Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CAPT-1531 Notify emails #3034

Merged
merged 5 commits into from
Aug 1, 2024
Merged

CAPT-1531 Notify emails #3034

merged 5 commits into from
Aug 1, 2024

Conversation

rjlynch
Copy link
Contributor

@rjlynch rjlynch commented Jul 25, 2024

Different policies have different rejection reasons. While the rejection
reasons for ECP, LUP, and TSLR are similar (ECP has an extra reason),
future policies (specifically IRP) have different rejection reasons.

This PR moves the available rejection reasons into a constant in the
policy, so new policies can just add their reasons there and update the
locales file.

Notes for review
This PR is split into a couple of commits, so probably best reviewed separately. First commit moves the rejection reasons into the policy, second one adds the new rejection reasons for IRP. The store accessor makes this feel a little awkward so I'm open to any different approaches for doing this (could just keep all the rejection reasons in the Decision class)

decision
email

@rjlynch rjlynch added the deploy Deploy a review app for this PR label Jul 25, 2024
@rjlynch rjlynch force-pushed the CAPT-1531/irp-notify-emails branch from 21e781d to 9762ba4 Compare July 25, 2024 15:36
@rjlynch rjlynch force-pushed the CAPT-1531/irp-notify-emails branch from 9762ba4 to 7188a34 Compare July 25, 2024 15:43
@rjlynch rjlynch force-pushed the CAPT-1531/irp-notify-emails branch from 211e7b6 to b43cdce Compare July 26, 2024 11:04
@rjlynch rjlynch changed the title Move rejection reasons into the policy CAPT-1531 / Move rejection reasons into the policy Jul 29, 2024
@rjlynch rjlynch changed the title CAPT-1531 / Move rejection reasons into the policy CAPT-1531 Move rejection reasons into the policy Jul 29, 2024
@rjlynch rjlynch changed the title CAPT-1531 Move rejection reasons into the policy CAPT-1531 Notify emails Jul 29, 2024
@rjlynch rjlynch marked this pull request as ready for review July 30, 2024 15:12
rjlynch added 5 commits August 1, 2024 11:37
Different policies have different rejection reasons. While the rejection
reasons for ECP, LUP, and TSLR are similar (ECP has an extra reason),
future policies (specifically IRP) have different rejection reasons.

This commit moves the available rejection reasons into a constant in the
policy, so new policies can just add their reasons there and update the
locales file.
Adds the rejection reasons given in the ticket. I've also updated the
notify email rejection template to work with these reasons.
Want to make sure the claim mailer specs cover irp emails.
IRP emails currenlty don't have a notify reply to id so we've added a
safe navigation operator to the "reply_to_id" field.
Adds two more rejection options to IRP
When running the personal data scrubber spec we create a `Decision`
instance. A `Decision` requires a rejection reason to be valid. The
rejection reason is taken from the `Decision`'s `Claim`'s policy'. To
avoid needing to remove / disable the test, and risk not reintroducing
it, we've added a temporary `placeholder` rejection reason (the other
option was stubbing the `ADMIN_DECISION_REJECTED_REASONS` constant in
the spec). When we get round to adding the rejection reasons for FE we
can remove this place holder and the tests will work with whatever is
the first value in the `ADMIN_DECISION_REJECTED_REASONS` constant.
@rjlynch rjlynch force-pushed the CAPT-1531/irp-notify-emails branch from abd4f3e to f7d9cf2 Compare August 1, 2024 11:22
@rjlynch rjlynch merged commit 23ebcd5 into master Aug 1, 2024
12 checks passed
@rjlynch rjlynch deleted the CAPT-1531/irp-notify-emails branch August 1, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy Deploy a review app for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants