From 13fdbcaae18f0a5b6d80997859e04b389ffaa8b2 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 29 Nov 2024 15:01:59 +0000 Subject: [PATCH 01/10] add unsubscribe functionality --- app/controllers/unsubscribe_controller.rb | 37 +++++++++++++++ app/forms/unsubscribe/confirmation_form.rb | 29 ++++++++++++ app/mailers/reminder_mailer.rb | 41 +++++++++++------ app/views/unsubscribe/create.html.erb | 11 +++++ app/views/unsubscribe/show.html.erb | 27 +++++++++++ .../subscription_not_found.html.erb | 15 ++++++ config/environments/development.rb | 3 +- config/environments/test.rb | 3 +- config/routes.rb | 5 ++ spec/features/unsubscribe_spec.rb | 26 +++++++++++ spec/forms/current_school_form_spec.rb | 8 ---- spec/forms/employed_directly_form_spec.rb | 8 ---- spec/forms/form_spec.rb | 16 ------- spec/forms/gender_form_spec.rb | 8 ---- .../entire_term_contract_form_spec.rb | 8 ---- .../induction_completed_form_spec.rb | 8 ---- .../itt_academic_year_form_spec.rb | 8 ---- .../claim_school_form_spec.rb | 8 ---- spec/forms/personal_details_form_spec.rb | 10 ---- spec/forms/poor_performance_form_spec.rb | 8 ---- spec/forms/provide_mobile_number_form_spec.rb | 10 ---- spec/forms/supply_teacher_form_spec.rb | 8 ---- .../unsubscribe/confirmation_form_spec.rb | 46 +++++++++++++++++++ spec/mailers/reminder_mailer_spec.rb | 15 ++++++ spec/requests/admin/admin_amendments_spec.rb | 10 ++-- spec/requests/unsubscribe_spec.rb | 25 ++++++++++ 26 files changed, 268 insertions(+), 133 deletions(-) create mode 100644 app/controllers/unsubscribe_controller.rb create mode 100644 app/forms/unsubscribe/confirmation_form.rb create mode 100644 app/views/unsubscribe/create.html.erb create mode 100644 app/views/unsubscribe/show.html.erb create mode 100644 app/views/unsubscribe/subscription_not_found.html.erb create mode 100644 spec/features/unsubscribe_spec.rb create mode 100644 spec/forms/unsubscribe/confirmation_form_spec.rb create mode 100644 spec/mailers/reminder_mailer_spec.rb create mode 100644 spec/requests/unsubscribe_spec.rb diff --git a/app/controllers/unsubscribe_controller.rb b/app/controllers/unsubscribe_controller.rb new file mode 100644 index 0000000000..b0f7ae2efc --- /dev/null +++ b/app/controllers/unsubscribe_controller.rb @@ -0,0 +1,37 @@ +class UnsubscribeController < ApplicationController + skip_forgery_protection + + def show + @form = Unsubscribe::ConfirmationForm.new(form_params) + + if @form.reminder.nil? + render "subscription_not_found" + end + end + + def create + @form = Unsubscribe::ConfirmationForm.new(form_params) + + if @form.reminder.nil? + head :bad_request + else + @form.reminder.destroy + end + end + + private + + def form_params + params.permit([:id]) + end + + def current_journey_routing_name + params[:journey] + end + helper_method :current_journey_routing_name + + def current_journey + Journeys.for_routing_name(params[:journey]) + end + helper_method :current_journey +end diff --git a/app/forms/unsubscribe/confirmation_form.rb b/app/forms/unsubscribe/confirmation_form.rb new file mode 100644 index 0000000000..726180ac3c --- /dev/null +++ b/app/forms/unsubscribe/confirmation_form.rb @@ -0,0 +1,29 @@ +module Unsubscribe + class ConfirmationForm + include ActiveModel::Model + include ActiveModel::Attributes + + attribute :id, :string + + def reminder + @reminder ||= Reminder.find_by(id:) + end + + def obfuscasted_email + head, tail = reminder.email_address.split("@") + + mask = case head.size + when 1, 2, 3 + "*" * head.size + else + [head[0], "*" * (head.length - 2), head[-1]].join + end + + [mask, "@", tail].join + end + + def journey_name + I18n.t("journey_name", scope: reminder.journey::I18N_NAMESPACE).downcase + end + end +end diff --git a/app/mailers/reminder_mailer.rb b/app/mailers/reminder_mailer.rb index e48a712a07..54313f5cc6 100644 --- a/app/mailers/reminder_mailer.rb +++ b/app/mailers/reminder_mailer.rb @@ -19,7 +19,13 @@ def email_verification(reminder, one_time_password, journey_name) journey_name: } - send_mail(OTP_EMAIL_NOTIFY_TEMPLATE_ID, personalisation) + template_mail( + OTP_EMAIL_NOTIFY_TEMPLATE_ID, + to: @reminder.email_address, + reply_to_id: GENERIC_NOTIFY_REPLY_TO_ID, + subject: @subject, + personalisation: + ) end def reminder_set(reminder) @@ -29,10 +35,17 @@ def reminder_set(reminder) personalisation = { first_name: extract_first_name(@reminder.full_name), support_email_address: support_email_address, - next_application_window: @reminder.send_year + next_application_window: @reminder.send_year, + unsubscribe_url: unsubscribe_url(reminder:) } - send_mail(REMINDER_SET_NOTIFY_TEMPLATE_ID, personalisation) + template_mail( + REMINDER_SET_NOTIFY_TEMPLATE_ID, + to: @reminder.email_address, + reply_to_id: GENERIC_NOTIFY_REPLY_TO_ID, + subject: @subject, + personalisation: + ) end # TODO: This template only accommodates LUP/ECP claims currently. Needs to @@ -47,22 +60,22 @@ def reminder(reminder) support_email_address: support_email_address } - send_mail(REMINDER_APPLICATION_WINDOW_OPEN_NOTIFY_TEMPLATE_ID, personalisation) - end - - private - - def extract_first_name(fullname) - (fullname || "").split(" ").first - end - - def send_mail(template_id, personalisation) template_mail( - template_id, + REMINDER_APPLICATION_WINDOW_OPEN_NOTIFY_TEMPLATE_ID, to: @reminder.email_address, reply_to_id: GENERIC_NOTIFY_REPLY_TO_ID, subject: @subject, personalisation: ) end + + private + + def unsubscribe_url(reminder:) + "https://#{ENV["CANONICAL_HOSTNAME"]}/#{reminder.journey::ROUTING_NAME}/unsubscribe/reminders/#{reminder.id}" + end + + def extract_first_name(fullname) + (fullname || "").split(" ").first + end end diff --git a/app/views/unsubscribe/create.html.erb b/app/views/unsubscribe/create.html.erb new file mode 100644 index 0000000000..4a138d01e6 --- /dev/null +++ b/app/views/unsubscribe/create.html.erb @@ -0,0 +1,11 @@ +<%= content_for(:page_title) { "Unsubscribe" } %> + +
+
+ <%= govuk_panel(title_text: "Unsubscribe complete") %> + +

