From a21b552eccb507d21c517fd38f552f14f65798b5 Mon Sep 17 00:00:00 2001 From: meyric Date: Fri, 23 Feb 2024 11:58:32 +0000 Subject: [PATCH] Update delivery method The delivery method is where the actual sending and in our case previewing happens - for that reason it is where the calls to the Notify API are made. We've updated this class to make it work for the new way messages are formed and for simplicity. We've stopped using webmock in the tests as that felt too much intergration, it felt like we are testing the notifications_client rather than our code as long as we pass the correct params to the client we should feel confident the right thing will happen. --- lib/mail/notify/delivery_method.rb | 42 ++- spec/mail/notify/delivery_method_spec.rb | 321 +++++++++++++++-------- spec/spec_helper.rb | 1 - 3 files changed, 222 insertions(+), 142 deletions(-) diff --git a/lib/mail/notify/delivery_method.rb b/lib/mail/notify/delivery_method.rb index ab4079f..deaa6c9 100644 --- a/lib/mail/notify/delivery_method.rb +++ b/lib/mail/notify/delivery_method.rb @@ -6,20 +6,28 @@ class DeliveryMethod attr_accessor :settings, :response def initialize(settings) - raise ArgumentError, "You must specify an API key" if settings[:api_key].blank? + raise ArgumentError, "You must specify an Notify API key" if settings[:api_key].blank? @settings = settings end - def deliver!(mail) - @mail = mail - @personalisation = Personalisation.new(mail) - send_email + def deliver!(message) + params = { + template_id: message.template_id, + email_address: message.to.first, + personalisation: message.personalisation, + email_reply_to_id: message.reply_to_id, + reference: message.reference + } + + client.send_email(params.compact) end - def preview(mail) - personalisation = Personalisation.new(mail).to_h - template_id = mail[:template_id].to_s + def preview(message) + template_id = message.template_id + personalisation = message.personalisation + + Rails.logger.info("Getting Notify preview for template id #{template_id}") client.generate_template_preview(template_id, personalisation: personalisation) end @@ -28,24 +36,6 @@ def preview(mail) def client @client ||= Notifications::Client.new(@settings[:api_key], @settings[:base_url]) end - - def email_params - { - email_address: @mail.to.first, - template_id: @mail[:template_id].to_s, - personalisation: @personalisation.to_h, - email_reply_to_id: optional_param(:reply_to_id), - reference: optional_param(:reference) - } - end - - def optional_param(name) - @mail[name].presence&.to_s - end - - def send_email - @response = client.send_email(email_params.compact) - end end end end diff --git a/spec/mail/notify/delivery_method_spec.rb b/spec/mail/notify/delivery_method_spec.rb index aab5fe5..5a64507 100644 --- a/spec/mail/notify/delivery_method_spec.rb +++ b/spec/mail/notify/delivery_method_spec.rb @@ -1,165 +1,256 @@ # frozen_string_literal: true require "spec_helper" +require "mailers/test_mailer" RSpec.describe Mail::Notify::DeliveryMethod do - before do - ActionMailer::Base.add_delivery_method(:notify, Mail::Notify::DeliveryMethod) - ActionMailer::Base.delivery_method = :notify - ActionMailer::Base.notify_settings = { - api_key: api_key - } + let(:mock_notifications_client) do + notifications_client = double(Notifications::Client) + allow(notifications_client).to receive(:send_email) + allow(notifications_client).to receive(:generate_template_preview) + allow(Notifications::Client).to receive(:new).and_return(notifications_client) + notifications_client end let(:api_key) do "staging-e1f4c969-b675-4a0d-a14d-623e7c2d3fd8-24fea27b-824e-4259-b5ce-1badafe98150" end - let(:notify) { double(:notify) } - let(:preview) { double(Notifications::Client::TemplatePreview) } - let(:mailer) { TestMailer.my_mail } - let(:personalisation) { {} } - let(:request_body) do - { - email_address: "myemail@gmail.com", - template_id: "template-id", - personalisation: personalisation - } - end - - let!(:stub) do - stub_request(:post, "https://api.notifications.service.gov.uk/v2/notifications/email") - .with(body: request_body) - .to_return(body: { - id: "aceed36e-6aee-494c-a09f-88b68904bad6" - }.to_json) - end - - context "when API key is blank" do - let(:api_key) { "" } - it "raises an error" do - expect { mailer.deliver! }.to raise_error( - ArgumentError, - "You must specify an API key" - ) - end + before do + ActionMailer::Base.add_delivery_method(:notify, Mail::Notify::DeliveryMethod) + ActionMailer::Base.delivery_method = :notify + ActionMailer::Base.notify_settings = { + api_key: api_key + } end - context "with a view" do - let(:personalisation) do - { - body: "# bar\r\n\r\nBar baz", - subject: "Hello there!" - } + describe "settings" do + let(:notify) { double(:notify) } + let(:preview) { double(Notifications::Client::TemplatePreview) } + let(:message) do + TestMailer.with( + template_id: "test-id", + to: "test.name@email.co.uk" + ).test_template_mail end - it "has access to the settings" do - expect(mailer.delivery_method.settings[:api_key]).to eq(api_key) - end + it "provides the API key when one is set" do + mock_notifications_client + settings = {api_key: api_key} - it "calls Notify's send_email service with the correct details " do - mailer.deliver! + delivery_method = described_class.new(settings) + delivery_method.deliver!(message) - expect(stub).to have_been_requested + expect(Notifications::Client).to have_received(:new).with(api_key, nil) end - it "gives access to the response" do - mailer.deliver! + it "raises an error when the API key is not set" do + mock_notifications_client + settings = {api_key: nil} - expect(mailer.delivery_method.response.id).to eq( - "aceed36e-6aee-494c-a09f-88b68904bad6" - ) + expect { described_class.new(settings) } + .to raise_error ArgumentError, "You must specify an Notify API key" end - it "shows a preview" do - preview_stub = stub_request(:post, "https://api.notifications.service.gov.uk/v2/template/template-id/preview") - .to_return(status: 200, body: {}.to_json) + it "provides the base url when one is set" do + mock_notifications_client + base_url = "example.com" + settings = {api_key: api_key, base_url: base_url} - mailer.preview + delivery_method = described_class.new(settings) + delivery_method.deliver!(message) - expect(preview_stub).to have_been_requested + expect(Notifications::Client).to have_received(:new).with(api_key, base_url) end end - context "with a template" do - let(:personalisation) do - { - foo: "bar" - } - end - let(:mailer) { TestMailer.my_other_mail } - - it "has access to the settings" do - expect(mailer.delivery_method.settings[:api_key]).to eq(api_key) + context "with a template email" do + context "without any personalisation" do + let(:message) do + TestMailer.with( + template_id: "test-id-without-personalisation", + to: "test.name@email.co.uk" + ).test_template_mail + end + + describe "#deliver!" do + it "calls the Notifications API with the correct params" do + notifications_client = mock_notifications_client + + message.delivery_method.deliver!(message) + + expect(notifications_client).to have_received(:send_email).with( + template_id: "test-id-without-personalisation", + email_address: "test.name@email.co.uk", + personalisation: {} + ) + end + end + + describe "#preview" do + it "calls the Notifications API with the correct params" do + notifications_client = mock_notifications_client + + message.delivery_method.preview(message) + + expect(notifications_client).to have_received(:generate_template_preview).with( + "test-id-without-personalisation", + {personalisation: {}} + ) + end + end end - it "calls Notify's send_email service with the correct details " do - mailer.deliver! + context "with personalisation" do + let(:message) { + TestMailer.with( + template_id: "test-id-with-personalisation", + to: "test.name@email.co.uk", + personalisation: {name: "Name", age: "25"} + ).test_template_mail + } - expect(stub).to have_been_requested - end + describe "#deliver!" do + it "calls the Notifications API with the correct params" do + notifications_client = mock_notifications_client - it "gives access to the response" do - mailer.deliver! + message.delivery_method.deliver!(message) - expect(mailer.delivery_method.response.id).to eq( - "aceed36e-6aee-494c-a09f-88b68904bad6" - ) - end + expect(notifications_client).to have_received(:send_email).with( + template_id: "test-id-with-personalisation", + email_address: "test.name@email.co.uk", + personalisation: {age: "25", name: "Name"} + ) + end + end - it "shows a preview" do - preview_stub = stub_request(:post, "https://api.notifications.service.gov.uk/v2/template/template-id/preview") - .to_return(status: 200, body: {}.to_json) + describe "#preiview" do + it "calls the Notifications API with the correct params" do + notifications_client = mock_notifications_client - mailer.preview + message.delivery_method.preview(message) - expect(preview_stub).to have_been_requested + expect(notifications_client).to have_received(:generate_template_preview).with( + "test-id-with-personalisation", + {personalisation: {age: "25", name: "Name"}} + ) + end + end end end - context "with optional fields included" do - let(:mailer) { TestMailer.my_mail_optional_fields } - let(:request_body) do - hash_including( - email_reply_to_id: "custom-reply-to-id", - reference: "ABC123XYZ" - ) - end + context "with a view email" do + let(:message) { + TestMailer.with( + template_id: "test-id-with-a-view", + to: "test.name@email.co.uk", + subject: "Another subject" + ).test_view_mail + } - it "calls Notify’s send_email service with the optional fields" do - mailer.deliver! + describe "#deliver!" do + it "calls the Notifications API with the correct params" do + notifications_client = mock_notifications_client - expect(stub).to have_been_requested - end - end + message.delivery_method.deliver!(message) - context "when a base url is specified" do - before do - ActionMailer::Base.notify_settings = { - api_key: api_key, - base_url: "http://example.com" - } + expect(notifications_client).to have_received(:send_email).with( + template_id: "test-id-with-a-view", + email_address: "test.name@email.co.uk", + personalisation: {subject: "Another subject", body: "This is the body from the mailers view.\r\n"} + ) + end end - let(:personalisation) do - { - body: "# bar\r\n\r\nBar baz", - subject: "Hello there!" - } - end + describe "#preview" do + it "calls the Notifications API with the correct params" do + notifications_client = mock_notifications_client + + message.delivery_method.preview(message) - let!(:stub) do - stub_request(:post, "http://example.com/v2/notifications/email") - .with(body: request_body) - .to_return(body: { - id: "aceed36e-6aee-494c-a09f-88b68904bad6" - }.to_json) + expect(notifications_client).to have_received(:generate_template_preview).with( + "test-id-with-a-view", + {personalisation: {subject: "Another subject", body: "This is the body from the mailers view.\r\n"}} + ) + end end + end - it "calls appends the new base url to the request" do - mailer.deliver! + describe "Notify optional fields" do + describe "email_reply_to_id" do + it "is present in the API call when set" do + message = TestMailer.with( + template_id: "test-id", + to: "test.name@email.co.uk", + reply_to_id: "test-reply-to-id" + ).test_template_mail + + notifications_client = mock_notifications_client + + message.delivery_method.deliver!(message) + + expect(notifications_client).to have_received(:send_email).with( + template_id: "test-id", + email_address: "test.name@email.co.uk", + personalisation: {}, + email_reply_to_id: "test-reply-to-id" + ) + end + + it "is not present in the call to the Notify API when not set" do + message = TestMailer.with( + template_id: "test-id", + to: "test.name@email.co.uk" + ).test_template_mail + + notifications_client = mock_notifications_client + + message.delivery_method.deliver!(message) + + expect(notifications_client).to have_received(:send_email).with( + template_id: "test-id", + email_address: "test.name@email.co.uk", + personalisation: {} + ) + end + end - expect(stub).to have_been_requested + describe "refence" do + it "is present in the API call when set" do + message = TestMailer.with( + template_id: "test-id", + to: "test.name@email.co.uk", + reference: "test-reference" + ).test_template_mail + + notifications_client = mock_notifications_client + + message.delivery_method.deliver!(message) + + expect(notifications_client).to have_received(:send_email).with( + template_id: "test-id", + email_address: "test.name@email.co.uk", + personalisation: {}, + reference: "test-reference" + ) + end + + it "is not present in the call to the Notify API when not set" do + message = TestMailer.with( + template_id: "test-id", + to: "test.name@email.co.uk" + ).test_template_mail + + notifications_client = mock_notifications_client + + message.delivery_method.deliver!(message) + + expect(notifications_client).to have_received(:send_email).with( + template_id: "test-id", + email_address: "test.name@email.co.uk", + personalisation: {} + ) + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index af240cb..f4e353d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,7 +14,6 @@ require "action_controller" require "action_mailer" require "pry" -require "webmock/rspec" require "mail/notify"