Merge pull request #932 from hennevogel/bugfix_email_settings

Fix email notifications

app/controllers/admin/cfps_controller.rb

@@ -14,8 +14,10 @@ module Admin
 
     def create
       @cfp = @program.build_cfp(cfp_params)
+      send_mail_on_cfp_dates_updates = @cfp.notify_on_cfp_date_update?
 
       if @cfp.save
+        ConferenceCfpUpdateMailJob.perform_later(@conference) if send_mail_on_cfp_dates_updates
         redirect_to admin_conference_program_cfp_path,
                     notice: 'Call for papers successfully created.'
       else
@@ -31,7 +33,7 @@ module Admin
       send_mail_on_cfp_dates_updates = @cfp.notify_on_cfp_date_update?
 
       if @cfp.update_attributes(cfp_params)
-        Mailbot.delay.send_on_cfps_dates_updates(@conference) if send_mail_on_cfp_dates_updates
+        ConferenceCfpUpdateMailJob.perform_later(@conference) if send_mail_on_cfp_dates_updates
         redirect_to admin_conference_program_cfp_path(@conference.short_title),
                     notice: 'Call for papers successfully updated.'
       else

app/controllers/admin/conference_controller.rb

@@ -85,7 +85,7 @@ module Admin
       send_mail_on_conf_update = @conference.notify_on_dates_changed?
 
       if @conference.update_attributes(conference_params)
-        Mailbot.delay.conference_date_update_mail(@conference) if send_mail_on_conf_update
+        ConferenceDateUpdateMailJob.perform_later(@conference) if send_mail_on_conf_update
         redirect_to edit_admin_conference_path(id: @conference.short_title),
                     notice: 'Conference was successfully updated.'
       else

app/controllers/admin/emails_controller.rb

@@ -4,10 +4,15 @@ module Admin
     load_and_authorize_resource class: EmailSettings
 
     def update
-      @conference.email_settings.update_attributes(email_params)
+      if @conference.email_settings.update(email_params)
         redirect_to admin_conference_emails_path(
                     @conference.short_title),
-                  notice: 'Settings have been successfully updated.'
+                    notice: 'Email settings have been successfully updated.'
+      else
+        redirect_to admin_conference_emails_path(
+                    @conference.short_title),
+                    error: "Updating email settings failed. #{@conference.email_settings.errors.to_a.join('. ')}."
+      end
     end
 
     def index
@@ -24,8 +29,8 @@ module Admin
                                              :send_on_conference_dates_updated, :conference_dates_updated_subject, :conference_dates_updated_body,
                                              :send_on_conference_registration_dates_updated, :conference_registration_dates_updated_subject, :conference_registration_dates_updated_body,
                                              :send_on_venue_updated, :venue_updated_subject, :venue_updated_body,
-                                             :send_on_call_for_papers_dates_updated, :call_for_papers_dates_updated_subject, :call_for_papers_dates_updated_body,
+                                             :send_on_cfp_dates_updated, :cfp_dates_updated_subject, :cfp_dates_updated_body,
-                                             :send_on_call_for_papers_schedule_public, :call_for_papers_schedule_public_subject, :call_for_papers_schedule_public_body)
+                                             :send_on_program_schedule_public, :program_schedule_public_subject, :program_schedule_public_body)
     end
   end
 end

app/controllers/admin/programs_controller.rb

@@ -11,10 +11,10 @@ module Admin
       authorize! :update, @conference.program
       @program = @conference.program
       @program.assign_attributes(program_params)
-#       send_mail_on_schedule_public = @program.notify_on_schedule_public?
+      send_mail_on_schedule_public = @program.notify_on_schedule_public?
 
       if @program.update_attributes(program_params)
-#         Mailbot.delay.send_on_schedule_public(@conference) if send_mail_on_schedule_public
+        ConferenceScheduleUpdateMailJob.perform_later(@conference) if send_mail_on_schedule_public
         redirect_to admin_conference_program_path(@conference.short_title),
                     notice: 'The program was successfully updated.'
       else

app/controllers/admin/registration_periods_controller.rb

@@ -12,7 +12,7 @@ module Admin
       send_mail_on_reg_update = @conference.notify_on_registration_dates_changed?
 
       if @registration_period.save
-        Mailbot.delay.conference_registration_date_update_mail(@conference) if send_mail_on_reg_update
+        ConferenceRegistrationDateUpdateMailJob.perform_later(@conference) if send_mail_on_reg_update
         redirect_to admin_conference_registration_period_path(@conference.short_title),
                     notice: 'Registration Period successfully updated.'
       else