+ You will no longer receive your <%= @form.journey_name %> reminder to <%= @form.obfuscasted_email %>. +

+
+
diff --git a/app/views/unsubscribe/show.html.erb b/app/views/unsubscribe/show.html.erb new file mode 100644 index 0000000000..4011922864 --- /dev/null +++ b/app/views/unsubscribe/show.html.erb @@ -0,0 +1,27 @@ +<%= content_for(:page_title) { "Unsubscribe" } %> + +
+
+ <%= form_with model: @form, + url: unsubscribe_index_path, + scope: "", + builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> + + <%= f.hidden_field :id %> + +

+ Are you sure you wish to unsubscribe from your <%= @form.journey_name %> reminder? +

+ +

+ You will no longer receive your reminder to <%= @form.obfuscasted_email %>. +

+ +

+ However, you will still continue to receive updates about your claim as it progresses. +

+ + <%= f.govuk_submit "Unsubscribe" %> + <% end %> +

+
diff --git a/app/views/unsubscribe/subscription_not_found.html.erb b/app/views/unsubscribe/subscription_not_found.html.erb new file mode 100644 index 0000000000..dc26268a90 --- /dev/null +++ b/app/views/unsubscribe/subscription_not_found.html.erb @@ -0,0 +1,15 @@ +
+
+

+ We can’t find your subscription +

+ +

+ You may have already unsubscribed. If you haven’t, check your email and copy the link again. +

+ +

+ If you need more help, contact support at <%= govuk_mail_to I18n.t("support_email_address", scope: [current_journey::I18N_NAMESPACE]), I18n.t("support_email_address", scope: [current_journey::I18N_NAMESPACE]) %> +

