Skip to content

Commit

Permalink
Refactor email handling
Browse files Browse the repository at this point in the history
I believe these changes will make it easier to extend
the `annual_leave_request#update_status` method to
accomodate other actions, such as editing and
withdrawing annual leave requests.

The present solution is very rigid, requiring new
EmailsHelper methods to be written for each new
update action. The new solution simply needs a new
StatusUpdate class to be added for the new action,
and for that action to be added to the case options
in the private `status_update` method in
AnnualLeaveRequestController. This brings the code much
closer to being open for extension.
  • Loading branch information
Jonathan Young committed Sep 8, 2023
1 parent 8f5e6bb commit 2d9c4b7
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 128 deletions.
47 changes: 37 additions & 10 deletions app/controllers/annual_leave_requests_controller.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require "status_update"

class AnnualLeaveRequestsController < ApplicationController
def new
@annual_leave_request = AnnualLeaveRequest.new
Expand All @@ -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)
notify_client.send_email(new_request_email_hash)
redirect_to annual_leave_request_confirmation_path
else
render "new"
Expand All @@ -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
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

Expand All @@ -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
68 changes: 0 additions & 68 deletions app/helpers/emails_helper.rb

This file was deleted.

66 changes: 66 additions & 0 deletions lib/status_update.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions spec/controllers/annual_leave_requests_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

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_fake_client)
sign_in user
end

Expand Down Expand Up @@ -48,7 +48,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_fake_client)
sign_in line_manager
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/features/user/request_annual_leave_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 0 additions & 45 deletions spec/helpers/emails_helper_spec.rb

This file was deleted.

0 comments on commit 2d9c4b7

Please sign in to comment.