Refactor emails handling

app/controllers/annual_leave_requests_controller.rb

@@ -1,3 +1,5 @@
+require "status_update"
+
 class AnnualLeaveRequestsController < ApplicationController
   def new
     @annual_leave_request = AnnualLeaveRequest.new
@@ -13,7 +15,7 @@ def create
     @annual_leave_request = current_user.annual_leave_requests.build(annual_leave_request_params)
 
     if @annual_leave_request.save
-      helpers.send_new_request_email(@annual_leave_request)
+      @email_response_notification = notify_client.send_email(new_request_email_hash)
       redirect_to annual_leave_request_confirmation_path
     else
       render "new"
@@ -37,11 +39,10 @@ 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)
-      helpers.send_status_updated_email(@annual_leave_request)
-      redirect_to status_update_confirmation_page
+      @email_response_notification = notify_client.send_email(status_update.email_hash)
+      redirect_to status_update.confirmation_page_path
     else
-      render "approve" if annual_leave_request_params[:status] == "approved"
-      render "deny" if annual_leave_request_params[:status] == "denied"
+      render status_update.action
     end
   end
 
@@ -63,12 +64,38 @@ def annual_leave_request_params
     params.require(:annual_leave_request).permit(:date_from, :date_to, :days_required, :status, :confirm_approval, :denial_reason)
   end
 
-  def status_update_confirmation_page
-    case annual_leave_request_params[:status]
+  def status_update
+    case @annual_leave_request.status
     when "approved"
-      confirm_annual_leave_request_approval_path
+      ApprovedStatusUpdate
     when "denied"
-      confirm_annual_leave_request_denial_path
-    end
+      DeniedStatusUpdate
+    end.new(@annual_leave_request)
+  end
+
+  def new_request_email_hash
+    {
+      email_address: line_manager.email,
+      template_id: "1587d50b-c12e-4698-b10e-cf414de26f36",
+      personalisation: {
+        line_manager_name: "#{line_manager.given_name} #{line_manager.family_name}",
+        name: "#{user.given_name} #{user.family_name}",
+        date_from: @annual_leave_request.date_from.to_fs(:rfc822),
+        date_to: @annual_leave_request.date_to.to_fs(:rfc822),
+        days_required: @annual_leave_request.days_required,
+      },
+    }
+  end
+
+  def notify_client
+    @notify_client ||= Notifications::Client.new(ENV["NOTIFY_API_KEY"])
+  end
+
+  def line_manager
+    @annual_leave_request.user.line_manager
+  end
+
+  def user
+    @annual_leave_request.user
   end
 end

lib/status_update.rb

@@ -0,0 +1,66 @@
+class ApprovedStatusUpdate
+  include Rails.application.routes.url_helpers
+
+  attr_reader :annual_leave_request, :user, :line_manager
+
+  def initialize(annual_leave_request)
+    @annual_leave_request = annual_leave_request
+    @user = annual_leave_request.user
+    @line_manager = user.line_manager
+  end
+
+  def confirmation_page_path
+    confirm_annual_leave_request_approval_path
+  end
+
+  def action
+    "approve"
+  end
+
+  def email_hash
+    {
+      email_address: user.email,
+      template_id: "34542d49-8b91-412c-9393-c186a04a7d1c",
+      personalisation: {
+        line_manager_name: "#{line_manager.given_name} #{line_manager.family_name}",
+        name: "#{user.given_name} #{user.family_name}",
+        date_from: annual_leave_request.date_from.to_fs(:rfc822),
+        date_to: annual_leave_request.date_to.to_fs(:rfc822),
+      },
+    }
+  end
+end
+
+class DeniedStatusUpdate
+  include Rails.application.routes.url_helpers
+
+  attr_reader :annual_leave_request, :user, :line_manager
+
+  def initialize(annual_leave_request)
+    @annual_leave_request = annual_leave_request
+    @user = annual_leave_request.user
+    @line_manager = user.line_manager
+  end
+
+  def confirmation_page_path
+    confirm_annual_leave_request_denial_path
+  end
+
+  def action
+    "deny"
+  end
+
+  def email_hash
+    {
+      email_address: user.email,
+      template_id: "ec9035df-9c98-4e0e-8826-47768c311745",
+      personalisation: {
+        line_manager_name: "#{line_manager.given_name} #{line_manager.family_name}",
+        name: "#{user.given_name} #{user.family_name}",
+        date_from: annual_leave_request.date_from.to_fs(:rfc822),
+        date_to: annual_leave_request.date_to.to_fs(:rfc822),
+        denial_reason: annual_leave_request.denial_reason,
+      },
+    }
+  end
+end

