diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 96aecf083..1573ed590 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -8,8 +8,7 @@ class CommentsController < ApplicationController def create if @comment.save - Mailer.comment(@comment).deliver_later if email_on_debate_comment? - Mailer.reply(@comment).deliver_later if email_on_comment_reply? + CommentNotifier.new(comment: @comment).process else render :new end @@ -55,14 +54,6 @@ class CommentsController < ApplicationController @commentable = Comment.find_commentable(comment_params[:commentable_type], comment_params[:commentable_id]) end - def email_on_debate_comment? - @comment.commentable.author.email_on_debate_comment? - end - - def email_on_comment_reply? - @comment.reply? && @comment.parent.author.email_on_comment_reply? - end - def administrator_comment? ["1", true].include?(comment_params[:as_administrator]) && can?(:comment_as_administrator, Debate) end diff --git a/app/models/comment.rb b/app/models/comment.rb index 493dbeb1c..73bd0b732 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -22,7 +22,7 @@ class Comment < ActiveRecord::Base scope :sort_for_moderation, -> { order(flags_count: :desc, updated_at: :desc) } scope :pending_flag_review, -> { where(ignored_flag_at: nil, hidden_at: nil) } - scope :with_ignored_flag, -> { where("ignored_flag_at IS NOT NULL AND hidden_at IS NULL") } + scope :with_ignored_flag, -> { where(hidden_at: nil).where.not(ignored_flag_at: nil) } scope :flagged, -> { where("flags_count > 0") } scope :for_render, -> { with_hidden.includes(user: :organization) } diff --git a/app/models/comment_notifier.rb b/app/models/comment_notifier.rb new file mode 100644 index 000000000..8a82e9df0 --- /dev/null +++ b/app/models/comment_notifier.rb @@ -0,0 +1,33 @@ +class CommentNotifier + def initialize(args = {}) + @comment = args.fetch(:comment) + @author = @comment.author + end + + def process + send_comment_email + send_reply_email + end + + private + + def send_comment_email + Mailer.comment(@comment).deliver_later if email_on_debate_comment? + end + + def send_reply_email + Mailer.reply(@comment).deliver_later if email_on_comment_reply? + end + + def email_on_debate_comment? + commentable_author = @comment.commentable.author + commentable_author != @author && commentable_author.email_on_debate_comment? + end + + def email_on_comment_reply? + return false unless @comment.reply? + parent_author = @comment.parent.author + parent_author != @author && parent_author.email_on_comment_reply? + end + +end diff --git a/app/models/debate.rb b/app/models/debate.rb index fdf9cb6b9..0347f0edd 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -27,7 +27,7 @@ class Debate < ActiveRecord::Base scope :sort_for_moderation, -> { order(flags_count: :desc, updated_at: :desc) } scope :pending_flag_review, -> { where(ignored_flag_at: nil, hidden_at: nil) } - scope :with_ignored_flag, -> { where("ignored_flag_at IS NOT NULL AND hidden_at IS NULL") } + scope :with_ignored_flag, -> { where.not(ignored_flag_at: nil).where(hidden_at: nil) } scope :flagged, -> { where("flags_count > 0") } scope :for_render, -> { includes(:tags) } scope :sort_by_hot_score , -> { order(hot_score: :desc) } diff --git a/app/models/organization.rb b/app/models/organization.rb index dc340b08c..fc9b8cec1 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -7,9 +7,9 @@ class Organization < ActiveRecord::Base delegate :email, :phone_number, to: :user - scope :pending, -> { where('organizations.verified_at is null and rejected_at is null') } - scope :verified, -> { where("organizations.verified_at is not null and (rejected_at is null or rejected_at < organizations.verified_at)") } - scope :rejected, -> { where("rejected_at is not null and (organizations.verified_at is null or organizations.verified_at < rejected_at)") } + scope :pending, -> { where(verified_at: nil, rejected_at: nil) } + scope :verified, -> { where.not(verified_at: nil).where("(rejected_at IS NULL or rejected_at < organizations.verified_at)") } + scope :rejected, -> { where.not(rejected_at: nil).where("(organizations.verified_at IS NULL or organizations.verified_at < rejected_at)") } def verify update(verified_at: Time.now) diff --git a/lib/acts_as_paranoid_aliases.rb b/lib/acts_as_paranoid_aliases.rb index e8abab530..6fd875ef6 100644 --- a/lib/acts_as_paranoid_aliases.rb +++ b/lib/acts_as_paranoid_aliases.rb @@ -31,11 +31,11 @@ module ActsAsParanoidAliases module ClassMethods def with_confirmed_hide - where("confirmed_hide_at IS NOT NULL") + where.not(confirmed_hide_at: nil) end def without_confirmed_hide - where("confirmed_hide_at IS NULL") + where(confirmed_hide_at: nil) end def with_hidden @@ -57,4 +57,3 @@ module ActsAsParanoidAliases end end end - diff --git a/spec/features/emails_spec.rb b/spec/features/emails_spec.rb index 86c794fac..b6a9d1db5 100644 --- a/spec/features/emails_spec.rb +++ b/spec/features/emails_spec.rb @@ -24,40 +24,59 @@ feature 'Emails' do expect(email).to have_body_text(edit_user_password_path) end - scenario "Debate comment", :js do - user = create(:user, email_on_debate_comment: true) - debate = create(:debate, author: user) - comment_on(debate) + context 'Debate comments' do + scenario "Send email on debate comment", :js do + user = create(:user, email_on_debate_comment: true) + debate = create(:debate, author: user) + comment_on(debate) - email = open_last_email - expect(email).to have_subject('Someone has commented on your debate') - expect(email).to deliver_to(debate.author) - expect(email).to have_body_text(debate_path(debate)) + email = open_last_email + expect(email).to have_subject('Someone has commented on your debate') + expect(email).to deliver_to(debate.author) + expect(email).to have_body_text(debate_path(debate)) + end + + scenario 'Do not send email about own debate comments', :js do + user = create(:user, email_on_debate_comment: true) + debate = create(:debate, author: user) + comment_on(debate, user) + + expect { open_last_email }.to raise_error "No email has been sent!" + end + + scenario 'Do not send email about debate comment unless set in preferences', :js do + user = create(:user, email_on_debate_comment: false) + debate = create(:debate, author: user) + comment_on(debate) + + expect { open_last_email }.to raise_error "No email has been sent!" + end end - scenario "Comment reply", :js do - user = create(:user, email_on_comment_reply: true) - reply_to(user) + context 'Comment replies' do + scenario "Send email on comment reply", :js do + user = create(:user, email_on_comment_reply: true) + reply_to(user) - email = open_last_email - expect(email).to have_subject('Someone has replied to your comment') - expect(email).to deliver_to(user) - expect(email).to have_body_text(debate_path(Comment.first.debate)) + email = open_last_email + expect(email).to have_subject('Someone has replied to your comment') + expect(email).to deliver_to(user) + expect(email).to have_body_text(debate_path(Comment.first.debate)) + end + + scenario "Do not send email about own replies to own comments", :js do + user = create(:user, email_on_comment_reply: true) + reply_to(user, user) + + expect { open_last_email }.to raise_error "No email has been sent!" + end + + scenario "Do not send email about comment reply unless set in preferences", :js do + user = create(:user, email_on_comment_reply: false) + reply_to(user) + + expect { open_last_email }.to raise_error "No email has been sent!" + end end - scenario 'Do not send email about debate comment unless set in preferences', :js do - user = create(:user, email_on_debate_comment: false) - debate = create(:debate, author: user) - comment_on(debate) - - expect { open_last_email }.to raise_error "No email has been sent!" - end - - scenario "Do not send email about comment reply unless set in preferences", :js do - user = create(:user, email_on_comment_reply: false) - reply_to(user) - - expect { open_last_email }.to raise_error "No email has been sent!" - end - -end \ No newline at end of file +end diff --git a/spec/support/common_actions.rb b/spec/support/common_actions.rb index 61974864d..508daa297 100644 --- a/spec/support/common_actions.rb +++ b/spec/support/common_actions.rb @@ -44,8 +44,8 @@ module CommonActions click_button 'Send me reset password instructions' end - def comment_on(debate) - user = create(:user) + def comment_on(debate, user = nil) + user ||= create(:user) login_as(user) visit debate_path(debate) @@ -56,10 +56,11 @@ module CommonActions expect(page).to have_content 'Have you thought about...?' end - def reply_to(user) - manuela = create(:user) + def reply_to(original_user, manuela = nil) + manuela ||= create(:user) + debate = create(:debate) - comment = create(:comment, commentable: debate, user: user) + comment = create(:comment, commentable: debate, user: original_user) login_as(manuela) visit debate_path(debate)