From 14011f8828ee8869b0ff451d5f5607b36e09a5d3 Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Thu, 31 Mar 2016 16:51:16 +0200 Subject: [PATCH 01/11] Fix email notifications --- app/controllers/admin/cfps_controller.rb | 2 +- app/controllers/admin/emails_controller.rb | 17 ++++++++++------ app/controllers/admin/programs_controller.rb | 4 ++-- app/mailers/mailbot.rb | 1 + app/models/conference.rb | 21 +++++++++++--------- app/models/event.rb | 2 -- app/models/program.rb | 8 ++++++++ app/models/venue.rb | 14 ++++++------- spec/features/email_spec.rb | 2 +- 9 files changed, 43 insertions(+), 28 deletions(-) diff --git a/app/controllers/admin/cfps_controller.rb b/app/controllers/admin/cfps_controller.rb index fadf16fa..5d660d16 100644 --- a/app/controllers/admin/cfps_controller.rb +++ b/app/controllers/admin/cfps_controller.rb @@ -31,7 +31,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 + Mailbot.delay.send_on_cfp_dates_updates(@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 diff --git a/app/controllers/admin/emails_controller.rb b/app/controllers/admin/emails_controller.rb index 203a74e0..adff714a 100644 --- a/app/controllers/admin/emails_controller.rb +++ b/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) - redirect_to admin_conference_emails_path( - @conference.short_title), - notice: 'Settings have been successfully updated.' + if @conference.email_settings.update(email_params) + redirect_to admin_conference_emails_path( + @conference.short_title), + 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_call_for_papers_schedule_public, :call_for_papers_schedule_public_subject, :call_for_papers_schedule_public_body) + :send_on_cfp_dates_updated, :cfp_dates_updated_subject, :cfp_dates_updated_body, + :send_on_program_schedule_public, :program_schedule_public_subject, :program_schedule_public_body) end end end diff --git a/app/controllers/admin/programs_controller.rb b/app/controllers/admin/programs_controller.rb index 0e3e5e2c..c0d20644 100644 --- a/app/controllers/admin/programs_controller.rb +++ b/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 + Mailbot.delay.send_on_schedule_public(@conference) if send_mail_on_schedule_public redirect_to admin_conference_program_path(@conference.short_title), notice: 'The program was successfully updated.' else diff --git a/app/mailers/mailbot.rb b/app/mailers/mailbot.rb index 5cc09053..7e5d23e8 100644 --- a/app/mailers/mailbot.rb +++ b/app/mailers/mailbot.rb @@ -97,6 +97,7 @@ class Mailbot < ActionMailer::Base end def build_email(conference, to, subject, body) + logger.debug "Sending mail about #{subject} to #{to}" mail(to: to, from: conference.contact.email, reply_to: conference.contact.email, diff --git a/app/models/conference.rb b/app/models/conference.rb index e43ef9ae..0a2ff5d6 100644 --- a/app/models/conference.rb +++ b/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?) && - self.email_settings.send_on_conference_dates_updated && - !self.email_settings.conference_dates_updated_subject.blank? && - self.email_settings.conference_dates_updated_body + return false unless self.email_settings.send_on_conference_dates_updated + # do not notify unless one of the dates changed + return false unless self.start_date_changed? || self.end_date_changed? + # 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 && - (registration_period.start_date_changed? || registration_period.end_date_changed?) && - email_settings.send_on_conference_registration_dates_updated && - !email_settings.conference_registration_dates_updated_subject.blank? && - email_settings.conference_registration_dates_updated_body + return false unless self.email_settings.send_on_conference_registration_dates_updated + # do not notify unless we allow a registration + return false unless self.registration_period + # do not notify unless one of the dates changed + 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? diff --git a/app/models/event.rb b/app/models/event.rb index ce77480f..6e42ad99 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -122,7 +122,6 @@ 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.delay.acceptance_mail(self) end end @@ -132,7 +131,6 @@ 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.delay.rejection_mail(self) end end diff --git a/app/models/program.rb b/app/models/program.rb index dbc64b91..cbc27418 100644 --- a/app/models/program.rb +++ b/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 ## diff --git a/app/models/venue.rb b/app/models/venue.rb index 25d3b8cd..2fcbf961 100644 --- a/app/models/venue.rb +++ b/app/models/venue.rb @@ -30,15 +30,15 @@ class Venue < ActiveRecord::Base private def send_mail_notification - Mailbot.delay.send_email_on_venue_updated(conference) if venue_notify?(conference) + Mailbot.delay.send_email_on_venue_updated(conference) if notify_on_venue_changed? end - def venue_notify?(conference) - (self.name_changed? || self.street_changed?) && - (!self.name.blank? && !self.street.blank?) && - (conference.email_settings.send_on_venue_updated && - !conference.email_settings.venue_updated_subject.blank? && - conference.email_settings.venue_updated_body) + def notify_on_venue_changed? + return false unless conference.email_settings.send_on_venue_updated + # do not notify unless the address changed + return false unless self.name_changed? || self.street_changed? || self.city_changed? || self.country_changed? + # do not notify unless the mail content is set up + (!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 diff --git a/spec/features/email_spec.rb b/spec/features/email_spec.rb index 25a67235..cdc6cdfe 100644 --- a/spec/features/email_spec.rb +++ b/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') From 21280cd9d874bd3d8e5ffd1c941733826ebe4e3a Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Wed, 20 Apr 2016 18:59:24 +0200 Subject: [PATCH 02/11] Switch mail sending to active_job Before we were using the delayed_job method of delaying mail sending which isn't working anymore on rails 4. --- app/controllers/admin/cfps_controller.rb | 2 +- app/controllers/admin/conference_controller.rb | 2 +- app/controllers/admin/programs_controller.rb | 2 +- app/controllers/admin/registration_periods_controller.rb | 4 ++-- app/models/comment.rb | 2 +- app/models/event.rb | 6 +++--- app/models/registration.rb | 2 +- app/models/venue.rb | 2 +- config/application.rb | 1 + 9 files changed, 12 insertions(+), 11 deletions(-) diff --git a/app/controllers/admin/cfps_controller.rb b/app/controllers/admin/cfps_controller.rb index 5d660d16..119faf78 100644 --- a/app/controllers/admin/cfps_controller.rb +++ b/app/controllers/admin/cfps_controller.rb @@ -31,7 +31,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_cfp_dates_updates(@conference) if send_mail_on_cfp_dates_updates + Mailbot.send_on_cfp_dates_updates(@conference).deliver_later if send_mail_on_cfp_dates_updates redirect_to admin_conference_program_cfp_path(@conference.short_title), notice: 'Call for papers successfully updated.' else diff --git a/app/controllers/admin/conference_controller.rb b/app/controllers/admin/conference_controller.rb index dd520157..78fd68b4 100644 --- a/app/controllers/admin/conference_controller.rb +++ b/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 + Mailbot.conference_date_update_mail(@conference).deliver_later if send_mail_on_conf_update redirect_to edit_admin_conference_path(id: @conference.short_title), notice: 'Conference was successfully updated.' else diff --git a/app/controllers/admin/programs_controller.rb b/app/controllers/admin/programs_controller.rb index c0d20644..a4bcefba 100644 --- a/app/controllers/admin/programs_controller.rb +++ b/app/controllers/admin/programs_controller.rb @@ -14,7 +14,7 @@ module Admin 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 + Mailbot.send_on_schedule_public(@conference).deliver_later if send_mail_on_schedule_public redirect_to admin_conference_program_path(@conference.short_title), notice: 'The program was successfully updated.' else diff --git a/app/controllers/admin/registration_periods_controller.rb b/app/controllers/admin/registration_periods_controller.rb index 85a7eef4..727f15a8 100644 --- a/app/controllers/admin/registration_periods_controller.rb +++ b/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 + Mailbot.conference_registration_date_update_mail(@conference).deliver_later 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 + Mailbot.conference_registration_date_update_mail(@conference).deliver_later if send_mail_on_reg_update redirect_to admin_conference_registration_period_path(@conference.short_title), notice: 'Registration Period successfully updated.' else diff --git a/app/models/comment.rb b/app/models/comment.rb index c699f18a..7449b051 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -56,6 +56,6 @@ class Comment < ActiveRecord::Base private def send_notification - Mailbot.delay.send_notification_email_for_comment(self) + Mailbot.send_notification_email_for_comment(self).deliver_later end end diff --git a/app/models/event.rb b/app/models/event.rb index 6e42ad99..d8652c90 100644 --- a/app/models/event.rb +++ b/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,7 +122,7 @@ class Event < ActiveRecord::Base program.conference.email_settings.accepted_body && program.conference.email_settings.accepted_subject && !options[:send_mail].blank? - Mailbot.delay.acceptance_mail(self) + Mailbot.acceptance_mail(self).deliver_later end end @@ -131,7 +131,7 @@ class Event < ActiveRecord::Base program.conference.email_settings.rejected_body && program.conference.email_settings.rejected_subject && !options[:send_mail].blank? - Mailbot.delay.rejection_mail(self) + Mailbot.rejection_mail(self).deliver_later end end diff --git a/app/models/registration.rb b/app/models/registration.rb index 0d314c3d..2026af56 100644 --- a/app/models/registration.rb +++ b/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 diff --git a/app/models/venue.rb b/app/models/venue.rb index 2fcbf961..ac138c23 100644 --- a/app/models/venue.rb +++ b/app/models/venue.rb @@ -30,7 +30,7 @@ class Venue < ActiveRecord::Base private def send_mail_notification - Mailbot.delay.send_email_on_venue_updated(conference) if notify_on_venue_changed? + Mailbot.send_email_on_venue_updated(conference).deliver_later if notify_on_venue_changed? end def notify_on_venue_changed? diff --git a/config/application.rb b/config/application.rb index 6f863991..9123e5bb 100644 --- a/config/application.rb +++ b/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 From e758c302927237fd5c9c46072e4f5bb381eb72cf Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Thu, 21 Apr 2016 15:08:00 +0200 Subject: [PATCH 03/11] Use ActiveJob to send conference date update mails --- app/controllers/admin/conference_controller.rb | 2 +- app/jobs/conference_date_update_mail_job.rb | 9 +++++++++ app/mailers/mailbot.rb | 14 +++++++------- 3 files changed, 17 insertions(+), 8 deletions(-) create mode 100644 app/jobs/conference_date_update_mail_job.rb diff --git a/app/controllers/admin/conference_controller.rb b/app/controllers/admin/conference_controller.rb index 78fd68b4..def55090 100644 --- a/app/controllers/admin/conference_controller.rb +++ b/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.conference_date_update_mail(@conference).deliver_later 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 diff --git a/app/jobs/conference_date_update_mail_job.rb b/app/jobs/conference_date_update_mail_job.rb new file mode 100644 index 00000000..e8eeae12 --- /dev/null +++ b/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 diff --git a/app/mailers/mailbot.rb b/app/mailers/mailbot.rb index 7e5d23e8..901a70d6 100644 --- a/app/mailers/mailbot.rb +++ b/app/mailers/mailbot.rb @@ -35,13 +35,13 @@ class Mailbot < ActionMailer::Base conference.email_settings.generate_event_mail(event, conference.email_settings.confirmed_without_registration_body)) end - def conference_date_update_mail(conference) - User.joins(:subscriptions).merge(conference.subscriptions).each do |user| - build_email(conference, - user.email, - conference.email_settings.conference_dates_updated_subject, - conference.email_settings.generate_email_on_conf_updates(conference, user, conference.email_settings.conference_dates_updated_body)) - end + def conference_date_update_mail(conference, user) + mail(to: user.email, + from: conference.contact.email, + subject: conference.email_settings.conference_dates_updated_subject, + body: conference.email_settings.generate_email_on_conf_updates(conference, + user, + conference.email_settings.conference_dates_updated_body)) end def conference_registration_date_update_mail(conference) From 4dd15dc62c3c1ee9b0b8396d8effd113569c2e2d Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Thu, 21 Apr 2016 15:38:44 +0200 Subject: [PATCH 04/11] Use ActiveJob to send registration period update mails --- .../admin/registration_periods_controller.rb | 4 ++-- ...conference_registration_date_update_mail_job.rb | 9 +++++++++ app/mailers/mailbot.rb | 14 +++++++------- 3 files changed, 18 insertions(+), 9 deletions(-) create mode 100644 app/jobs/conference_registration_date_update_mail_job.rb diff --git a/app/controllers/admin/registration_periods_controller.rb b/app/controllers/admin/registration_periods_controller.rb index 727f15a8..07e9a735 100644 --- a/app/controllers/admin/registration_periods_controller.rb +++ b/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.conference_registration_date_update_mail(@conference).deliver_later 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.conference_registration_date_update_mail(@conference).deliver_later 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 diff --git a/app/jobs/conference_registration_date_update_mail_job.rb b/app/jobs/conference_registration_date_update_mail_job.rb new file mode 100644 index 00000000..9015d8af --- /dev/null +++ b/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 diff --git a/app/mailers/mailbot.rb b/app/mailers/mailbot.rb index 901a70d6..06f24789 100644 --- a/app/mailers/mailbot.rb +++ b/app/mailers/mailbot.rb @@ -44,13 +44,13 @@ class Mailbot < ActionMailer::Base conference.email_settings.conference_dates_updated_body)) end - def conference_registration_date_update_mail(conference) - User.joins(:subscriptions).merge(conference.subscriptions).uniq.joins('INNER JOIN registrations ON registrations.user_id != users.id').merge(conference.registrations).each do |user| - build_email(conference, - user.email, - conference.email_settings.conference_registration_dates_updated_subject, - conference.email_settings.generate_email_on_conf_updates(conference, user, conference.email_settings.conference_registration_dates_updated_body)) - end + def conference_registration_date_update_mail(conference, user) + mail(to: user.email, + from: conference.contact.email, + subject: conference.email_settings.conference_registration_dates_updated_subject, + body: conference.email_settings.generate_email_on_conf_updates(conference, + user, + conference.email_settings.conference_registration_dates_updated_body)) end def send_email_on_venue_updated(conference) From 2ea7bda639cfad31008b8f9cfbd9b0fbec4458c8 Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Thu, 21 Apr 2016 16:03:09 +0200 Subject: [PATCH 05/11] Use ActiveJob to send venue update mails --- app/jobs/conference_venue_update_mail_job.rb | 9 +++++++++ app/mailers/mailbot.rb | 14 +++++++------- app/models/venue.rb | 4 ++-- 3 files changed, 18 insertions(+), 9 deletions(-) create mode 100644 app/jobs/conference_venue_update_mail_job.rb diff --git a/app/jobs/conference_venue_update_mail_job.rb b/app/jobs/conference_venue_update_mail_job.rb new file mode 100644 index 00000000..2aa960d1 --- /dev/null +++ b/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 diff --git a/app/mailers/mailbot.rb b/app/mailers/mailbot.rb index 06f24789..d9581ba4 100644 --- a/app/mailers/mailbot.rb +++ b/app/mailers/mailbot.rb @@ -53,13 +53,13 @@ class Mailbot < ActionMailer::Base conference.email_settings.conference_registration_dates_updated_body)) end - def send_email_on_venue_updated(conference) - User.joins(:subscriptions).merge(conference.subscriptions).each do |user| - build_email(conference, - user.email, - conference.email_settings.venue_updated_subject, - conference.email_settings.generate_email_on_conf_updates(conference, user, conference.email_settings.venue_updated_body)) - end + def conference_venue_update_mail(conference, user) + mail(to: user.email, + from: conference.contact.email, + subject: conference.email_settings.venue_updated_subject, + body: conference.email_settings.generate_email_on_conf_updates(conference, + user, + conference.email_settings.venue_updated_body)) end def send_on_schedule_public(conference) diff --git a/app/models/venue.rb b/app/models/venue.rb index ac138c23..2604a0a9 100644 --- a/app/models/venue.rb +++ b/app/models/venue.rb @@ -12,7 +12,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}" @@ -30,7 +30,7 @@ class Venue < ActiveRecord::Base private def send_mail_notification - Mailbot.send_email_on_venue_updated(conference).deliver_later if notify_on_venue_changed? + ConferenceVenueUpdateMailJob.perform_later(conference) if notify_on_venue_changed? end def notify_on_venue_changed? From 82e356dde0a9dcb15b84057b06b29adcdd14de6e Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Thu, 21 Apr 2016 16:16:16 +0200 Subject: [PATCH 06/11] Use ActiveJob to send schedule update mails --- app/controllers/admin/programs_controller.rb | 2 +- app/jobs/conference_schedule_update_mail_job.rb | 9 +++++++++ app/mailers/mailbot.rb | 14 +++++++------- 3 files changed, 17 insertions(+), 8 deletions(-) create mode 100644 app/jobs/conference_schedule_update_mail_job.rb diff --git a/app/controllers/admin/programs_controller.rb b/app/controllers/admin/programs_controller.rb index a4bcefba..9727f82b 100644 --- a/app/controllers/admin/programs_controller.rb +++ b/app/controllers/admin/programs_controller.rb @@ -14,7 +14,7 @@ module Admin send_mail_on_schedule_public = @program.notify_on_schedule_public? if @program.update_attributes(program_params) - Mailbot.send_on_schedule_public(@conference).deliver_later 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 diff --git a/app/jobs/conference_schedule_update_mail_job.rb b/app/jobs/conference_schedule_update_mail_job.rb new file mode 100644 index 00000000..a6c73eca --- /dev/null +++ b/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 diff --git a/app/mailers/mailbot.rb b/app/mailers/mailbot.rb index d9581ba4..c10160d8 100644 --- a/app/mailers/mailbot.rb +++ b/app/mailers/mailbot.rb @@ -62,13 +62,13 @@ class Mailbot < ActionMailer::Base conference.email_settings.venue_updated_body)) end - def send_on_schedule_public(conference) - User.joins(:subscriptions).merge(conference.subscriptions).each do |user| - build_email(conference, - user.email, - conference.email_settings.program_schedule_public_subject, - conference.email_settings.generate_email_on_conf_updates(conference, user, conference.email_settings.program_schedule_public_body)) - end + def conference_schedule_update_mail(conference, user) + mail(to: user.email, + from: conference.contact.email, + subject: conference.email_settings.program_schedule_public_subject, + body: conference.email_settings.generate_email_on_conf_updates(conference, + user, + conference.email_settings.program_schedule_public_body)) end def send_on_cfp_dates_updates(conference) From a698ce44fe79d4b4b7da2565aa55677142cf7c34 Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Thu, 21 Apr 2016 16:26:37 +0200 Subject: [PATCH 07/11] Use ActiveJob to send cfp update mails --- app/controllers/admin/cfps_controller.rb | 4 +++- app/jobs/conference_cfp_update_mail_job.rb | 9 +++++++++ app/mailers/mailbot.rb | 14 +++++++------- 3 files changed, 19 insertions(+), 8 deletions(-) create mode 100644 app/jobs/conference_cfp_update_mail_job.rb diff --git a/app/controllers/admin/cfps_controller.rb b/app/controllers/admin/cfps_controller.rb index 119faf78..22bc3899 100644 --- a/app/controllers/admin/cfps_controller.rb +++ b/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.send_on_cfp_dates_updates(@conference).deliver_later 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 diff --git a/app/jobs/conference_cfp_update_mail_job.rb b/app/jobs/conference_cfp_update_mail_job.rb new file mode 100644 index 00000000..f732c068 --- /dev/null +++ b/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 diff --git a/app/mailers/mailbot.rb b/app/mailers/mailbot.rb index c10160d8..c112f9cb 100644 --- a/app/mailers/mailbot.rb +++ b/app/mailers/mailbot.rb @@ -71,13 +71,13 @@ class Mailbot < ActionMailer::Base conference.email_settings.program_schedule_public_body)) end - def send_on_cfp_dates_updates(conference) - User.joins(:subscriptions).merge(conference.subscriptions).each do |user| - build_email(conference, - user.email, - conference.email_settings.cfp_dates_updated_subject, - conference.email_settings.generate_email_on_conf_updates(conference, user, conference.email_settings.cfp_dates_updated_body)) - end + def conference_cfp_update_mail(conference, user) + mail(to: user.email, + from: conference.contact.email, + subject: conference.email_settings.cfp_dates_updated_subject, + body: conference.email_settings.generate_email_on_conf_updates(conference, + user, + conference.email_settings.cfp_dates_updated_body)) end def send_notification_email_for_comment(comment) From 37e0b5f5b88d3eef9ec67a6ddd1b45dcbb6cc8a8 Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Thu, 21 Apr 2016 16:36:53 +0200 Subject: [PATCH 08/11] Get rid of build_email It's always easy to add another layer of indirection... --- app/mailers/mailbot.rb | 52 +++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/app/mailers/mailbot.rb b/app/mailers/mailbot.rb index c112f9cb..9a6e3980 100644 --- a/app/mailers/mailbot.rb +++ b/app/mailers/mailbot.rb @@ -1,38 +1,41 @@ class Mailbot < ActionMailer::Base default from: 'no-reply@example.com' - def registration_mail(conference, person) - build_email(conference, - person.email, - conference.email_settings.registration_subject, - conference.email_settings.generate_email_on_conf_updates(conference, person, conference.email_settings.registration_body)) + def registration_mail(conference, user) + mail(to: user.email, + from: conference.contact.email, + subject: conference.email_settings.registration_subject, + body: conference.email_settings.generate_email_on_conf_updates(conference, + user, + conference.email_settings.registration_body)) end def acceptance_mail(event) conference = event.program.conference - person = event.submitter - build_email(conference, - person.email, - conference.email_settings.accepted_subject, - conference.email_settings.generate_event_mail(event, conference.email_settings.accepted_body)) + + mail(to: event.submitter.email, + from: conference.contact.email, + subject: conference.email_settings.accepted_subject, + 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, - person.email, - conference.email_settings.rejected_subject, - conference.email_settings.generate_event_mail(event, conference.email_settings.rejected_body)) + + mail(to: event.submitter.email, + from: conference.contact.email, + subject: conference.email_settings.rejected_subject, + 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, - person.email, - conference.email_settings.confirmed_without_registration_subject, - conference.email_settings.generate_event_mail(event, conference.email_settings.confirmed_without_registration_body)) + + mail(to: event.submitter.email, + from: conference.contact.email, + subject: conference.email_settings.confirmed_without_registration_subject, + body: conference.email_settings.generate_event_mail(event, + conference.email_settings.confirmed_without_registration_body)) end def conference_date_update_mail(conference, user) @@ -95,13 +98,4 @@ class Mailbot < ActionMailer::Base subject: "New comment has been posted for #{@event.title}") end end - - def build_email(conference, to, subject, body) - logger.debug "Sending mail about #{subject} to #{to}" - mail(to: to, - from: conference.contact.email, - reply_to: conference.contact.email, - subject: subject, - body: body) - end end From 8392fca0dcef15c8cfd0b91efd2423c0642221de Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Thu, 21 Apr 2016 16:38:50 +0200 Subject: [PATCH 09/11] Default from in Mailbot was never used --- app/mailers/mailbot.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/mailers/mailbot.rb b/app/mailers/mailbot.rb index 9a6e3980..e0259298 100644 --- a/app/mailers/mailbot.rb +++ b/app/mailers/mailbot.rb @@ -1,6 +1,4 @@ class Mailbot < ActionMailer::Base - default from: 'no-reply@example.com' - def registration_mail(conference, user) mail(to: user.email, from: conference.contact.email, From 3391d095493410a59cf4690c679f8bf1575b2ad9 Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Fri, 22 Apr 2016 11:29:38 +0200 Subject: [PATCH 10/11] Use ActiveJob to send comment mails --- app/jobs/event_comment_mail_job.rb | 11 +++++++++++ app/mailers/mailbot.rb | 19 ++++++++----------- app/models/comment.rb | 2 +- .../comment_template.text.erb | 0 4 files changed, 20 insertions(+), 12 deletions(-) create mode 100644 app/jobs/event_comment_mail_job.rb rename app/views/{admin/emails => mailbot}/comment_template.text.erb (100%) diff --git a/app/jobs/event_comment_mail_job.rb b/app/jobs/event_comment_mail_job.rb new file mode 100644 index 00000000..28564a1d --- /dev/null +++ b/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 diff --git a/app/mailers/mailbot.rb b/app/mailers/mailbot.rb index e0259298..3f8cda14 100644 --- a/app/mailers/mailbot.rb +++ b/app/mailers/mailbot.rb @@ -1,4 +1,5 @@ class Mailbot < ActionMailer::Base + def registration_mail(conference, user) mail(to: user.email, from: conference.contact.email, @@ -81,19 +82,15 @@ class Mailbot < ActionMailer::Base 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 + @user = user + + mail(to: @user.email, + from: @conference.contact.email, + template_name: 'comment_template', + subject: "New comment has been posted for #{@event.title}") end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 7449b051..1f501c3f 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -56,6 +56,6 @@ class Comment < ActiveRecord::Base private def send_notification - Mailbot.send_notification_email_for_comment(self).deliver_later + EventCommentMailJob.perform_later(self) end end diff --git a/app/views/admin/emails/comment_template.text.erb b/app/views/mailbot/comment_template.text.erb similarity index 100% rename from app/views/admin/emails/comment_template.text.erb rename to app/views/mailbot/comment_template.text.erb From 20d908e08586a5ece4b0f2652a138046cf48f4c0 Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Fri, 22 Apr 2016 14:53:10 +0200 Subject: [PATCH 11/11] Adapt the mailbot spec to the last changes --- app/mailers/mailbot.rb | 1 - spec/mailers/mailbot_spec.rb | 19 ++++++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/app/mailers/mailbot.rb b/app/mailers/mailbot.rb index 3f8cda14..3d70f18d 100644 --- a/app/mailers/mailbot.rb +++ b/app/mailers/mailbot.rb @@ -1,5 +1,4 @@ class Mailbot < ActionMailer::Base - def registration_mail(conference, user) mail(to: user.email, from: conference.contact.email, diff --git a/spec/mailers/mailbot_spec.rb b/spec/mailers/mailbot_spec.rb index b91a8d7a..5bafb4f7 100644 --- a/spec/mailers/mailbot_spec.rb +++ b/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 } - 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' } + let(:mail) { Mailbot.confirm_reminder_mail(event).deliver_now } end end end