Merge pull request #370 from alphagov/add-petition-emails

Add petition emails

app/assets/stylesheets/petitions/admin/views/_shared.scss

@@ -20,6 +20,10 @@
     margin-bottom: $gutter-half;
   }
 
+  .character-count {
+    margin-bottom: 0;
+  }
+
   // Pagination
   .pagination {
     margin: $gutter-half 0;

app/controllers/admin/debate_outcomes_controller.rb

@@ -9,7 +9,7 @@ def show
 
   def update
     if @debate_outcome.update(debate_outcome_params)
-      EmailDebateOutcomesJob.run_later_tonight(@petition)
+      EmailDebateOutcomesJob.run_later_tonight(petition: @petition)
       redirect_to [:admin, @petition], notice: 'Email will be sent overnight'
     else
       render 'admin/petitions/show'

app/controllers/admin/government_response_controller.rb

@@ -9,7 +9,7 @@ def show
 
   def update
     if @government_response.update_attributes(government_response_params)
-      EmailThresholdResponseJob.run_later_tonight(@petition)
+      EmailThresholdResponseJob.run_later_tonight(petition: @petition)
       redirect_to [:admin, @petition], notice: 'Email will be sent overnight'
     else
       render 'admin/petitions/show'

app/controllers/admin/petition_emails_controller.rb

@@ -0,0 +1,38 @@
+class Admin::PetitionEmailsController < Admin::AdminController
+  respond_to :html
+  before_action :fetch_petition
+  before_action :build_email
+
+  def new
+    render 'admin/petitions/show'
+  end
+
+  def create
+    if @email.update(email_params)
+      EmailPetitionersJob.run_later_tonight(petition: @petition, email: @email)
+      PetitionMailer.email_signer(@petition, feedback_signature, @email).deliver_now
+
+      redirect_to [:admin, @petition], notice: 'Email will be sent overnight'
+    else
+      render 'admin/petitions/show'
+    end
+  end
+
+  private
+
+  def fetch_petition
+    @petition = Petition.moderated.find(params[:petition_id])
+  end
+
+  def build_email
+    @email = @petition.emails.build(sent_by: current_user.pretty_name)
+  end
+
+  def email_params
+    params.require(:petition_email).permit(:subject, :body)
+  end
+
+  def feedback_signature
+    FeedbackSignature.new(@petition)
+  end
+end

app/controllers/admin/petitions_controller.rb

@@ -20,7 +20,7 @@ def edit_scheduled_debate_date
   def update_scheduled_debate_date
     fetch_petition_for_scheduled_debate_date
     if @petition.update(update_scheduled_debate_date_params)
-      EmailDebateScheduledJob.run_later_tonight(@petition)
+      EmailDebateScheduledJob.run_later_tonight(petition: @petition)
       redirect_to admin_petition_url(@petition), notice: "Email will be sent overnight"
     else
       render :edit_scheduled_debate_date

app/controllers/admin/schedule_debate_controller.rb

@@ -8,7 +8,7 @@ def show
 
   def update
     if @petition.update_attributes(params_for_update)
-      EmailDebateScheduledJob.run_later_tonight(@petition)
+      EmailDebateScheduledJob.run_later_tonight(petition: @petition)
       redirect_to [:admin, @petition], notice: "Email will be sent overnight"
     else
       render 'admin/petitions/show'

app/jobs/concerns/email_all_petition_signatories.rb

@@ -1,75 +1,83 @@
 module EmailAllPetitionSignatories
-  extend ActiveSupport::Concern
+  # Concern to add shared functionality to ActiveJob classes
+  # that are responsible for enqueuing send email jobs
 
-  #
-  # Concern to add shared functionality to ActiveJob classes that are responsible
-  # for enqueuing send email jobs
-  #
+  extend ActiveSupport::Concern
 
   included do
-    queue_as :default
+    class_attribute :email_delivery_job_class
+    class_attribute :timestamp_name
 
-    def self.run_later_tonight(petition)
-      requested_at = Time.current
+    attr_reader :petition, :requested_at
 
-      petition.set_email_requested_at_for(new.timestamp_name, to: requested_at)
+    queue_as :default
+  end
+
+  module ClassMethods
+    def run_later_tonight(**args)
+      petition, @requested_at = args[:petition], nil
 
-      set(wait_until: later_tonight).
-        perform_later(petition, requested_at.getutc.iso8601(6))
+      petition.set_email_requested_at_for(timestamp_name, to: requested_at)
+      set(wait_until: later_tonight).perform_later(**args.merge(requested_at: requested_at_iso8601))
     end
 
-    def self.later_tonight
-      1.day.from_now.at_midnight + rand(240).minutes + rand(60).seconds
+    private
+
+    def requested_at
+      @requested_at ||= Time.current
     end
-    private_class_method :later_tonight
 
-  end
+    def requested_at_iso8601
+      requested_at.getutc.iso8601(6)
+    end
 
+    def later_tonight
+      midnight + random_interval
+    end
 
+    def midnight
+      requested_at.end_of_day
+    end
 
-  def perform(petition, requested_at_string)
-    @petition = petition
-    @requested_at = requested_at_string.in_time_zone
-    do_work!
+    def random_interval
+      rand(240).minutes + rand(60).seconds
+    end
   end
 
-  def timestamp_name
-    raise NotImplementedError.new "Including classes must implement #timestamp_name method"
+  def perform(**args)
+    @petition, @requested_at = args[:petition], args[:requested_at]
+
+    # If the petition has been updated since the job
+    # was queued then don't send the emails.
+    unless petition_has_been_updated?
+      enqueue_send_email_jobs
+    end
   end
 
   private
 
-  attr_reader :petition, :requested_at
-
-  def do_work!
-    return if petition_has_been_updated?
-
-    logger.info("Starting #{self.class.name} for petition '#{petition.action}' with email requested at: #{petition_timestamp}")
-    enqueue_send_email_jobs
-    logger.info("Finished #{self.class.name} for petition '#{petition.action}'")
-
-  end
-
-  #
   # Batches the signataries to send emails to in groups of 1000
   # and enqueues a job to do the actual sending
-  #
   def enqueue_send_email_jobs
     signatures_to_email.find_each do |signature|
-      email_delivery_job_class.perform_later(
-        signature:                    signature,
-        timestamp_name:               timestamp_name,
-        petition:                     petition,
-        requested_at_as_string:       requested_at.getutc.iso8601(6)
-      )
+      email_delivery_job_class.perform_later(**mailer_arguments(signature))
     end
   end
 
+  def mailer_arguments(signature)
+    {
+      signature:      signature,
+      timestamp_name: timestamp_name,
+      petition:       petition,
+      requested_at:   requested_at
+    }
+  end
+
   # admins can ask to send the email multiple times and each time they
   # ask we enqueues a new job to send out emails with a new timestamp
   # we want to execute only the latest job enqueued
   def petition_has_been_updated?
-    (petition_timestamp - requested_at).abs > 1
+    (petition_timestamp - requested_at.in_time_zone).abs > 1
   end
 
   def petition_timestamp
@@ -79,11 +87,4 @@ def petition_timestamp
   def signatures_to_email
     petition.signatures_to_email_for(timestamp_name)
   end
-
-  # The job class that handles the actual email sending for this job type
-  def email_delivery_job_class
-    raise NotImplementedError.new "Including classes must implement #email_delivery_job_class method"
-  end
 end
-
-

app/jobs/concerns/email_delivery.rb

@@ -1,25 +1,44 @@
 module EmailDelivery
-  extend ActiveSupport::Concern
-
-  #
   # Send a single email to a recipient informing them about a petition that they have signed
   # Implemented as a custom job rather than using action mailers #deliver_later so we can do
   # extra checking before sending the email
-  #
+
+  extend ActiveSupport::Concern
+
+  PERMANENT_FAILURES = [
+    Net::SMTPFatalError,
+    Net::SMTPSyntaxError
+  ]
+
+  TEMPORARY_FAILURES = [
+    Net::SMTPAuthenticationError,
+    Net::OpenTimeout,
+    Net::SMTPServerBusy,
+    Errno::ECONNRESET,
+    Errno::ECONNREFUSED,
+    Errno::ETIMEDOUT,
+    Timeout::Error
+  ]
 
   included do
+    attr_reader :signature, :timestamp_name, :petition, :requested_at
     queue_as :default
-  end
 
-  def perform(signature:, timestamp_name:, petition:,
-    requested_at_as_string:, mailer: PetitionMailer.name, logger: nil)
+    rescue_from *PERMANENT_FAILURES do |exception|
+      log_exception(exception)
+    end
+
+    rescue_from *TEMPORARY_FAILURES do |exception|
+      log_exception(exception)
+      retry_job
+    end
+  end
 
-    @mailer = mailer.constantize
-    @signature = signature
-    @petition = petition
-    @requested_at = requested_at_as_string.in_time_zone
-    @timestamp_name = timestamp_name
-    @logger = logger
+  def perform(**args)
+    @signature = args[:signature]
+    @petition = args[:petition]
+    @requested_at = args[:requested_at].in_time_zone
+    @timestamp_name = args[:timestamp_name]
 
     if can_send_email?
       send_email
@@ -29,29 +48,24 @@ def perform(signature:, timestamp_name:, petition:,
 
   private
 
-  attr_reader :mailer, :signature, :timestamp_name, :petition, :requested_at
+  def log_exception(exception)
+    logger.info(log_message(exception))
+  end
+
+  def log_message(exception)
+    "#{exception.class.name} while sending email for #{self.class.name} to: #{signature.email} for #{petition.action}"
+  end
 
   def can_send_email?
     petition_has_not_been_updated? && email_not_previously_sent?
   end
 
   def send_email
     create_email.deliver_now
+  end
 
-    rescue Errno::ECONNREFUSED, Errno::ETIMEDOUT, Net::SMTPError, Timeout::Error => e
-      # log that the send failed
-      logger.info("#{e.class.name} while sending email for #{self.class.name} to: #{signature.email} for #{signature.petition.action}")
-
-      #
-      # TODO: check the error and if it is a n AWS SES rate error:
-      # 454 Throttling failure: Maximum sending rate exceeded
-      # 454 Throttling failure: Daily message quota exceeded
-      #
-      # Then reschedule the send for a day later rather than keep failing
-      #
-
-      # reraise to rerun the job later via the job retry mechanism
-      raise e
+  def mailer
+    PetitionMailer
   end
 
   def create_email
@@ -69,13 +83,11 @@ def petition_timestamp
   # We do not want to send the email if the petition has been updated
   # As email sending is enqueued straight after a petition has been updated
   def petition_has_not_been_updated?
-    (petition_timestamp - requested_at).abs < 1
+    (petition_timestamp - requested_at.in_time_zone).abs < 1
   end
 
-  #
   # Have we already sent an email for this petition version?
   # If we have then the timestamp for the signature will match the timestamp for the petition
-  #
   def email_not_previously_sent?
     # check that the signature is still in the list of signatures
     petition.signatures_to_email_for(timestamp_name).where(id: signature.id).exists?

app/jobs/deliver_debate_outcome_email_job.rb

@@ -4,5 +4,4 @@ class DeliverDebateOutcomeEmailJob < ActiveJob::Base
   def create_email
     mailer.notify_signer_of_debate_outcome signature.petition, signature
   end
-
 end

app/jobs/deliver_debate_scheduled_email_job.rb

@@ -4,5 +4,4 @@ class DeliverDebateScheduledEmailJob < ActiveJob::Base
   def create_email
     mailer.notify_signer_of_debate_scheduled signature.petition, signature
   end
-
 end

app/jobs/deliver_petition_email_job.rb

@@ -0,0 +1,14 @@
+class DeliverPetitionEmailJob < ActiveJob::Base
+  include EmailDelivery
+
+  attr_reader :email
+
+  def perform(**args)
+    @email = args[:email]
+    super
+  end
+
+  def create_email
+    mailer.email_signer petition, signature, email
+  end
+end

app/jobs/deliver_threshold_response_email_job.rb

@@ -4,5 +4,4 @@ class DeliverThresholdResponseEmailJob < ActiveJob::Base
   def create_email
     mailer.notify_signer_of_threshold_response signature.petition, signature
   end
-
 end

app/jobs/email_debate_outcomes_job.rb

@@ -1,11 +1,6 @@
 class EmailDebateOutcomesJob < ActiveJob::Base
   include EmailAllPetitionSignatories
 
-  def email_delivery_job_class
-    DeliverDebateOutcomeEmailJob
-  end
-
-  def timestamp_name
-    'debate_outcome'
-  end
+  self.email_delivery_job_class = DeliverDebateOutcomeEmailJob
+  self.timestamp_name = 'debate_outcome'
 end

app/jobs/email_debate_scheduled_job.rb

@@ -1,11 +1,6 @@
 class EmailDebateScheduledJob < ActiveJob::Base
   include EmailAllPetitionSignatories
 
-  def email_delivery_job_class
-    DeliverDebateScheduledEmailJob
-  end
-
-  def timestamp_name
-    'debate_scheduled'
-  end
+  self.email_delivery_job_class = DeliverDebateScheduledEmailJob
+  self.timestamp_name = 'debate_scheduled'
 end