Improving testing and putting back yesterdays stupid changes

Author: msquance-stem

app/jobs/send_cms_emails_job.rb

@@ -17,7 +17,7 @@ def process_template(template)
     users = Programmes::ProgressQuery.new(data.programme, data.activity_state, data.enrolled, data.completed_programme_activity_groups).call
 
     users.each do |user|
-      CmsMailer.with(template: data, user_id: user.id).send_template.deliver_later
+      CmsMailer.with(template_slug: data.slug, user_id: user.id).send_template.deliver_later
     end
   end
 end

app/mailers/cms_mailer.rb

@@ -1,10 +1,16 @@
 class CmsMailer < ApplicationMailer
   def send_template
-    @template = params[:template]
+    @template_slug = params[:template_slug]
     @user = User.find(params[:user_id])
 
-    @subject = @template.subject(@user)
+    begin
+      @template = Cms::Collections::EmailTemplate.get(@template_slug).template
 
-    mail(to: @user.email, subject: @subject)
+      @subject = @template.subject(@user)
+
+      mail(to: @user.email, subject: @subject)
+    rescue ActiveRecord::RecordNotFound
+      Sentry.capture_message("Failed to load the email template #{@template_slug}")
+    end
   end
 end

spec/jobs/send_cms_emails_job_spec.rb

@@ -3,6 +3,7 @@
 RSpec.describe SendCmsEmailsJob, type: :job do
   let(:email_template_1_slug) { "send-cms-email-template-1-slug" }
   let(:email_template_2_slug) { "send-cms-email-template-2-slug" }
+  let(:users) { create_list(:user, 2) }
 
   let(:email_template_1) {
     Cms::Mocks::EmailTemplate.generate_raw_data(slug: email_template_1_slug)
@@ -15,9 +16,16 @@
     stub_strapi_email_template(email_template_1_slug, email_template: email_template_1)
     stub_strapi_email_template(email_template_2_slug, email_template: email_template_2)
     stub_strapi_email_templates(email_templates: [email_template_1, email_template_2])
+
+    allow_any_instance_of(Programmes::ProgressQuery).to receive(:call).and_return(users)
   end
 
-  it "should iterate through templates" do
-    described_class.perform_now
+  it "should enqueue both templates" do
+    expect do
+      described_class.perform_now
+    end.to have_enqueued_mail(CmsMailer, :send_template).with(a_hash_including(params: {user_id: users.first.id, template_slug: email_template_1_slug}))
+      .and have_enqueued_mail(CmsMailer, :send_template).with(a_hash_including(params: {user_id: users.first.id, template_slug: email_template_2_slug}))
+      .and have_enqueued_mail(CmsMailer, :send_template).with(a_hash_including(params: {user_id: users.second.id, template_slug: email_template_1_slug}))
+      .and have_enqueued_mail(CmsMailer, :send_template).with(a_hash_including(params: {user_id: users.second.id, template_slug: email_template_2_slug}))
   end
 end

spec/mailers/cms_mailer_spec.rb

@@ -51,27 +51,28 @@
     ]
   }
   let(:email_template) {
-    Cms::Providers::Strapi::Factories::ModelFactory.to_email_template(
-      Cms::Mocks::EmailTemplate.generate_data(
-        subject:,
-        slug:,
-        email_content:
-      )
+    Cms::Mocks::EmailTemplate.generate_raw_data(
+      subject:,
+      slug:,
+      email_content:
     )
   }
   let(:email_template_merge_subject) {
-    Cms::Providers::Strapi::Factories::ModelFactory.to_email_template(
-      Cms::Mocks::EmailTemplate.generate_data(
-        subject: "Congrats {first_name} you did {last_cpd_title}",
-        slug: slug_with_merge_subject,
-        email_content:
-      )
+    Cms::Mocks::EmailTemplate.generate_raw_data(
+      subject: "Congrats {first_name} you did {last_cpd_title}",
+      slug: slug_with_merge_subject,
+      email_content:
     )
   }
 
+  before do
+    stub_strapi_email_template(slug, email_template:)
+    stub_strapi_email_template(slug_with_merge_subject, email_template: email_template_merge_subject)
+  end
+
   describe "send_template" do
     before do
-      @mail = CmsMailer.with(user_id: user.id, template: email_template).send_template
+      @mail = described_class.with(user_id: user.id, template_slug: slug).send_template
     end
 
     it "renders the headers" do
@@ -130,7 +131,7 @@
       before do
         travel_to 1.day.from_now do
           create(:completed_achievement, activity: second_activity, user:)
-          @future_mail = CmsMailer.with(user_id: user.id, template: email_template).send_template
+          @future_mail = described_class.with(user_id: user.id, template_slug: slug).send_template
         end
       end
 
@@ -146,7 +147,7 @@
 
   describe "send_template with merged subject" do
     before do
-      @mail = CmsMailer.with(user_id: user.id, template: email_template_merge_subject).send_template
+      @mail = described_class.with(user_id: user.id, template_slug: slug_with_merge_subject).send_template
     end
 
     it "renders the headers" do
@@ -155,4 +156,20 @@
       expect(@mail.from).to eq(["[email protected]"])
     end
   end
+
+  describe "with missing template" do
+    let(:missing_slug) { "missing_template_slug" }
+
+    before do
+      stub_strapi_email_template_missing(missing_slug)
+      allow(Sentry).to receive(:capture_message)
+    end
+
+    it "logs error to sentry" do
+      described_class.with(user_id: user.id, template_slug: missing_slug).send_template.deliver_now
+      expect(Sentry)
+        .to have_received(:capture_message)
+        .with("Failed to load the email template #{missing_slug}")
+    end
+  end
 end

spec/support/cms/providers/strapi/strapi_stubs.rb

@@ -174,6 +174,10 @@ def stub_strapi_email_template(key, email_template: Cms::Mocks::EmailTemplate.ge
     stub_request(:get, /^https:\/\/strapi.teachcomputing.org\/api\/email-templates\/#{key}/).to_return_json(body: {data: email_template})
   end
 
+  def stub_strapi_email_template_missing(key)
+    stub_request(:get, /^https:\/\/strapi.teachcomputing.org\/api\/email-templates\/#{key}/).to_return_json(body: not_found_response, status: 404)
+  end
+
   def stub_strapi_programme(key, programme: Cms::Mocks::Programme.generate_raw_data)
     if as_graphql
       stub_strapi_graphql_query("programmes", programme)