@@ -32,7 +32,7 @@ module Admin
       send_mail_on_reg_update = @conference.notify_on_registration_dates_changed?
 
       if @registration_period.update(registration_period_params)
-        Mailbot.delay.conference_registration_date_update_mail(@conference) if send_mail_on_reg_update
+        ConferenceRegistrationDateUpdateMailJob.perform_later(@conference) if send_mail_on_reg_update
         redirect_to admin_conference_registration_period_path(@conference.short_title),
                     notice: 'Registration Period successfully updated.'
       else

app/jobs/conference_cfp_update_mail_job.rb

@@ -0,0 +1,9 @@
+class ConferenceCfpUpdateMailJob < ActiveJob::Base
+  queue_as :default
+
+  def perform(conference)
+    conference.subscriptions.each do |subscription|
+      Mailbot.conference_cfp_update_mail(conference, subscription.user).deliver_now
+    end
+  end
+end

app/jobs/conference_date_update_mail_job.rb

@@ -0,0 +1,9 @@
+class ConferenceDateUpdateMailJob < ActiveJob::Base
+  queue_as :default
+
+  def perform(conference)
+    conference.subscriptions.each do |subscription|
+      Mailbot.conference_date_update_mail(conference, subscription.user).deliver_now
+    end
+  end
+end

app/jobs/conference_registration_date_update_mail_job.rb

@@ -0,0 +1,9 @@
+class ConferenceRegistrationDateUpdateMailJob < ActiveJob::Base
+  queue_as :default
+
+  def perform(conference)
+    conference.subscriptions.each do |subscription|
+      Mailbot.conference_registration_date_update_mail(conference, subscription.user).deliver_now
+    end
+  end
+end

app/jobs/conference_schedule_update_mail_job.rb

@@ -0,0 +1,9 @@
+class ConferenceScheduleUpdateMailJob < ActiveJob::Base
+  queue_as :default
+
+  def perform(conference)
+    conference.subscriptions.each do |subscription|
+      Mailbot.conference_schedule_update_mail(conference, subscription.user).deliver_now
+    end
+  end
+end

app/jobs/conference_venue_update_mail_job.rb

@@ -0,0 +1,9 @@
+class ConferenceVenueUpdateMailJob < ActiveJob::Base
+  queue_as :default
+
+  def perform(conference)
+    conference.subscriptions.each do |subscription|
+      Mailbot.conference_venue_update_mail(conference, subscription.user).deliver_now
+    end
+  end
+end

app/jobs/event_comment_mail_job.rb

@@ -0,0 +1,11 @@
+class EventCommentMailJob < ActiveJob::Base
+  queue_as :default
+
+  def perform(comment)
+    conference = comment.commentable.program.conference
+
+    User.comment_notifiable(conference).each do |user|
+      Mailbot.event_comment_mail(comment, user).deliver_now
+    end
+  end
+end

app/mailers/mailbot.rb

@@ -1,106 +1,95 @@
 class Mailbot < ActionMailer::Base
-  default from: 'no-reply@example.com'
+  def registration_mail(conference, user)
-
+    mail(to: user.email,
-  def registration_mail(conference, person)
+         from: conference.contact.email,
-    build_email(conference,
+         subject: conference.email_settings.registration_subject,
-                person.email,
+         body: conference.email_settings.generate_email_on_conf_updates(conference,
-                conference.email_settings.registration_subject,
+                                                                        user,
-                conference.email_settings.generate_email_on_conf_updates(conference, person, conference.email_settings.registration_body))
+                                                                        conference.email_settings.registration_body))
   end
 
   def acceptance_mail(event)
     conference = event.program.conference
-    person = event.submitter
+
-    build_email(conference,
+    mail(to: event.submitter.email,
-                person.email,
+         from: conference.contact.email,
-                conference.email_settings.accepted_subject,
+         subject: conference.email_settings.accepted_subject,
-                conference.email_settings.generate_event_mail(event, conference.email_settings.accepted_body))
+         body: conference.email_settings.generate_event_mail(event, conference.email_settings.accepted_body))
   end
 
   def rejection_mail(event)
     conference = event.program.conference
-    person = event.submitter
+
-    build_email(conference,
+    mail(to: event.submitter.email,
-                person.email,
+         from: conference.contact.email,
-                conference.email_settings.rejected_subject,
+         subject: conference.email_settings.rejected_subject,
-                conference.email_settings.generate_event_mail(event, conference.email_settings.rejected_body))
+         body: conference.email_settings.generate_event_mail(event, conference.email_settings.rejected_body))
   end
 
   def confirm_reminder_mail(event)
     conference = event.program.conference