spec/controllers/annual_leave_requests_controller_spec.rb

@@ -1,35 +1,29 @@
 RSpec.describe AnnualLeaveRequestsController do
   let(:user) { create(:user, line_manager_id: line_manager.id) }
+  let(:user_full_name) { "#{user.given_name} #{user.family_name}" }
   let(:line_manager) { create(:user, email: "[email protected]") }
-  let(:notify_fake_client) { instance_double(Notifications::Client, send_email: "FakeNotificationResponse") }
+  let(:notify_test_client) { Notifications::Client.new(ENV["NOTIFY_TEST_API_KEY"]) }
 
   describe "POST create" do
     setup do
-      allow(Notifications::Client).to receive(:new).and_return(notify_fake_client)
+      allow(controller).to receive(:notify_client).and_return(notify_test_client)
       sign_in user
     end
 
     it "emails line manager and redirects to confirmation page if annual leave request valid" do
       valid_request = build(:annual_leave_request, user_id: user.id)
-      new_request_email_hash = {
-        email_address: line_manager.email,
-        template_id: "1587d50b-c12e-4698-b10e-cf414de26f36",
-        personalisation: {
-          line_manager_name: "#{line_manager.given_name} #{line_manager.family_name}",
-          name: "#{user.given_name} #{user.family_name}",
-          date_from: valid_request.date_from.to_fs(:rfc822),
-          date_to: valid_request.date_to.to_fs(:rfc822),
-          days_required: valid_request.days_required,
-        },
-      }
 
       post :create, params: { annual_leave_request: {
         date_from: valid_request.date_from,
         date_to: valid_request.date_to,
         days_required: valid_request.days_required,
       } }
 
-      expect(notify_fake_client).to have_received(:send_email).with(new_request_email_hash)
+      email_response_notification = assigns(:email_response_notification)
+      email_response = notify_test_client.get_notification(email_response_notification.id)
+
+      expect(email_response.email_address).to eq(line_manager.email)
+      expect(email_response.subject).to eq("GOV.UK Holiday Logger – #{user_full_name} – New Annual Leave Request")
       expect(response).to redirect_to(annual_leave_request_confirmation_path)
     end
 
@@ -48,7 +42,7 @@
     let(:leave_request) { create(:annual_leave_request, user_id: user.id) }
 
     setup do
-      allow(Notifications::Client).to receive(:new).and_return(notify_fake_client)
+      allow(controller).to receive(:notify_client).and_return(notify_test_client)
       sign_in line_manager
     end
 
@@ -76,18 +70,7 @@
       expect(response).to render_template(:deny)
     end
 
-    it "sends an email to the line report when status is updated to approved" do
-      approved_request_email_hash = {
-        email_address: user.email,
-        template_id: "34542d49-8b91-412c-9393-c186a04a7d1c",
-        personalisation: {
-          line_manager_name: "#{line_manager.given_name} #{line_manager.family_name}",
-          name: "#{user.given_name} #{user.family_name}",
-          date_from: leave_request.date_from.to_fs(:rfc822),
-          date_to: leave_request.date_to.to_fs(:rfc822),
-        },
-      }
-
+    it "sends an email to the line report and redirects to confirmatioon page when status is updated to 'approved'" do
       patch :update_status, params: {
         annual_leave_request_id: leave_request.id,
         annual_leave_request: {
@@ -96,31 +79,29 @@
         },
       }
 