+
+
diff --git a/config/environments/development.rb b/config/environments/development.rb index d7b3cb6f23..c656970451 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -60,8 +60,7 @@ # Highlight code that triggered database queries in logs. config.active_record.verbose_query_logs = true - # Raise an exception if parameters that are not explicitly permitted are found - config.action_controller.action_on_unpermitted_parameters = :raise + config.action_controller.action_on_unpermitted_parameters = :log # Debug mode disables concatenation and preprocessing of assets. # This option may cause significant delays in view rendering with a large diff --git a/config/environments/test.rb b/config/environments/test.rb index a92b501287..1601e36b07 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -33,8 +33,7 @@ # Disable request forgery protection in test environment. config.action_controller.allow_forgery_protection = false - # Raise an exception if parameters that are not explicitly permitted are found - config.action_controller.action_on_unpermitted_parameters = :raise + config.action_controller.action_on_unpermitted_parameters = :log config.action_mailer.perform_caching = false diff --git a/config/routes.rb b/config/routes.rb index 6ed86fa3fa..0c055bb821 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -65,6 +65,11 @@ def matches?(request) end end + resources :unsubscribe, + path: "/unsubscribe/:resource_class", + constraints: {resource_class: /reminders/}, + only: [:create, :show] + scope constraints: {journey: /further-education-payments|additional-payments/} do resources :reminders, only: [:show, :update], diff --git a/spec/features/unsubscribe_spec.rb b/spec/features/unsubscribe_spec.rb new file mode 100644 index 0000000000..7b7e742af6 --- /dev/null +++ b/spec/features/unsubscribe_spec.rb @@ -0,0 +1,26 @@ +require "rails_helper" + +RSpec.feature "unsubscribe from reminders" do + let(:reminder) do + create( + :reminder, + journey_class: Journeys::FurtherEducationPayments.to_s + ) + end + + scenario "happy path" do + visit "/#{reminder.journey::ROUTING_NAME}/unsubscribe/reminders/#{reminder.id}" + expect(page).to have_content "Are you sure you wish to unsub" + + expect { + click_button("Unsubscribe") + }.to change(Reminder, :count).by(-1) + + expect(page).to have_content "Unsubscribe complete" + end + + scenario "when reminder does not exist" do + visit "/#{reminder.journey::ROUTING_NAME}/unsubscribe/reminders/idonotexist" + expect(page).to have_content "We can’t find your subscription" + end +end diff --git a/spec/forms/current_school_form_spec.rb b/spec/forms/current_school_form_spec.rb index ac9afa847e..b68836251f 100644 --- a/spec/forms/current_school_form_spec.rb +++ b/spec/forms/current_school_form_spec.rb @@ -19,14 +19,6 @@ ) end - context "unpermitted claim param" do - let(:params) { ActionController::Parameters.new({slug: slug, claim: {nonsense_id: 1}}) } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "#schools" do context "new form" do let(:params) { ActionController::Parameters.new({slug: slug, claim: {}}) } diff --git a/spec/forms/employed_directly_form_spec.rb b/spec/forms/employed_directly_form_spec.rb index 6ecda4cf73..55cf332164 100644 --- a/spec/forms/employed_directly_form_spec.rb +++ b/spec/forms/employed_directly_form_spec.rb @@ -17,14 +17,6 @@ ) end - context "unpermitted claim param" do - let(:params) { ActionController::Parameters.new({slug:, claim: {random_param: 1}}) } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "#employed_directly" do let(:params) { ActionController::Parameters.new({slug:, claim: {}}) } diff --git a/spec/forms/form_spec.rb b/spec/forms/form_spec.rb index ebfa528cd0..6df0bb5002 100644 --- a/spec/forms/form_spec.rb +++ b/spec/forms/form_spec.rb @@ -48,14 +48,6 @@ def initialize(journey_session) let(:claim_params) { {first_name: "test-name"} } describe "#initialize" do - context "with unpermitted params" do - let(:claim_params) { {unpermitted: "my-name"} } - - it "raises an error" do - expect { form }.to raise_error(ActionController::UnpermittedParameters) - end - end - context "with valid params" do let(:claim_params) { {first_name: "my-name"} } @@ -218,13 +210,5 @@ def initialize(journey_session) expect(form.permitted_params).to eql(ActionController::Parameters.new(claim_params.stringify_keys).permit!) end end - - context "with params containing attributes not defined on the form" do - let(:claim_params) { super().merge(unpermitted_attribute: "test-value") } - - it "raises an error" do - expect { form.permitted_params }.to raise_error(ActionController::UnpermittedParameters) - end - end end end diff --git a/spec/forms/gender_form_spec.rb b/spec/forms/gender_form_spec.rb index 44b1587993..f31b88f025 100644 --- a/spec/forms/gender_form_spec.rb +++ b/spec/forms/gender_form_spec.rb @@ -25,14 +25,6 @@ ) end - context "unpermitted claim param" do - let(:params) { ActionController::Parameters.new({slug: slug, claim: {nonsense_id: 1}}) } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "#payroll_gender" do let(:params) { ActionController::Parameters.new({slug: slug, claim: {}}) } diff --git a/spec/forms/journeys/additional_payments_for_teaching/entire_term_contract_form_spec.rb b/spec/forms/journeys/additional_payments_for_teaching/entire_term_contract_form_spec.rb index 6fd7e257a0..3c29ae7d14 100644 --- a/spec/forms/journeys/additional_payments_for_teaching/entire_term_contract_form_spec.rb +++ b/spec/forms/journeys/additional_payments_for_teaching/entire_term_contract_form_spec.rb @@ -17,14 +17,6 @@ ) end - context "unpermitted claim param" do - let(:params) { ActionController::Parameters.new({slug:, claim: {random_param: 1}}) } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "#has_entire_term_contract" do let(:params) { ActionController::Parameters.new({slug:, claim: {}}) } diff --git a/spec/forms/journeys/additional_payments_for_teaching/induction_completed_form_spec.rb b/spec/forms/journeys/additional_payments_for_teaching/induction_completed_form_spec.rb index 01f2e4bf20..285f522d93 100644 --- a/spec/forms/journeys/additional_payments_for_teaching/induction_completed_form_spec.rb +++ b/spec/forms/journeys/additional_payments_for_teaching/induction_completed_form_spec.rb @@ -17,14 +17,6 @@ ) end - context "unpermitted claim param" do - let(:params) { ActionController::Parameters.new({slug: slug, claim: {nonsense_id: 1}}) } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "#save" do let(:params) { ActionController::Parameters.new({slug: slug, claim: {induction_completed: true}}) } diff --git a/spec/forms/journeys/additional_payments_for_teaching/itt_academic_year_form_spec.rb b/spec/forms/journeys/additional_payments_for_teaching/itt_academic_year_form_spec.rb index 7cbdd1c6d6..676285cbbe 100644 --- a/spec/forms/journeys/additional_payments_for_teaching/itt_academic_year_form_spec.rb +++ b/spec/forms/journeys/additional_payments_for_teaching/itt_academic_year_form_spec.rb @@ -31,14 +31,6 @@ ) end - context "unpermitted claim param" do - let(:params) { ActionController::Parameters.new({slug: slug, claim: {nonsense_id: 1}}) } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "#itt_academic_year" do let(:params) { ActionController::Parameters.new({slug: slug, claim: {}}) } diff --git a/spec/forms/journeys/teacher_student_loan_reimbursement/claim_school_form_spec.rb b/spec/forms/journeys/teacher_student_loan_reimbursement/claim_school_form_spec.rb index 8a99a0d7b5..ed75fd423a 100644 --- a/spec/forms/journeys/teacher_student_loan_reimbursement/claim_school_form_spec.rb +++ b/spec/forms/journeys/teacher_student_loan_reimbursement/claim_school_form_spec.rb @@ -34,14 +34,6 @@ ) end - context "unpermitted claim param" do - let(:params) { ActionController::Parameters.new({slug: slug, claim: {nonsense_id: 1}}) } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "#schools" do context "new form" do let(:params) { ActionController::Parameters.new({slug: slug, claim: {}}) } diff --git a/spec/forms/personal_details_form_spec.rb b/spec/forms/personal_details_form_spec.rb index 6b8341d741..0c9c5b255d 100644 --- a/spec/forms/personal_details_form_spec.rb +++ b/spec/forms/personal_details_form_spec.rb @@ -30,16 +30,6 @@ ) end - context "unpermitted claim param" do - let(:params) { {nonsense_id: 1} } - let(:logged_in_with_tid) { nil } - let(:teacher_id_user_info) { {} } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "#show_name_section?" do context "when not logged_in_with_tid" do let(:logged_in_with_tid) { nil } diff --git a/spec/forms/poor_performance_form_spec.rb b/spec/forms/poor_performance_form_spec.rb index d4cf89040b..5f7d55b42d 100644 --- a/spec/forms/poor_performance_form_spec.rb +++ b/spec/forms/poor_performance_form_spec.rb @@ -11,14 +11,6 @@ it { expect(form).to be_a(Form) } - context "with unpermitted params" do - let(:claim_params) { {unpermitted_param: ""} } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "validations" do context "subject_to_formal_performance_action" do it "cannot be nil" do diff --git a/spec/forms/provide_mobile_number_form_spec.rb b/spec/forms/provide_mobile_number_form_spec.rb index 4b160b1cfd..02ceb4f9ca 100644 --- a/spec/forms/provide_mobile_number_form_spec.rb +++ b/spec/forms/provide_mobile_number_form_spec.rb @@ -29,16 +29,6 @@ ) end - context "unpermitted claim param" do - let(:provide_mobile_number) { nil } - - let(:params) { ActionController::Parameters.new({slug: slug, claim: {nonsense_id: 1}}) } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "validations" do let(:provide_mobile_number) { nil } diff --git a/spec/forms/supply_teacher_form_spec.rb b/spec/forms/supply_teacher_form_spec.rb index 52d2f33ace..6969780ec7 100644 --- a/spec/forms/supply_teacher_form_spec.rb +++ b/spec/forms/supply_teacher_form_spec.rb @@ -17,14 +17,6 @@ ) end - context "unpermitted claim param" do - let(:params) { ActionController::Parameters.new({slug:, claim: {random_param: 1}}) } - - it "raises an error" do - expect { form }.to raise_error ActionController::UnpermittedParameters - end - end - describe "#employed_as_supply_teacher" do let(:params) { ActionController::Parameters.new({slug:, claim: {}}) } diff --git a/spec/forms/unsubscribe/confirmation_form_spec.rb b/spec/forms/unsubscribe/confirmation_form_spec.rb new file mode 100644 index 0000000000..9ae101be09 --- /dev/null +++ b/spec/forms/unsubscribe/confirmation_form_spec.rb @@ -0,0 +1,46 @@ +require "rails_helper" + +RSpec.describe Unsubscribe::ConfirmationForm do + describe "#obfuscasted_email" do + let(:reminder) { + create( + :reminder, + email_address: + ) + } + + subject { described_class.new(id: reminder.id) } + + context "happy path" do + let(:email_address) { "hello@example.com" } + + it "obfuscates" do + expect(subject.obfuscasted_email).to eql "h***o@example.com" + end + end + + context "1 char email" do + let(:email_address) { "h@example.com" } + + it "obfuscates" do + expect(subject.obfuscasted_email).to eql "*@example.com" + end + end + + context "2 char email" do + let(:email_address) { "hh@example.com" } + + it "obfuscates" do + expect(subject.obfuscasted_email).to eql "**@example.com" + end + end + + context "3 char email" do + let(:email_address) { "hah@example.com" } + + it "obfuscates" do + expect(subject.obfuscasted_email).to eql "***@example.com" + end + end + end +end diff --git a/spec/mailers/reminder_mailer_spec.rb b/spec/mailers/reminder_mailer_spec.rb new file mode 100644 index 0000000000..ad94f31a3f --- /dev/null +++ b/spec/mailers/reminder_mailer_spec.rb @@ -0,0 +1,15 @@ +require "rails_helper" + +RSpec.describe ReminderMailer do + let(:reminder) { + create(:reminder, :with_fe_reminder) + } + + describe "#reminder_set" do + it "includes unsubscribe_url in personalisation" do + mail = described_class.reminder_set(reminder) + + expect(mail.personalisation[:unsubscribe_url]).to eql("https://www.example.com/further-education-payments/unsubscribe/reminders/#{reminder.id}") + end + end +end diff --git a/spec/requests/admin/admin_amendments_spec.rb b/spec/requests/admin/admin_amendments_spec.rb index ac416edf14..20c9868604 100644 --- a/spec/requests/admin/admin_amendments_spec.rb +++ b/spec/requests/admin/admin_amendments_spec.rb @@ -87,15 +87,11 @@ expect(response.body).to include("To amend the claim you must change at least one value") end - it "raises an error and does not create an amendment or update the claim when attempting to modify an attribute that isn't in the allowed list" do + it "does not create an amendment or update the claim when attempting to modify an attribute that isn't in the allowed list" do original_counts = [claim.reference, claim.amendments.size] - expect { - post admin_claim_amendments_url(claim, amendment: {claim: {reference: "REF12345"}, - notes: "Claimant made a typo"}) - }.to raise_error( - ActionController::UnpermittedParameters - ) + post admin_claim_amendments_url(claim, amendment: {claim: {reference: "REF12345"}, + notes: "Claimant made a typo"}) expect([claim.reference, claim.amendments.size]).to eq(original_counts) end diff --git a/spec/requests/unsubscribe_spec.rb b/spec/requests/unsubscribe_spec.rb new file mode 100644 index 0000000000..053e929d26 --- /dev/null +++ b/spec/requests/unsubscribe_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe "unsubscribe", type: :request do + let!(:reminder) { create(:reminder, :with_fe_reminder) } + + describe "POST #create" do + context "happy path" do + it "unsubscribes from reminder via one click unsubscribe" do + expect { + post "/further-education-payments/unsubscribe/reminders", params: {id: reminder.id} + }.to change(Reminder, :count).by(-1) + + expect(response).to be_successful + end + end + + context "when no such reminder" do + it "returns 400 error" do + post "/further-education-payments/unsubscribe/reminders", params: {id: "idonotexist"} + + expect(response).to be_bad_request + end + end + end +end From 07c149fb2dcee66326d247436d8edf1cdf7a3ab0 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 6 Dec 2024 11:20:39 +0000 Subject: [PATCH 02/10] unsubscribe soft deletes reminder --- app/controllers/unsubscribe_controller.rb | 2 +- app/forms/unsubscribe/confirmation_form.rb | 2 +- app/models/reminder.rb | 5 +++++ ...41206105631_add_deleted_at_to_reminders.rb | 5 +++++ db/schema.rb | 3 ++- spec/factories/reminders.rb | 4 ++++ spec/features/unsubscribe_spec.rb | 21 ++++++++++++++++--- spec/requests/unsubscribe_spec.rb | 20 +++++++++++++----- 8 files changed, 51 insertions(+), 11 deletions(-) create mode 100644 db/migrate/20241206105631_add_deleted_at_to_reminders.rb diff --git a/app/controllers/unsubscribe_controller.rb b/app/controllers/unsubscribe_controller.rb index b0f7ae2efc..bd2551e6e6 100644 --- a/app/controllers/unsubscribe_controller.rb +++ b/app/controllers/unsubscribe_controller.rb @@ -15,7 +15,7 @@ def create if @form.reminder.nil? head :bad_request else - @form.reminder.destroy + @form.reminder.soft_delete! end end diff --git a/app/forms/unsubscribe/confirmation_form.rb b/app/forms/unsubscribe/confirmation_form.rb index 726180ac3c..3ee53d7d46 100644 --- a/app/forms/unsubscribe/confirmation_form.rb +++ b/app/forms/unsubscribe/confirmation_form.rb @@ -6,7 +6,7 @@ class ConfirmationForm attribute :id, :string def reminder - @reminder ||= Reminder.find_by(id:) + @reminder ||= Reminder.not_deleted.find_by(id:) end def obfuscasted_email diff --git a/app/models/reminder.rb b/app/models/reminder.rb index 4f59c79262..835eae7dc3 100644 --- a/app/models/reminder.rb +++ b/app/models/reminder.rb @@ -7,6 +7,7 @@ class Reminder < ApplicationRecord scope :by_journey, ->(journey) { where(journey_class: journey.to_s) } scope :inside_academic_year, -> { where(itt_academic_year: AcademicYear.current.to_s) } scope :to_be_sent, -> { email_verified.not_yet_sent.inside_academic_year } + scope :not_deleted, -> { where(deleted_at: nil) } def journey journey_class.constantize @@ -21,4 +22,8 @@ def itt_academic_year read_attribute(:itt_academic_year) ) end + + def soft_delete! + update!(deleted_at: Time.now) + end end diff --git a/db/migrate/20241206105631_add_deleted_at_to_reminders.rb b/db/migrate/20241206105631_add_deleted_at_to_reminders.rb new file mode 100644 index 0000000000..ac3fdad0bc --- /dev/null +++ b/db/migrate/20241206105631_add_deleted_at_to_reminders.rb @@ -0,0 +1,5 @@ +class AddDeletedAtToReminders < ActiveRecord::Migration[7.2] + def change + add_column :reminders, :deleted_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index d82cd6d019..ce7d2dbb69 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2024_12_05_121421) do +ActiveRecord::Schema[8.0].define(version: 2024_12_06_105631) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_catalog.plpgsql" @@ -429,6 +429,7 @@ t.string "itt_academic_year", limit: 9 t.string "itt_subject" t.text "journey_class", null: false + t.datetime "deleted_at" t.index ["journey_class"], name: "index_reminders_on_journey_class" end diff --git a/spec/factories/reminders.rb b/spec/factories/reminders.rb index fe72cc3990..5b143f6766 100644 --- a/spec/factories/reminders.rb +++ b/spec/factories/reminders.rb @@ -11,5 +11,9 @@ trait :with_fe_reminder do journey_class { Journeys::FurtherEducationPayments.to_s } end + + trait :soft_deleted do + deleted_at { 1.second.ago } + end end end diff --git a/spec/features/unsubscribe_spec.rb b/spec/features/unsubscribe_spec.rb index 7b7e742af6..5e3b0f7816 100644 --- a/spec/features/unsubscribe_spec.rb +++ b/spec/features/unsubscribe_spec.rb @@ -12,9 +12,9 @@ visit "/#{reminder.journey::ROUTING_NAME}/unsubscribe/reminders/#{reminder.id}" expect(page).to have_content "Are you sure you wish to unsub" - expect { - click_button("Unsubscribe") - }.to change(Reminder, :count).by(-1) + click_button("Unsubscribe") + + expect(reminder.reload.deleted_at).to be_present expect(page).to have_content "Unsubscribe complete" end @@ -23,4 +23,19 @@ visit "/#{reminder.journey::ROUTING_NAME}/unsubscribe/reminders/idonotexist" expect(page).to have_content "We can’t find your subscription" end + + context "when reminder already soft deleted" do + let(:reminder) do + create( + :reminder, + :soft_deleted, + journey_class: Journeys::FurtherEducationPayments.to_s + ) + end + + scenario "cannot find subscription" do + visit "/#{reminder.journey::ROUTING_NAME}/unsubscribe/reminders/#{reminder.id}" + expect(page).to have_content "We can’t find your subscription" + end + end end diff --git a/spec/requests/unsubscribe_spec.rb b/spec/requests/unsubscribe_spec.rb index 053e929d26..c58a25b02b 100644 --- a/spec/requests/unsubscribe_spec.rb +++ b/spec/requests/unsubscribe_spec.rb @@ -1,14 +1,14 @@ require "rails_helper" RSpec.describe "unsubscribe", type: :request do - let!(:reminder) { create(:reminder, :with_fe_reminder) } + let(:reminder) { create(:reminder, :with_fe_reminder) } describe "POST #create" do context "happy path" do - it "unsubscribes from reminder via one click unsubscribe" do - expect { - post "/further-education-payments/unsubscribe/reminders", params: {id: reminder.id} - }.to change(Reminder, :count).by(-1) + it "sets deleted_at from reminder via one click unsubscribe" do + post "/further-education-payments/unsubscribe/reminders", params: {id: reminder.id} + + expect(reminder.reload.deleted_at).to be_present expect(response).to be_successful end @@ -21,5 +21,15 @@ expect(response).to be_bad_request end end + + context "when already soft deleted" do + let(:reminder) { create(:reminder, :with_fe_reminder, :soft_deleted) } + + it "returns 400 error" do + post "/further-education-payments/unsubscribe/reminders", params: {id: reminder.id} + + expect(response).to be_bad_request + end + end end end From 41d05575824e74eece060017f4576edce46ab96c Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 6 Dec 2024 13:09:57 +0000 Subject: [PATCH 03/10] reminders now supports reverse soft delete --- .../reminders/email_verification_form.rb | 6 +- app/forms/reminders/personal_details_form.rb | 2 + .../reminders/email_verification_form_spec.rb | 56 +++++++++++++++++++ .../reminders/personal_details_form_spec.rb | 43 ++++++++++++++ 4 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 spec/forms/reminders/email_verification_form_spec.rb create mode 100644 spec/forms/reminders/personal_details_form_spec.rb diff --git a/app/forms/reminders/email_verification_form.rb b/app/forms/reminders/email_verification_form.rb index 9fa29889a3..1fa86eaaef 100644 --- a/app/forms/reminders/email_verification_form.rb +++ b/app/forms/reminders/email_verification_form.rb @@ -25,6 +25,8 @@ def save! journey_class: journey.to_s ) + reminder.update!(deleted_at: nil) + ReminderMailer.reminder_set(reminder).deliver_now end @@ -34,7 +36,9 @@ def set_reminder_from_claim private def itt_subject - journey_session.answers.eligible_itt_subject + if journey_session.answers.respond_to?(:eligible_itt_subject) + journey_session.answers.eligible_itt_subject + end end def next_academic_year diff --git a/app/forms/reminders/personal_details_form.rb b/app/forms/reminders/personal_details_form.rb index f6dd71a987..d5b70d29d2 100644 --- a/app/forms/reminders/personal_details_form.rb +++ b/app/forms/reminders/personal_details_form.rb @@ -53,6 +53,8 @@ def set_reminder_from_claim journey_class: journey.to_s ) + reminder.update!(deleted_at: nil) + ReminderMailer.reminder_set(reminder).deliver_now else false diff --git a/spec/forms/reminders/email_verification_form_spec.rb b/spec/forms/reminders/email_verification_form_spec.rb new file mode 100644 index 0000000000..1d773408c5 --- /dev/null +++ b/spec/forms/reminders/email_verification_form_spec.rb @@ -0,0 +1,56 @@ +require "rails_helper" + +RSpec.describe Reminders::EmailVerificationForm do + let!(:journey_configuration) do + create(:journey_configuration, :further_education_payments) + end + + let(:journey) { Journeys::FurtherEducationPayments } + let(:journey_session) do + create( + :further_education_payments_session, + answers: { + reminder_full_name: "John Doe", + reminder_email_address: "john.doe@example.com" + } + ) + end + + subject do + described_class.new( + journey_session:, + journey:, + params: ActionController::Parameters.new, + session: {} + ) + end + + describe "#save!" do + let(:fake_passing_validator) { OpenStruct.new(valid?: true) } + + before do + allow(OneTimePassword::Validator).to receive(:new).and_return(fake_passing_validator) + end + + context "when reminder previously soft deleted" do + let!(:reminder) do + create( + :reminder, + :soft_deleted, + full_name: "John Doe", + email_address: "john.doe@example.com", + email_verified: true, + itt_subject: nil, + itt_academic_year: journey_configuration.current_academic_year.succ, + journey_class: journey.to_s + ) + end + + it "becomes undeleted" do + expect { + subject.save! + }.to change { reminder.reload.deleted_at }.to(nil) + end + end + end +end diff --git a/spec/forms/reminders/personal_details_form_spec.rb b/spec/forms/reminders/personal_details_form_spec.rb new file mode 100644 index 0000000000..f3da958ac6 --- /dev/null +++ b/spec/forms/reminders/personal_details_form_spec.rb @@ -0,0 +1,43 @@ +require "rails_helper" + +RSpec.describe Reminders::PersonalDetailsForm do + let!(:journey_configuration) do + create(:journey_configuration, :further_education_payments) + end + + let(:journey) { Journeys::FurtherEducationPayments } + let(:journey_session) { create(:further_education_payments_session) } + let(:claim) { create(:claim, :submitted) } + + subject do + described_class.new( + journey_session:, + journey:, + params: ActionController::Parameters.new, + session: {"submitted_claim_id" => claim.id} + ) + end + + describe "#set_reminder_from_claim" do + context "when reminder previously soft deleted" do + let!(:reminder) do + create( + :reminder, + :soft_deleted, + full_name: claim.full_name, + email_address: claim.email_address, + email_verified: true, + itt_subject: claim.eligible_itt_subject, + itt_academic_year: journey_configuration.current_academic_year.succ, + journey_class: journey.to_s + ) + end + + it "becomes undeleted" do + expect { + subject.set_reminder_from_claim + }.to change { reminder.reload.deleted_at }.to(nil) + end + end + end +end From 11136dd20fbc67beda31f079503e3eaab8b35ae2 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 6 Dec 2024 13:16:19 +0000 Subject: [PATCH 04/10] update analytics config with reminders#deleted_at --- config/analytics.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/analytics.yml b/config/analytics.yml index 153be13283..709a5234bf 100644 --- a/config/analytics.yml +++ b/config/analytics.yml @@ -18,6 +18,7 @@ shared: - itt_subject - sent_one_time_password_at - journey_class + - deleted_at :tasks: - id - name From 8ea3507a9b518c380bfc61d71ad5b302af8b6b08 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Thu, 2 Jan 2025 12:14:45 +0000 Subject: [PATCH 05/10] reminders now uses Deletable module --- app/controllers/unsubscribe_controller.rb | 2 +- app/forms/reminders/email_verification_form.rb | 2 +- app/models/concerns/deletable.rb | 4 ++++ app/models/reminder.rb | 3 ++- db/migrate/20241206105631_add_deleted_at_to_reminders.rb | 2 +- 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/controllers/unsubscribe_controller.rb b/app/controllers/unsubscribe_controller.rb index bd2551e6e6..9d1010e73e 100644 --- a/app/controllers/unsubscribe_controller.rb +++ b/app/controllers/unsubscribe_controller.rb @@ -15,7 +15,7 @@ def create if @form.reminder.nil? head :bad_request else - @form.reminder.soft_delete! + @form.reminder.mark_as_deleted! end end diff --git a/app/forms/reminders/email_verification_form.rb b/app/forms/reminders/email_verification_form.rb index 1fa86eaaef..4a6dcf5446 100644 --- a/app/forms/reminders/email_verification_form.rb +++ b/app/forms/reminders/email_verification_form.rb @@ -25,7 +25,7 @@ def save! journey_class: journey.to_s ) - reminder.update!(deleted_at: nil) + reminder.undelete! ReminderMailer.reminder_set(reminder).deliver_now end diff --git a/app/models/concerns/deletable.rb b/app/models/concerns/deletable.rb index 5b846b0a80..0e09cae5db 100644 --- a/app/models/concerns/deletable.rb +++ b/app/models/concerns/deletable.rb @@ -16,4 +16,8 @@ def mark_as_deleted! update!(deleted_at: Time.zone.now) end + + def undelete! + update!(deleted_at: nil) + end end diff --git a/app/models/reminder.rb b/app/models/reminder.rb index 835eae7dc3..58942a1f5b 100644 --- a/app/models/reminder.rb +++ b/app/models/reminder.rb @@ -1,4 +1,6 @@ class Reminder < ApplicationRecord + include Deletable + attribute :sent_one_time_password_at, :datetime attribute :one_time_password, :string, limit: 6 @@ -7,7 +9,6 @@ class Reminder < ApplicationRecord scope :by_journey, ->(journey) { where(journey_class: journey.to_s) } scope :inside_academic_year, -> { where(itt_academic_year: AcademicYear.current.to_s) } scope :to_be_sent, -> { email_verified.not_yet_sent.inside_academic_year } - scope :not_deleted, -> { where(deleted_at: nil) } def journey journey_class.constantize diff --git a/db/migrate/20241206105631_add_deleted_at_to_reminders.rb b/db/migrate/20241206105631_add_deleted_at_to_reminders.rb index ac3fdad0bc..cebdab8475 100644 --- a/db/migrate/20241206105631_add_deleted_at_to_reminders.rb +++ b/db/migrate/20241206105631_add_deleted_at_to_reminders.rb @@ -1,4 +1,4 @@ -class AddDeletedAtToReminders < ActiveRecord::Migration[7.2] +class AddDeletedAtToReminders < ActiveRecord::Migration[8.0] def change add_column :reminders, :deleted_at, :datetime end From f7bb8bdc001e2b8f59c2fe1bd3f212a0b6761fbc Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 3 Jan 2025 09:33:43 +0000 Subject: [PATCH 06/10] tweak unsubscribe copy --- app/views/unsubscribe/show.html.erb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/views/unsubscribe/show.html.erb b/app/views/unsubscribe/show.html.erb index 4011922864..6a9d73be09 100644 --- a/app/views/unsubscribe/show.html.erb +++ b/app/views/unsubscribe/show.html.erb @@ -10,7 +10,7 @@ <%= f.hidden_field :id %>