-    person = event.submitter
+
-    build_email(conference,
+    mail(to: event.submitter.email,
-                person.email,
+         from: conference.contact.email,
-                conference.email_settings.confirmed_without_registration_subject,
+         subject: conference.email_settings.confirmed_without_registration_subject,
-                conference.email_settings.generate_event_mail(event, conference.email_settings.confirmed_without_registration_body))
+         body: conference.email_settings.generate_event_mail(event,
+                                                             conference.email_settings.confirmed_without_registration_body))
   end
 
-  def conference_date_update_mail(conference)
+  def conference_date_update_mail(conference, user)
-    User.joins(:subscriptions).merge(conference.subscriptions).each do |user|
+    mail(to: user.email,
-      build_email(conference,
+         from: conference.contact.email,
-                  user.email,
+         subject: conference.email_settings.conference_dates_updated_subject,
-                  conference.email_settings.conference_dates_updated_subject,
+         body: conference.email_settings.generate_email_on_conf_updates(conference,
-                  conference.email_settings.generate_email_on_conf_updates(conference, user, conference.email_settings.conference_dates_updated_body))
+                                                                        user,
-    end
+                                                                        conference.email_settings.conference_dates_updated_body))
   end
 
-  def conference_registration_date_update_mail(conference)
+  def conference_registration_date_update_mail(conference, user)
-    User.joins(:subscriptions).merge(conference.subscriptions).uniq.joins('INNER JOIN registrations ON registrations.user_id != users.id').merge(conference.registrations).each do |user|
+    mail(to: user.email,
-      build_email(conference,
+         from: conference.contact.email,
-                  user.email,
+         subject: conference.email_settings.conference_registration_dates_updated_subject,
-                  conference.email_settings.conference_registration_dates_updated_subject,
+         body: conference.email_settings.generate_email_on_conf_updates(conference,
-                  conference.email_settings.generate_email_on_conf_updates(conference, user, conference.email_settings.conference_registration_dates_updated_body))
+                                                                        user,
-    end
+                                                                        conference.email_settings.conference_registration_dates_updated_body))
   end
 
-  def send_email_on_venue_updated(conference)
+  def conference_venue_update_mail(conference, user)
-    User.joins(:subscriptions).merge(conference.subscriptions).each do |user|
+    mail(to: user.email,
-      build_email(conference,
+         from: conference.contact.email,
-                  user.email,
+         subject: conference.email_settings.venue_updated_subject,
-                  conference.email_settings.venue_updated_subject,
+         body: conference.email_settings.generate_email_on_conf_updates(conference,
-                  conference.email_settings.generate_email_on_conf_updates(conference, user, conference.email_settings.venue_updated_body))
+                                                                        user,
-    end
+                                                                        conference.email_settings.venue_updated_body))
   end
 
-  def send_on_schedule_public(conference)
+  def conference_schedule_update_mail(conference, user)
-    User.joins(:subscriptions).merge(conference.subscriptions).each do |user|
+    mail(to: user.email,
-      build_email(conference,
+         from: conference.contact.email,
-                  user.email,
+         subject: conference.email_settings.program_schedule_public_subject,
-                  conference.email_settings.program_schedule_public_subject,
+         body: conference.email_settings.generate_email_on_conf_updates(conference,
-                  conference.email_settings.generate_email_on_conf_updates(conference, user, conference.email_settings.program_schedule_public_body))
+                                                                        user,
-    end
+                                                                        conference.email_settings.program_schedule_public_body))
   end
 
-  def send_on_cfp_dates_updates(conference)
+  def conference_cfp_update_mail(conference, user)
-    User.joins(:subscriptions).merge(conference.subscriptions).each do |user|
+    mail(to: user.email,
-      build_email(conference,
+         from: conference.contact.email,
-                  user.email,
+         subject: conference.email_settings.cfp_dates_updated_subject,
-                  conference.email_settings.cfp_dates_updated_subject,
+         body: conference.email_settings.generate_email_on_conf_updates(conference,
-                  conference.email_settings.generate_email_on_conf_updates(conference, user, conference.email_settings.cfp_dates_updated_body))
+                                                                        user,
-    end
+                                                                        conference.email_settings.cfp_dates_updated_body))
   end
 
-  def send_notification_email_for_comment(comment)
+  def event_comment_mail(comment, user)
     @comment = comment
     @event = @comment.commentable
     @conference = @event.program.conference
