Iterate on newsletter sending logic

Author: rossta

app/controllers/admin/newsletters_controller.rb

@@ -1,13 +1,12 @@
 class Admin::NewslettersController < ApplicationController
-  before_action :set_newsletter, only: %i[show edit update destroy deliver]
-
   # GET /admin/newsletters
   def index
     @newsletters = Newsletter.all
   end
 
   # GET /admin/newsletters/1
   def show
+    @newsletter = Newsletter.find(params[:id])
   end
 
   # GET /admin/newsletters/new
@@ -17,6 +16,7 @@ def new
 
   # GET /admin/newsletters/1/edit
   def edit
+    @newsletter = Newsletter.find(params[:id])
   end
 
   # POST /admin/newsletters
@@ -32,6 +32,8 @@ def create
 
   # PATCH/PUT /admin/newsletters/1
   def update
+    @newsletter = Newsletter.find(params[:id])
+
     if @newsletter.update(newsletter_params)
       redirect_to [:admin, @newsletter], notice: "Newsletter was successfully updated.", status: :see_other
     else
@@ -41,32 +43,31 @@ def update
 
   # DELETE /admin/newsletters/1
   def destroy
+    @newsletter = Newsletter.find(params[:id])
+
     @newsletter.destroy!
+
     redirect_to admin_newsletters_path, notice: "Newsletter was successfully destroyed.", status: :see_other
   end
 
   # PATCH /admin/newsletters/1/deliver
   def deliver
     @newsletter = Newsletter.find(params[:id])
+
     recipients = deliver_live? ? User.subscribers : User.test_recipients
     label = deliver_live? ? "LIVE" : "TEST"
 
     if recipients.empty?
-      return redirect_to [:admin, @newsletter], alert: "No recipients found."
+      return redirect_to [:admin, @newsletter], alert: "No recipients fond."
     end
 
-    NewsletterNotifier.deliver_to(recipients, newsletter: @newsletter)
+    NewsletterNotifier.deliver_to(recipients, newsletter: @newsletter, live: deliver_live?)
 
     redirect_to [:admin, @newsletter], notice: "[#{label}] Newsletter was successfully delivered."
   end
 
   private
 
-  # Use callbacks to share common setup or constraints between actions.
-  def set_newsletter
-    @newsletter = Newsletter.find(params[:id])
-  end
-
   # Only allow a list of trusted parameters through.
   def newsletter_params
     params.fetch(:newsletter, {}).permit(:title, :content)

app/notifiers/newsletter_notifier.rb

@@ -1,6 +1,8 @@
 class NewsletterNotifier < NotificationEvent
-  def self.deliver_to(users, newsletter:, **)
-    new(params: {newsletter_id: newsletter.id}).deliver(users, **)
+  Error = Class.new(StandardError)
+
+  def self.deliver_to(users, newsletter:, live: false, **)
+    new(params: {newsletter_id: newsletter.id, live:}).deliver(users, **)
   end
 
   def deliver_notifications_in_bulk
@@ -9,16 +11,18 @@ def deliver_notifications_in_bulk
 
     PostmarkClient.deliver_messages(messages)
 
-    newsletter.touch(:sent_at)
+    newsletter.touch(:sent_at) if deliver_live?
   end
 
   private
 
-  def postmark_client
-    @postmark_client ||= Postmark::ApiClient.new(Rails.configuration.settings.postmark_api_token)
+  def deliver_live?
+    !!params[:live]
   end
 
   def build_newsletter_messages(newsletter)
+    assert_test_recipients!
+
     recipients.find_each.filter_map do |user|
       if !user.confirmed?
         log_skip(newsletter, user, "user not confirmed")
@@ -39,4 +43,14 @@ def build_newsletter_messages(newsletter)
   def log_skip(newsletter, user, reason)
     Rails.logger.info "#[#{self.class}] Skipping delivery of newsletter #{newsletter.id} for: #{user.class.name}##{user.id} — #{reason}"
   end
+
+  def assert_test_recipients!
+    return if deliver_live?
+
+    test_recipients = User.test_recipients.pluck(:email)
+
+    if recipients.any? { |user| test_recipients.exclude?(user.email) }
+      raise Error, "Attempted to deliver test newsletter to non-test recipients:  #{id}"
+    end
+  end
 end

app/views/admin/newsletters/show.html.erb

@@ -1,8 +1,8 @@
 <%= render Pages::Header.new(title: "Admin: #{@newsletter.title}") %>
 <div class="section-content container py-gap">
   <div class="flex gap-x-2">
-    <%= button_to "Send Test", deliver_admin_newsletter_path(@newsletter), class: "button secondary" %>
-    <%= button_to "Send Live", deliver_admin_newsletter_path(@newsletter, live: true), class: "button primary" %>
+    <%= button_to "Send Test", deliver_admin_newsletter_path(@newsletter), method: :patch, class: "button secondary" %>
+    <%= button_to "Send Live", deliver_admin_newsletter_path(@newsletter, live: true), method: :patch, class: "button primary" %>
   </div>
 
   <%= render @newsletter %>

app/views/application/_flash.html.erb

@@ -1,5 +1,5 @@
 <% flash.each do |flash_type, message| %>
-  <div class="flash__message flash_#{flash_type}">
+  <div class="flash__message flash__#{flash_type}">
     <%= message %>
   </div>
 <% end %>

config/routes.rb

@@ -84,7 +84,7 @@
 
     resources :newsletters do
       member do
-        post :deliver
+        patch :deliver
       end
     end
 

spec/factories/newsletters.rb

@@ -4,7 +4,7 @@
     content { Faker::Markdown.sandwich(sentences: 3) }
 
     trait :sent do
-      sent_at { 1.day.ago }
+      sent_at { |n| n.days.ago }
     end
   end
 end

spec/mailers/emails/newsletter_mailer_spec.rb

@@ -13,7 +13,7 @@
     end
 
     it "renders the body" do
-      expect(mail.body.encoded).to match(Markdown::Base.new(newsletter.content).call)
+      expect(mail.body.encoded).to match(newsletter.content.split("\n").first)
     end
   end
 end