- Are you sure you wish to unsubscribe from your <%= @form.journey_name %> reminder? + Are you sure you wish to unsubscribe from your reminder?

@@ -21,6 +21,10 @@ However, you will still continue to receive updates about your claim as it progresses.

+

+ You will also continue to receive communications related to the claim service, please contact <%= govuk_mail_to I18n.t(:support_email_address) %> if you no longer wish these kinds of updates. +

+ <%= f.govuk_submit "Unsubscribe" %> <% end %> From 984532ce80a2e579a08fd2395c8f178fc2c1bb9d Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Thu, 9 Jan 2025 14:26:51 +0000 Subject: [PATCH 07/10] refactor to use named method --- app/forms/reminders/personal_details_form.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/forms/reminders/personal_details_form.rb b/app/forms/reminders/personal_details_form.rb index d5b70d29d2..8a9f5ec773 100644 --- a/app/forms/reminders/personal_details_form.rb +++ b/app/forms/reminders/personal_details_form.rb @@ -53,7 +53,7 @@ def set_reminder_from_claim journey_class: journey.to_s ) - reminder.update!(deleted_at: nil) + reminder.undelete! ReminderMailer.reminder_set(reminder).deliver_now else From 89a772b1f438f2cad415a165f4ca944b0bd0f270 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 10 Jan 2025 10:52:00 +0000 Subject: [PATCH 08/10] unsubscribe copy tweaks --- app/forms/unsubscribe/confirmation_form.rb | 3 ++- app/views/unsubscribe/show.html.erb | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/forms/unsubscribe/confirmation_form.rb b/app/forms/unsubscribe/confirmation_form.rb index 3ee53d7d46..dd3cf80945 100644 --- a/app/forms/unsubscribe/confirmation_form.rb +++ b/app/forms/unsubscribe/confirmation_form.rb @@ -23,7 +23,8 @@ def obfuscasted_email end def journey_name - I18n.t("journey_name", scope: reminder.journey::I18N_NAMESPACE).downcase + default = I18n.t("journey_name", scope: reminder.journey::I18N_NAMESPACE).downcase + I18n.t("policy_short_name", scope: reminder.journey::I18N_NAMESPACE, default:).downcase end end end diff --git a/app/views/unsubscribe/show.html.erb b/app/views/unsubscribe/show.html.erb index 6a9d73be09..c680dcd213 100644 --- a/app/views/unsubscribe/show.html.erb +++ b/app/views/unsubscribe/show.html.erb @@ -14,15 +14,15 @@

