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-1862] Add unsubscribe functionality #3445

Merged
merged 10 commits into from
Jan 17, 2025
Merged

[CAPT-1862] Add unsubscribe functionality #3445

merged 10 commits into from
Jan 17, 2025

Conversation

asmega
Copy link
Contributor

@asmega asmega commented Dec 2, 2024

Context

Changes

  • Send unsubscribe_url as part of reminder emails in the personalisation section so they can be appended to emails sent to users
  • Should a user click on this link they are sent to the send to confirm if they wish to unsubscribe
  • As recommended by notify https://docs.notifications.service.gov.uk/ruby.html#unsubscribe-link-at-the-bottom-of-the-email-recommended
  • Upon success they get show the confirmation panel that transaction is complete
  • Should user visit invalid link or if they are already unsubscribed they are show a page with copy explaining the situation
  • config.action_controller.action_on_unpermitted_parameters has been changed back to the rails default setting which happens to also be the setting used in production. not sure why this was ever changed in the first place as it makes development and test behave differently to how production would behave

Notes

  • One click unsubscribe is not present in this change, however the server side mechanism has been added
  • This is because the library we are using at the moment does not support this notify feature see Support unsubscribe links dxw/mail-notify#166
  • We either need to switch to official library or add the needed functionality to support this feature

Screenshots

After user click unsubscribe link in email

Screenshot 2025-01-09 at 14 18 23


Upon successfully unsubscribing

complete


Wrong link or user already unsubscribed

404

Comment on lines +68 to +72
resources :unsubscribe,
path: "/unsubscribe/:resource_class",
constraints: {resource_class: /reminders/},
only: [:create, :show]

Copy link
Contributor Author

@asmega asmega Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resource_class is not currently used. but has been added so can be extended to support unsubscribing of other resource in the future easily if needed

<div class="govuk-grid-column-two-thirds">
<%= form_with model: @form,
url: unsubscribe_index_path,
scope: "",
Copy link
Contributor Author

@asmega asmega Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • empty scope is used here so form does not use nested parameters
  • this means the structure of the data supports a simpler format allowing the same controller endpoint to support one click unsubscriptions

slorek
slorek previously requested changes Dec 6, 2024
Copy link
Contributor

@slorek slorek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implmenting soft delete - could I ask that you instead re-use the Deletable concern to avoid duplication

@asmega asmega force-pushed the unsubscribe branch 2 times, most recently from 0b141b2 to daf4409 Compare January 9, 2025 14:20
@asmega asmega added the deploy Deploy a review app for this PR label Jan 15, 2025
Copy link
Contributor

@rjlynch rjlynch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks really good. Though I do like the missing parameters raising in dev / test pretty useful for catching issues, still Rails defaults are usually best to stick to

@asmega asmega dismissed slorek’s stale review January 17, 2025 10:31

Changes implemented

@asmega asmega merged commit 5bcdb52 into master Jan 17, 2025
15 checks passed
@asmega asmega deleted the unsubscribe branch January 17, 2025 10:32
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