-      expect(notify_fake_client).to have_received(:send_email).with(approved_request_email_hash)
+      email_response_notification = assigns(:email_response_notification)
+      email_response = notify_test_client.get_notification(email_response_notification.id)
+
+      expect(email_response.email_address).to eq(user.email)
+      expect(email_response.subject).to eq("GOV.UK Holiday Logger – Annual Leave Request Approved")
+      expect(response).to redirect_to(confirm_annual_leave_request_approval_path)
     end
 
-    it "sends an email to the line report when status is updated to 'denied'" do
+    it "sends an email to the line report and redirects to confirmatioon page when status is updated to 'denied'" do
       patch :update_status, params: {
         annual_leave_request_id: leave_request.id,
         annual_leave_request: {
           status: "denied",
           denial_reason: "some valid reason",
         },
       }
-      leave_request.reload
-      denied_request_email_hash = {
-        email_address: user.email,
-        template_id: "ec9035df-9c98-4e0e-8826-47768c311745",
-        personalisation: {
-          line_manager_name: "#{line_manager.given_name} #{line_manager.family_name}",
-          name: "#{user.given_name} #{user.family_name}",
-          date_from: leave_request.date_from.to_fs(:rfc822),
-          date_to: leave_request.date_to.to_fs(:rfc822),
-          denial_reason: leave_request.denial_reason,
-        },
-      }
 
-      expect(notify_fake_client).to have_received(:send_email).with(denied_request_email_hash)
+      email_response_notification = assigns(:email_response_notification)
+      email_response = notify_test_client.get_notification(email_response_notification.id)
+
+      expect(email_response.email_address).to eq(user.email)
+      expect(email_response.subject).to eq("GOV.UK Holiday Logger – Annual Leave Request Denied")
+      expect(response).to redirect_to(confirm_annual_leave_request_denial_path)
     end
   end
 end

spec/features/line_manager/approve_annual_leave_request_spec.rb

@@ -3,7 +3,7 @@
 RSpec.feature "Line manager can approve an annual leave request" do
   setup do
     notify_test_client = Notifications::Client.new(ENV["NOTIFY_TEST_API_KEY"])
-    allow_any_instance_of(EmailsHelper).to receive(:client).and_return(notify_test_client) # rubocop:disable RSpec/AnyInstance
+    allow_any_instance_of(AnnualLeaveRequestsController).to receive(:notify_client).and_return(notify_test_client) # rubocop:disable RSpec/AnyInstance
   end
 
   scenario do

spec/features/line_manager/deny_annual_leave_request_spec.rb

@@ -3,7 +3,7 @@
 RSpec.feature "Line manager can deny an annual leave request" do
   setup do
     notify_test_client = Notifications::Client.new(ENV["NOTIFY_TEST_API_KEY"])
-    allow_any_instance_of(EmailsHelper).to receive(:client).and_return(notify_test_client) # rubocop:disable RSpec/AnyInstance
+    allow_any_instance_of(AnnualLeaveRequestsController).to receive(:notify_client).and_return(notify_test_client) # rubocop:disable RSpec/AnyInstance
   end
 
   scenario do

spec/features/user/request_annual_leave_spec.rb

@@ -6,7 +6,7 @@
 
   setup do
     notify_test_client = Notifications::Client.new(ENV["NOTIFY_TEST_API_KEY"])
-    allow_any_instance_of(EmailsHelper).to receive(:client).and_return(notify_test_client) # rubocop:disable RSpec/AnyInstance
+    allow_any_instance_of(AnnualLeaveRequestsController).to receive(:notify_client).and_return(notify_test_client) # rubocop:disable RSpec/AnyInstance
   end
 
   scenario do