- You will no longer receive your reminder to <%= @form.obfuscasted_email %>. + You will no longer receive reminders regarding future application windows for targeted retention incentive payments from the Department for Education.

- However, you will still continue to receive updates about your claim as it progresses. + However, if you have already submitted a claim for a targeted retention incentive payment, you will still continue to receive updates about the progress of your claim.

- You will also continue to receive communications related to the claim service, please contact <%= govuk_mail_to I18n.t(:support_email_address) %> if you no longer wish these kinds of updates. + You may also continue to receive communications related to the Claim Additional Payments service. Please email <%= govuk_mail_to I18n.t(:support_email_address) %> if you no longer want to receive emails.

<%= f.govuk_submit "Unsubscribe" %> From f6cbb113673d9112fb17f424d95ba0ad7745dc31 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Thu, 16 Jan 2025 15:24:09 +0000 Subject: [PATCH 09/10] update global support email address --- config/locales/en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 69cd7ad387..85d1102038 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -43,7 +43,7 @@ en: format: unit: "£" service_name: "Claim additional payments for teaching" - support_email_address: "additionalteachingpayment@digital.education.gov.uk" + support_email_address: "additional.teachingpayment@education.gov.uk" check_your_answers: heading_send_application: Confirm and send your application statement: From b3209039cd5720c790c6e03a9177c857165446bd Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 17 Jan 2025 08:55:56 +0000 Subject: [PATCH 10/10] tweak reminder confirmation copy --- app/views/reminders/confirmation.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/reminders/confirmation.html.erb b/app/views/reminders/confirmation.html.erb index 60d6112cfe..cc2f591d01 100644 --- a/app/views/reminders/confirmation.html.erb +++ b/app/views/reminders/confirmation.html.erb @@ -9,7 +9,7 @@

What happens next

- We will send you a verification email now. If you don’t receive one, + We will send you a confirmation email now. If you don’t receive one, contact us on <%= mail_to t("support_email_address", scope: journey::I18N_NAMESPACE), t("support_email_address", scope: journey::I18N_NAMESPACE), class: "govuk-link" %>.