-    recipients = User.comment_notifiable(@conference) # with scope
-    recipients.each do |user|
     @user = user
+
     mail(to: @user.email,
          from: @conference.contact.email,
-           reply_to: @conference.contact.email,
-           template_path: 'admin/emails',
          template_name: 'comment_template',
          subject: "New comment has been posted for #{@event.title}")
   end
-  end
-
-  def build_email(conference, to, subject, body)
-    mail(to: to,
-         from: conference.contact.email,
-         reply_to: conference.contact.email,
-         subject: subject,
-         body: body)
-  end
 end

app/models/comment.rb

@@ -56,6 +56,6 @@ class Comment < ActiveRecord::Base
   private
 
   def send_notification
-    Mailbot.delay.send_notification_email_for_comment(self)
+    EventCommentMailJob.perform_later(self)
   end
 end

app/models/conference.rb

@@ -514,10 +514,11 @@ class Conference < ActiveRecord::Base
   # * +True+ -> If conference is updated and all other parameters are set
   # * +False+ -> Either conference is not updated or one or more parameter is not set
   def notify_on_dates_changed?
-    (self.start_date_changed? || self.end_date_changed?) &&
+    return false unless self.email_settings.send_on_conference_dates_updated
-    self.email_settings.send_on_conference_dates_updated &&
+    # do not notify unless one of the dates changed
-    !self.email_settings.conference_dates_updated_subject.blank? &&
+    return false unless self.start_date_changed? || self.end_date_changed?
-    self.email_settings.conference_dates_updated_body
+    # do not notify unless the mail content is set up
+    (!email_settings.conference_dates_updated_subject.blank? && !email_settings.conference_dates_updated_body.blank?)
   end
 
   ##
@@ -527,11 +528,13 @@ class Conference < ActiveRecord::Base
   # * +True+ -> If registration dates is updated and all other parameters are set
   # * +False+ -> Either registration date is not updated or one or more parameter is not set
   def notify_on_registration_dates_changed?
-    registration_period &&
+    return false unless self.email_settings.send_on_conference_registration_dates_updated
-    (registration_period.start_date_changed? || registration_period.end_date_changed?) &&
+    # do not notify unless we allow a registration
-    email_settings.send_on_conference_registration_dates_updated &&
+    return false unless self.registration_period
-    !email_settings.conference_registration_dates_updated_subject.blank? &&
+    # do not notify unless one of the dates changed
-    email_settings.conference_registration_dates_updated_body
+    return false unless registration_period.start_date_changed? || registration_period.end_date_changed?
+    # do not notify unless the mail content is set up
+    (!email_settings.conference_registration_dates_updated_subject.blank? && !email_settings.conference_registration_dates_updated_body.blank?)
   end
 
   def registration_limit_exceeded?

app/models/event.rb

@@ -112,7 +112,7 @@ class Event < ActiveRecord::Base
         program.conference.email_settings.confirmed_without_registration_body  &&
         program.conference.email_settings.confirmed_without_registration_subject
       if program.conference.registrations.where(user_id: submitter.id).first.nil?
-        Mailbot.delay.confirm_reminder_mail(self)
+        Mailbot.confirm_reminder_mail(self).deliver_later
       end
     end
   end
@@ -122,8 +122,7 @@ class Event < ActiveRecord::Base
         program.conference.email_settings.accepted_body &&
         program.conference.email_settings.accepted_subject &&
         !options[:send_mail].blank?
-      Rails.logger.debug 'Sending event acceptance mail'
+      Mailbot.acceptance_mail(self).deliver_later
-      Mailbot.delay.acceptance_mail(self)
     end
   end
 
@@ -132,8 +131,7 @@ class Event < ActiveRecord::Base
         program.conference.email_settings.rejected_body &&
         program.conference.email_settings.rejected_subject &&
         !options[:send_mail].blank?
-      Rails.logger.debug 'Sending rejected mail'
+      Mailbot.rejection_mail(self).deliver_later
-      Mailbot.delay.rejection_mail(self)
     end
   end
 

app/models/program.rb

@@ -64,6 +64,14 @@ class Program < ActiveRecord::Base
     cfp.present? && (cfp.start_date..cfp.end_date).cover?(Date.current)
   end
 
+  def notify_on_schedule_public?
+    return false unless conference.email_settings.send_on_program_schedule_public
+    # do not notify if the schedule is not public
+    return false unless schedule_public
+    # do not notify unless the mail content is set up
+    (!conference.email_settings.program_schedule_public_subject.blank? && !conference.email_settings.program_schedule_public_body.blank?)
+  end
+
   private
 
   ##

