From 7b97567ad0f3aa7ada6ad26f78259004f1e4c467 Mon Sep 17 00:00:00 2001 From: Jonathan Young Date: Fri, 8 Sep 2023 13:50:35 +0100 Subject: [PATCH] Refactor emails_helper This refactors the emails_helper module so that it uses the same method for sending emails about status updates, whether it is denied or approved. This is with the aim of making the workflow more open to extension, because at the moment it is not. --- .../annual_leave_requests_controller.rb | 19 +++----- app/helpers/emails_helper.rb | 43 +++++++++++-------- spec/helpers/emails_helper_spec.rb | 20 ++++----- 3 files changed, 40 insertions(+), 42 deletions(-) diff --git a/app/controllers/annual_leave_requests_controller.rb b/app/controllers/annual_leave_requests_controller.rb index be3a817..e5c96a3 100644 --- a/app/controllers/annual_leave_requests_controller.rb +++ b/app/controllers/annual_leave_requests_controller.rb @@ -37,8 +37,8 @@ def update_status @annual_leave_request = line_reports_leave_requests.find(params[:annual_leave_request_id]) if @annual_leave_request.update(annual_leave_request_params) - send_status_update_email - redirect_to_status_update_confirmation_page + helpers.send_status_updated_email(@annual_leave_request) + redirect_to status_update_confirmation_page else render "approve" if annual_leave_request_params[:status] == "approved" render "deny" if annual_leave_request_params[:status] == "denied" @@ -63,21 +63,12 @@ def annual_leave_request_params params.require(:annual_leave_request).permit(:date_from, :date_to, :days_required, :status, :confirm_approval, :denial_reason) end - def send_status_update_email + def status_update_confirmation_page case annual_leave_request_params[:status] when "approved" - helpers.send_approved_request_email(@annual_leave_request) + confirm_annual_leave_request_approval_path when "denied" - helpers.send_denied_request_email(@annual_leave_request) - end - end - - def redirect_to_status_update_confirmation_page - case annual_leave_request_params[:status] - when "approved" - redirect_to confirm_annual_leave_request_approval_path - when "denied" - redirect_to confirm_annual_leave_request_denial_path + confirm_annual_leave_request_denial_path end end end diff --git a/app/helpers/emails_helper.rb b/app/helpers/emails_helper.rb index e7cd919..64c6cd4 100644 --- a/app/helpers/emails_helper.rb +++ b/app/helpers/emails_helper.rb @@ -16,11 +16,29 @@ def send_new_request_email(annual_leave_request) ) end - def send_approved_request_email(annual_leave_request) + def send_status_updated_email(annual_leave_request) + case annual_leave_request.status + when "approved" + client.send_email(approval_email_hash(annual_leave_request)) + when "denied" + client.send_email(denial_email_hash(annual_leave_request)) + end + end + +private + + def client + @client ||= Notifications::Client.new(notify_api_key) + end + + def notify_api_key + ENV["NOTIFY_API_KEY"] + end + + def approval_email_hash(annual_leave_request) user = annual_leave_request.user line_manager = user.line_manager - - client.send_email( + { email_address: user.email, template_id: "34542d49-8b91-412c-9393-c186a04a7d1c", personalisation: { @@ -29,14 +47,13 @@ def send_approved_request_email(annual_leave_request) date_from: annual_leave_request.date_from.to_fs(:rfc822), date_to: annual_leave_request.date_to.to_fs(:rfc822), }, - ) + } end - def send_denied_request_email(annual_leave_request) + def denial_email_hash(annual_leave_request) user = annual_leave_request.user line_manager = user.line_manager - - client.send_email( + { email_address: user.email, template_id: "ec9035df-9c98-4e0e-8826-47768c311745", personalisation: { @@ -46,16 +63,6 @@ def send_denied_request_email(annual_leave_request) date_to: annual_leave_request.date_to.to_fs(:rfc822), denial_reason: annual_leave_request.denial_reason, }, - ) - end - -private - - def client - @client ||= Notifications::Client.new(notify_api_key) - end - - def notify_api_key - ENV["NOTIFY_API_KEY"] + } end end diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb index c853f68..6eef6c0 100644 --- a/spec/helpers/emails_helper_spec.rb +++ b/spec/helpers/emails_helper_spec.rb @@ -18,28 +18,28 @@ end end - describe "#send_approved_request_email" do - it "sends an approval email to the user" do + describe "#send_status_updated_email" do + it "sends a denial email to the user when status is updated to 'denied'" do @client = Notifications::Client.new(ENV["NOTIFY_TEST_API_KEY"]) + annual_leave_request.status = "denied" + annual_leave_request.denial_reason = "some reason" - response_notification = helper.send_approved_request_email(annual_leave_request) + response_notification = helper.send_status_updated_email(annual_leave_request) response = client.get_notification(response_notification.id) expect(response.email_address).to eq(user.email) - expect(response.subject).to eq("GOV.UK Holiday Logger – Annual Leave Request Approved") + expect(response.subject).to eq("GOV.UK Holiday Logger – Annual Leave Request Denied") end - end - describe "#send_denied_request_email" do - it "sends a denial email to the user" do + it "sends an approval email to the user when status is updated to 'approved'" do @client = Notifications::Client.new(ENV["NOTIFY_TEST_API_KEY"]) - annual_leave_request.denial_reason = "some reason" + annual_leave_request.status = "approved" - response_notification = helper.send_denied_request_email(annual_leave_request) + response_notification = helper.send_status_updated_email(annual_leave_request) response = client.get_notification(response_notification.id) expect(response.email_address).to eq(user.email) - expect(response.subject).to eq("GOV.UK Holiday Logger – Annual Leave Request Denied") + expect(response.subject).to eq("GOV.UK Holiday Logger – Annual Leave Request Approved") end end end