diff --git a/app/mailers/notification_mailers/also_commented.rb b/app/mailers/notification_mailers/also_commented.rb index aa063cfa8..c6c4da8de 100644 --- a/app/mailers/notification_mailers/also_commented.rb +++ b/app/mailers/notification_mailers/also_commented.rb @@ -7,7 +7,6 @@ module NotificationMailers @comment = Comment.find_by_id(comment_id) if mail? - @headers[:from] = "\"#{@comment.author_name} (diaspora*)\" <#{AppConfig.mail.sender_address}>" @headers[:in_reply_to] = @headers[:references] = "<#{@comment.parent.guid}@#{AppConfig.pod_uri.host}>" if @comment.public? @headers[:subject] = "Re: #{@comment.comment_email_subject}" diff --git a/app/mailers/notification_mailers/comment_on_post.rb b/app/mailers/notification_mailers/comment_on_post.rb index 4782f52b2..c9957e86f 100644 --- a/app/mailers/notification_mailers/comment_on_post.rb +++ b/app/mailers/notification_mailers/comment_on_post.rb @@ -5,7 +5,6 @@ module NotificationMailers def set_headers(comment_id) @comment = Comment.find(comment_id) - @headers[:from] = "\"#{@comment.author_name} (diaspora*)\" <#{AppConfig.mail.sender_address}>" @headers[:in_reply_to] = @headers[:references] = "<#{@comment.parent.guid}@#{AppConfig.pod_uri.host}>" if @comment.public? @headers[:subject] = "Re: #{@comment.comment_email_subject}" diff --git a/app/mailers/notification_mailers/mentioned_in_comment.rb b/app/mailers/notification_mailers/mentioned_in_comment.rb index abb346ea8..0e9a51bba 100644 --- a/app/mailers/notification_mailers/mentioned_in_comment.rb +++ b/app/mailers/notification_mailers/mentioned_in_comment.rb @@ -5,6 +5,7 @@ module NotificationMailers def set_headers(target_id) # rubocop:disable Style/AccessorMethodName @comment = Mention.find_by_id(target_id).mentions_container + @headers[:in_reply_to] = @headers[:references] = "<#{@comment.parent.guid}@#{AppConfig.pod_uri.host}>" @headers[:subject] = I18n.t("notifier.mentioned.subject", name: @sender.name) end end diff --git a/app/mailers/notification_mailers/private_message.rb b/app/mailers/notification_mailers/private_message.rb index 9cbe71dc3..60a5013e8 100644 --- a/app/mailers/notification_mailers/private_message.rb +++ b/app/mailers/notification_mailers/private_message.rb @@ -3,11 +3,10 @@ module NotificationMailers attr_accessor :message, :conversation, :participants def set_headers(message_id) - @message = Message.find_by_id(message_id) + @message = Message.find_by_id(message_id) @conversation = @message.conversation @participants = @conversation.participants - @headers[:from] = "\"#{@message.author_name} (diaspora*)\" <#{AppConfig.mail.sender_address}>" @headers[:subject] = I18n.t("notifier.private_message.subject") @headers[:in_reply_to] = @headers[:references] = "<#{@conversation.guid}@#{AppConfig.pod_uri.host}>" end diff --git a/app/models/notifications/mentioned_in_comment.rb b/app/models/notifications/mentioned_in_comment.rb index 5e5466b68..35ef9b61a 100644 --- a/app/models/notifications/mentioned_in_comment.rb +++ b/app/models/notifications/mentioned_in_comment.rb @@ -11,7 +11,7 @@ module Notifications end def self.filter_mentions(mentions, mentionable, _recipient_user_ids) - mentions.joins(:person).merge(Person.allowed_to_be_mentioned_in_a_comment_to(mentionable.parent)) + mentions.includes(:person).merge(Person.allowed_to_be_mentioned_in_a_comment_to(mentionable.parent)) end def mail_job diff --git a/app/models/person.rb b/app/models/person.rb index e9eefc6ec..3da18333b 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -61,11 +61,7 @@ class Person < ActiveRecord::Base } scope :remote, -> { where('people.owner_id IS NULL') } scope :local, -> { where('people.owner_id IS NOT NULL') } - scope :for_json, -> { - select("people.id, people.guid, people.diaspora_handle") - .distinct - .includes(:profile) - } + scope :for_json, -> { select("people.id, people.guid, people.diaspora_handle").includes(:profile) } # @note user is passed in here defensively scope :all_from_aspects, ->(aspect_ids, user) { @@ -131,12 +127,13 @@ class Person < ActiveRecord::Base # @param [Post] the post for which we query mentionable in comments people # @return [Person::ActiveRecord_Relation] scope :allowed_to_be_mentioned_in_a_comment_to, ->(post) { - if post.public? - all - else - left_join_visible_post_interactions_on_authorship(post.id) - .where("comments.id IS NOT NULL OR likes.id IS NOT NULL OR people.id = #{post.author_id}") - end + allowed = if post.public? + all + else + left_join_visible_post_interactions_on_authorship(post.id) + .where("comments.id IS NOT NULL OR likes.id IS NOT NULL OR people.id = #{post.author_id}") + end + allowed.distinct } # This scope adds sorting of people in the order, appropriate for suggesting to a user (current user) who diff --git a/spec/integration/mentioning_spec.rb b/spec/integration/mentioning_spec.rb index 2e6b4292d..367758bf8 100644 --- a/spec/integration/mentioning_spec.rb +++ b/spec/integration/mentioning_spec.rb @@ -376,6 +376,18 @@ describe "mentioning", type: :request do end end end + + it "only creates one notification for the mentioned person, when mentioned person commented twice before" do + parent = FactoryGirl.create(:status_message_in_aspect, author: author.person) + mentioned_user = FactoryGirl.create(:user_with_aspect, friends: [author]) + mentioned_user.comment!(parent, "test comment 1") + mentioned_user.comment!(parent, "test comment 2") + comment = inlined_jobs do + author.comment!(parent, text_mentioning(mentioned_user)) + end + + expect(notifications_about_mentioning(mentioned_user, comment).count).to eq(1) + end end end end diff --git a/spec/mailers/notifier_spec.rb b/spec/mailers/notifier_spec.rb index 2ded0d80f..3adaca132 100644 --- a/spec/mailers/notifier_spec.rb +++ b/spec/mailers/notifier_spec.rb @@ -120,6 +120,11 @@ describe Notifier, type: :mailer do expect(mail.subject).to include(comment.author.name) end + it "IN-REPLY-TO and REFERENCES: references the commented post" do + expect(mail.in_reply_to).to eq("#{comment.parent.guid}@#{AppConfig.pod_uri.host}") + expect(mail.references).to eq("#{comment.parent.guid}@#{AppConfig.pod_uri.host}") + end + it "has the comment link in the body" do expect(mail.body.encoded).to include(post_url(comment.parent, anchor: comment.guid)) end @@ -276,7 +281,9 @@ describe Notifier, type: :mailer do let(:comment) { eve.comment!(commented_post, "Totally is") } describe ".comment_on_post" do - let(:comment_mail) { Notifier.send_notification("comment_on_post", bob.id, person.id, comment.id).deliver_now } + let(:comment_mail) { + Notifier.send_notification("comment_on_post", bob.id, eve.person.id, comment.id).deliver_now + } it "TO: goes to the right person" do expect(comment_mail.to).to eq([bob.email]) @@ -317,7 +324,7 @@ describe Notifier, type: :mailer do end describe ".also_commented" do - let(:comment_mail) { Notifier.send_notification("also_commented", bob.id, person.id, comment.id) } + let(:comment_mail) { Notifier.send_notification("also_commented", bob.id, eve.person.id, comment.id) } it "TO: goes to the right person" do expect(comment_mail.to).to eq([bob.email]) diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 4e65c6ccc..0180ba72a 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -24,16 +24,6 @@ describe Person, :type => :model do Person.for_json.first.serialized_public_key }.to raise_error ActiveModel::MissingAttributeError end - - it 'selects distinct people' do - aspect = bob.aspects.create(:name => 'hilarious people') - aspect.contacts << bob.contact_for(eve.person) - person_ids = Person.for_json.joins(:contacts => :aspect_memberships). - where(:contacts => {:user_id => bob.id}, - :aspect_memberships => {:aspect_id => bob.aspect_ids}).map{|p| p.id} - - expect(person_ids.uniq).to eq(person_ids) - end end describe '.local' do @@ -131,9 +121,17 @@ describe Person, :type => :model do ).to match_array([alice, bob, eve, kate].map(&:person_id)) end + it "selects distinct people" do + alice.comment!(status_bob, "hi") + alice.comment!(status_bob, "how are you?") + expect( + Person.allowed_to_be_mentioned_in_a_comment_to(status_bob).ids + ).to match_array([alice, bob].map(&:person_id)) + end + it "returns all for public posts" do status_bob.update(public: true) # set parent public - expect(Person.allowed_to_be_mentioned_in_a_comment_to(status_bob)).to eq(Person.all) + expect(Person.allowed_to_be_mentioned_in_a_comment_to(status_bob).ids).to match_array(Person.ids) end end