app/models/registration.rb

@@ -45,7 +45,7 @@ class Registration < ActiveRecord::Base
 
   def send_registration_mail
     if conference.email_settings.send_on_registration?
-      Mailbot.delay.registration_mail(conference, user)
+      Mailbot.registration_mail(conference, user).deliver_later
     end
   end
 

app/models/venue.rb

@@ -14,7 +14,7 @@ class Venue < ActiveRecord::Base
                                     content_type: [/jpg/, /jpeg/, /png/, /gif/],
                                     size: { in: 0..500.kilobytes }
 
-  after_update :send_mail_notification
+  before_save :send_mail_notification
 
   def address
     "#{street}, #{city}, #{country_name}"
@@ -32,15 +32,15 @@ class Venue < ActiveRecord::Base
   private
 
   def send_mail_notification
-    Mailbot.delay.send_email_on_venue_updated(conference) if venue_notify?(conference)
+    ConferenceVenueUpdateMailJob.perform_later(conference) if notify_on_venue_changed?
   end
 
-  def venue_notify?(conference)
+  def notify_on_venue_changed?
-    (self.name_changed? || self.street_changed?) &&
+    return false unless conference.email_settings.send_on_venue_updated
-    (!self.name.blank? && !self.street.blank?) &&
+    # do not notify unless the address changed
-    (conference.email_settings.send_on_venue_updated &&
+    return false unless self.name_changed? || self.street_changed? || self.city_changed? || self.country_changed?
-    !conference.email_settings.venue_updated_subject.blank? &&
+    # do not notify unless the mail content is set up
-    conference.email_settings.venue_updated_body)
+    (!conference.email_settings.venue_updated_subject.blank? && !conference.email_settings.venue_updated_body.blank?)
   end
 
   # TODO: create a module to be mixed into model to perform same operation

config/application.rb

@@ -64,5 +64,6 @@ module Osem
     # Errors raised within `after_rollback`/`after_commit` propagate normally
     # like in other Active Record callbacks.
     config.active_record.raise_in_transactional_callbacks = true
+    config.active_job.queue_adapter = :delayed_job
   end
 end

spec/features/email_spec.rb

@@ -54,7 +54,7 @@ feature EmailSettings do
       click_button 'Update Email settings'
 
       expect(flash).
-          to eq('Settings have been successfully updated.')
+          to eq('Email settings have been successfully updated.')
 
       expect(find('#email_settings_registration_subject').
                  value).to eq('Registration subject')

spec/mailers/mailbot_spec.rb

@@ -19,17 +19,20 @@ describe Mailbot do
       it 'assigns the email receiver, sender, reply_to' do
         expect(mail.to).to eq ['user@example.com']
         expect(mail.from).to eq ['conf@domain.com']
-        expect(mail.reply_to).to eq ['conf@domain.com']
       end
 
       it 'assigns the email body' do
         expect(mail.body).to eq 'Lorem ipsum dolor sit amet, consectetuer adipiscing elit'
       end
+
+      it 'delivers the email' do
+        expect(ActionMailer::Base.deliveries).to include(mail)
+      end
     end
 
     describe '.registration_mail' do
       include_examples 'mailer actions' do
-        let(:mail) { Mailbot.registration_mail conference, user }
+        let(:mail) { Mailbot.registration_mail(conference, user).deliver_now }
       end
     end
 
@@ -41,7 +44,7 @@ describe Mailbot do
       end
 
       include_examples 'mailer actions' do
-        let(:mail) { Mailbot.acceptance_mail event }
+        let(:mail) { Mailbot.acceptance_mail(event).deliver_now }
       end
     end
 
@@ -53,7 +56,7 @@ describe Mailbot do
       end
 
       include_examples 'mailer actions' do
-        let(:mail) { Mailbot.rejection_mail event }
+        let(:mail) { Mailbot.rejection_mail(event).deliver_now }
       end
     end
 
@@ -65,13 +68,7 @@ describe Mailbot do
       end
 
       include_examples 'mailer actions' do
-        let(:mail) { Mailbot.confirm_reminder_mail event }
+        let(:mail) { Mailbot.confirm_reminder_mail(event).deliver_now }
-      end
-    end
-
-    describe '.build_email' do
-      include_examples 'mailer actions' do
-        let(:mail) { Mailbot.build_email conference, 'user@example.com', 'Lorem Ipsum Dolsum', 'Lorem ipsum dolor sit amet, consectetuer adipiscing elit' }
       end
     end
   end