Remove private post content of also commented mails

merges #5931

Changelog.md

@@ -31,6 +31,7 @@
 * Support syntax highlighting for fenced code blocks [#5908](https://github.com/diaspora/diaspora/pull/5908)
 * Added link to diasporafoundation.org to invitation email [#5893](https://github.com/diaspora/diaspora/pull/5893)
 * Gracefully handle missing `og:url`s [#5926](https://github.com/diaspora/diaspora/pull/5926)
+* Remove private post content from "also commented" mails [#5931](https://github.com/diaspora/diaspora/pull/5931)
 
 # 0.5.0.1
 

app/mailers/notification_mailers/also_commented.rb

@@ -8,7 +8,11 @@ module NotificationMailers
 
       if mail?
         @headers[:from] = "\"#{@comment.author_name} (diaspora*)\" <#{AppConfig.mail.sender_address}>"
+        if @comment.public?
           @headers[:subject] = "Re: #{@comment.comment_email_subject}"
+        else
+          @headers[:subject] = I18n.t("notifier.also_commented.limited_subject")
+        end
       end
     end
 

config/locales/diaspora/en.yml

@@ -754,8 +754,10 @@ en:
         sharing: "has started sharing with you!"
         view_profile: "View %{name}’s profile"
     comment_on_post:
-        limited_subject: "There's a new comment on a post you commented"
+        limited_subject: "There's a new comment on one of your posts"
         reply: "Reply or view %{name}’s post >"
+    also_commented:
+        limited_subject: "There's a new comment on a post you commented"
     mentioned:
         subject: "%{name} has mentioned you on diaspora*"
         mentioned: "mentioned you in a post:"

spec/mailers/notifier_spec.rb

@@ -1,14 +1,14 @@
-require 'spec_helper'
+require "spec_helper"
 
-describe Notifier, :type => :mailer do
+describe Notifier, type: :mailer do
   let(:person) { FactoryGirl.create(:person) }
 
   before do
     Notifier.deliveries = []
   end
 
-  describe '.administrative' do
+  describe ".administrative" do
-    it 'mails a user' do
+    it "mails a user" do
       mails = Notifier.admin("Welcome to bureaucracy!", [bob])
       expect(mails.length).to eq(1)
       mail = mails.first
@@ -17,25 +17,26 @@ describe Notifier, :type => :mailer do
       expect(mail.body.encoded).to match /#{bob.username}/
     end
 
-    context 'mails a bunch of users' do
+    context "mails a bunch of users" do
       before do
         @users = []
         5.times do
           @users << FactoryGirl.create(:user)
         end
       end
-      it 'has a body' do
+      it "has a body" do
         mails = Notifier.admin("Welcome to bureaucracy!", @users)
         expect(mails.length).to eq(5)
         mails.each {|mail|
-          this_user = @users.detect{|u| mail.to == [u.email]}
+          this_user = @users.find {|u| mail.to == [u.email] }
           expect(mail.body.encoded).to match /Welcome to bureaucracy!/
           expect(mail.body.encoded).to match /#{this_user.username}/
         }
       end
 
       it "has attachments" do
-        mails = Notifier.admin("Welcome to bureaucracy!", @users, :attachments => [{:name => "retention stats", :file => "here is some file content"}])
+        mails = Notifier.admin("Welcome to bureaucracy!", @users,
+                               attachments: [{name: "retention stats", file: "here is some file content"}])
         expect(mails.length).to eq(5)
         mails.each {|mail|
           expect(mail.attachments.count).to eq(1)
@@ -44,21 +45,22 @@ describe Notifier, :type => :mailer do
     end
   end
 
-  describe '.single_admin' do
+  describe ".single_admin" do
-    it 'mails a user' do
+    it "mails a user" do
       mail = Notifier.single_admin("Welcome to bureaucracy!", bob)
       expect(mail.to).to eq([bob.email])
       expect(mail.body.encoded).to match /Welcome to bureaucracy!/
       expect(mail.body.encoded).to match /#{bob.username}/
     end
 
-    it 'has the layout' do
+    it "has the layout" do
       mail = Notifier.single_admin("Welcome to bureaucracy!", bob)
       expect(mail.body.encoded).to match /change your notification settings/
     end
 
-    it 'has an optional attachment' do
+    it "has an optional attachment" do
-      mail = Notifier.single_admin("Welcome to bureaucracy!", bob, :attachments => [{:name => "retention stats", :file => "here is some file content"}])
+      mail = Notifier.single_admin("Welcome to bureaucracy!", bob,
+                                   attachments: [{name: "retention stats", file: "here is some file content"}])
       expect(mail.attachments.length).to eq(1)
     end
   end
@@ -66,15 +68,15 @@ describe Notifier, :type => :mailer do
   describe ".started_sharing" do
     let!(:request_mail) { Notifier.started_sharing(bob.id, person.id) }
 
-    it 'goes to the right person' do
+    it "goes to the right person" do
       expect(request_mail.to).to eq([bob.email])
     end
 
-    it 'has the name of person sending the request' do
+    it "has the name of person sending the request" do
       expect(request_mail.body.encoded.include?(person.name)).to be true
     end
 
-    it 'has the css' do
+    it "has the css" do
       request_mail.body.encoded.include?("<style type='text/css'>")
     end
   end
@@ -83,25 +85,25 @@ describe Notifier, :type => :mailer do
     before do
       @user = alice
       @post = FactoryGirl.create(:status_message, public: true)
-      @mention = Mention.create(:person => @user.person, :post => @post)
+      @mention = Mention.create(person: @user.person, post: @post)
 
       @mail = Notifier.mentioned(@user.id, @post.author.id, @mention.id)
     end
 
-    it 'TO: goes to the right person' do
+    it "TO: goes to the right person" do
       expect(@mail.to).to eq([@user.email])
     end
 
-    it 'SUBJECT: has the name of person mentioning in the subject' do
+    it "SUBJECT: has the name of person mentioning in the subject" do
       expect(@mail.subject).to include(@post.author.name)
     end
 
-    it 'has the post text in the body' do
+    it "has the post text in the body" do
       expect(@mail.body.encoded).to include(@post.text)
     end
 
-    it 'should not include translation fallback' do
+    it "should not include translation fallback" do
-      expect(@mail.body.encoded).not_to include(I18n.translate 'notifier.a_post_you_shared')
+      expect(@mail.body.encoded).not_to include(I18n.translate "notifier.a_post_you_shared")
     end
   end
 
@@ -133,37 +135,7 @@ describe Notifier, :type => :mailer do
 
   describe ".liked" do
     before do
-      @post = FactoryGirl.create(:status_message, :author => alice.person, :public => true)
+      @post = FactoryGirl.create(:status_message, author: alice.person, public: true)
-      @like = @post.likes.create!(:author => bob.person)
-      @mail = Notifier.liked(alice.id, @like.author.id, @like.id)
-    end
-
-    it 'TO: goes to the right person' do
-      expect(@mail.to).to eq([alice.email])
-    end
-
-    it 'BODY: contains the original post' do
-      expect(@mail.body.encoded).to include(@post.message.plain_text)
-    end
-
-    it 'BODY: contains the name of person liking' do
-      expect(@mail.body.encoded).to include(@like.author.name)
-    end
-
-    it 'should not include translation fallback' do
-      expect(@mail.body.encoded).not_to include(I18n.translate 'notifier.a_post_you_shared')
-    end
-
-    it 'can handle a reshare' do
-      reshare = FactoryGirl.create(:reshare)
-      like = reshare.likes.create!(:author => bob.person)
-      Notifier.liked(alice.id, like.author.id, like.id)
-    end
-  end
-
-  describe ".liked limited" do
-    before do
-      @post = FactoryGirl.create(:status_message, author: alice.person, public: false)
       @like = @post.likes.create!(author: bob.person)
       @mail = Notifier.liked(alice.id, @like.author.id, @like.id)
     end
@@ -172,8 +144,8 @@ describe Notifier, :type => :mailer do
       expect(@mail.to).to eq([alice.email])
     end
 
-    it "BODY: not contains the original post" do
+    it "BODY: contains the original post" do
-      expect(@mail.body.encoded).not_to include(@post.message.plain_text)
+      expect(@mail.body.encoded).to include(@post.message.plain_text)
     end
 
     it "BODY: contains the name of person liking" do
@@ -183,43 +155,48 @@ describe Notifier, :type => :mailer do
     it "should not include translation fallback" do
       expect(@mail.body.encoded).not_to include(I18n.translate "notifier.a_post_you_shared")
     end
+
+    it "can handle a reshare" do
+      reshare = FactoryGirl.create(:reshare)
+      like = reshare.likes.create!(author: bob.person)
+      Notifier.liked(alice.id, like.author.id, like.id)
+    end
   end
 
   describe ".reshared" do
     before do
-      @post = FactoryGirl.create(:status_message, :author => alice.person, :public => true)
+      @post = FactoryGirl.create(:status_message, author: alice.person, public: true)
-      @reshare = FactoryGirl.create(:reshare, :root => @post, :author => bob.person)
+      @reshare = FactoryGirl.create(:reshare, root: @post, author: bob.person)
       @mail = Notifier.reshared(alice.id, @reshare.author.id, @reshare.id)
     end
 
-    it 'TO: goes to the right person' do
+    it "TO: goes to the right person" do
       expect(@mail.to).to eq([alice.email])
     end
 
-    it 'BODY: contains the truncated original post' do
+    it "BODY: contains the truncated original post" do
       expect(@mail.body.encoded).to include(@post.message.plain_text)
     end
 
-    it 'BODY: contains the name of person liking' do
+    it "BODY: contains the name of person liking" do
       expect(@mail.body.encoded).to include(@reshare.author.name)
     end
 
-    it 'should not include translation fallback' do
+    it "should not include translation fallback" do
-      expect(@mail.body.encoded).not_to include(I18n.translate 'notifier.a_post_you_shared')
+      expect(@mail.body.encoded).not_to include(I18n.translate "notifier.a_post_you_shared")
     end
   end
 
-
   describe ".private_message" do
     before do
       @user2 = bob
       @participant_ids = @user2.contacts.map {|c| c.person.id } + [@user2.person.id]
 
       @create_hash = {
-        :author => @user2.person,
+        author:              @user2.person,
-        :participant_ids => @participant_ids,
+        participant_ids:     @participant_ids,
-        :subject => "cool stuff",
+        subject:             "cool stuff",
-        :messages_attributes => [ {:author => @user2.person, :text => 'hey'} ]
+        messages_attributes: [{author: @user2.person, text: "hey"}]
       }
 
       @cnv = Conversation.create(@create_hash)
@@ -227,7 +204,7 @@ describe Notifier, :type => :mailer do
       @mail = Notifier.private_message(bob.id, @cnv.author.id, @cnv.messages.first.id)
     end
 
-    it 'TO: goes to the right person' do
+    it "TO: goes to the right person" do
       expect(@mail.to).to eq([bob.email])
     end
 
@@ -235,34 +212,39 @@ describe Notifier, :type => :mailer do
       expect(@mail["From"].to_s).to eq("\"#{@cnv.author.name} (diaspora*)\" <#{AppConfig.mail.sender_address}>")
     end
 
-    it 'SUBJECT: has a snippet of the post contents' do
+    it "SUBJECT: has a snippet of the post contents" do
       expect(@mail.subject).to eq(@cnv.subject)
     end
 
-    it 'SUBJECT: has "Re:" if not the first message in a conversation' do
+    it "SUBJECT: has 'Re:' if not the first message in a conversation" do
-      @cnv.messages << Message.new(:text => 'yo', :author => eve.person)
+      @cnv.messages << Message.new(text: "yo", author: eve.person)
       @mail = Notifier.private_message(bob.id, @cnv.author.id, @cnv.messages.last.id)
 
       expect(@mail.subject).to eq("Re: #{@cnv.subject}")
     end
 
-    it 'BODY: does not contain the message text' do
+    it "BODY: does not contain the message text" do
       expect(@mail.body.encoded).not_to include(@cnv.messages.first.text)
     end
 
-    it 'should not include translation fallback' do
+    it "should not include translation fallback" do
-      expect(@mail.body.encoded).not_to include(I18n.translate 'notifier.a_post_you_shared')
+      expect(@mail.body.encoded).not_to include(I18n.translate "notifier.a_post_you_shared")
     end
   end
 
   context "comments" do
-    let(:commented_post) {bob.post(:status_message, :text => "### Headline \r\n It's **really** sunny outside today, and this is a super long status message!  #notreally", :to => :all, :public => true)}
+    let(:commented_post) {
+      bob.post(:status_message,
+               text:   "### Headline \r\n It's **really** sunny outside today, and this is a super long status message!  #notreally",
+               to:     :all,
+               public: true)
+    }
     let(:comment) { eve.comment!(commented_post, "Totally is") }
 
     describe ".comment_on_post" do
       let(:comment_mail) { Notifier.comment_on_post(bob.id, person.id, comment.id).deliver_now }
 
-      it 'TO: goes to the right person' do
+      it "TO: goes to the right person" do
         expect(comment_mail.to).to eq([bob.email])
       end
 
@@ -270,28 +252,28 @@ describe Notifier, :type => :mailer do
         expect(comment_mail["From"].to_s).to eq("\"#{eve.name} (diaspora*)\" <#{AppConfig.mail.sender_address}>")
       end
 
-      it 'SUBJECT: has a snippet of the post contents, without markdown and without newlines' do
+      it "SUBJECT: has a snippet of the post contents, without markdown and without newlines" do
         expect(comment_mail.subject).to eq("Re: Headline")
       end
 
-      context 'BODY' do
+      context "BODY" do
         it "contains the comment" do
           expect(comment_mail.body.encoded).to include(comment.text)
         end
 
         it "contains the original post's link" do
-          expect(comment_mail.body.encoded.include?("#{comment.post.id.to_s}")).to be true
+          expect(comment_mail.body.encoded.include?("#{comment.post.id}")).to be true
         end
 
-        it 'should not include translation fallback' do
+        it "should not include translation fallback" do
-          expect(comment_mail.body.encoded).not_to include(I18n.translate 'notifier.a_post_you_shared')
+          expect(comment_mail.body.encoded).not_to include(I18n.translate "notifier.a_post_you_shared")
         end
       end
 
       [:reshare].each do |post_type|
         context post_type.to_s do
-          let(:commented_post) { FactoryGirl.create(post_type, :author => bob.person) }
+          let(:commented_post) { FactoryGirl.create(post_type, author: bob.person) }
-          it 'succeeds' do
+          it "succeeds" do
             expect {
               comment_mail
             }.not_to raise_error
@@ -303,35 +285,35 @@ describe Notifier, :type => :mailer do
     describe ".also_commented" do
       let(:comment_mail) { Notifier.also_commented(bob.id, person.id, comment.id) }
 
-      it 'TO: goes to the right person' do
+      it "TO: goes to the right person" do
         expect(comment_mail.to).to eq([bob.email])
       end
 
-      it 'FROM: has the name of person commenting as the sender' do
+      it "FROM: has the name of person commenting as the sender" do
         expect(comment_mail["From"].to_s).to eq("\"#{eve.name} (diaspora*)\" <#{AppConfig.mail.sender_address}>")
       end
 
-      it 'SUBJECT: has a snippet of the post contents, without markdown and without newlines' do
+      it "SUBJECT: has a snippet of the post contents, without markdown and without newlines" do
         expect(comment_mail.subject).to eq("Re: Headline")
       end
 
-      context 'BODY' do
+      context "BODY" do
         it "contains the comment" do
           expect(comment_mail.body.encoded).to include(comment.text)
         end
 
         it "contains the original post's link" do
-          expect(comment_mail.body.encoded).to include("#{comment.post.id.to_s}")
+          expect(comment_mail.body.encoded).to include("#{comment.post.id}")
         end
 
-        it 'should not include translation fallback' do
+        it "should not include translation fallback" do
-          expect(comment_mail.body.encoded).not_to include(I18n.translate 'notifier.a_post_you_shared')
+          expect(comment_mail.body.encoded).not_to include(I18n.translate "notifier.a_post_you_shared")
         end
       end
       [:reshare].each do |post_type|
         context post_type.to_s do
-          let(:commented_post) { FactoryGirl.create(post_type, :author => bob.person) }
+          let(:commented_post) { FactoryGirl.create(post_type, author: bob.person) }
-          it 'succeeds' do
+          it "succeeds" do
             expect {
               comment_mail
             }.not_to raise_error
@@ -339,6 +321,96 @@ describe Notifier, :type => :mailer do
         end
       end
     end
+  end
+
+  context "limited post" do
+    let(:limited_post) {
+      alice.post(:status_message, to: :all, public: false,
+        text: "### Limited headline \r\n It's **really** sunny outside today")
+    }
+
+    context "comments" do
+      let(:comment) { bob.comment!(limited_post, "Totally is") }
+
+      describe ".also_commented" do
+        let(:mail) { Notifier.also_commented(alice.id, bob.person.id, comment.id) }
+
+        it "TO: goes to the right person" do
+          expect(mail.to).to eq([alice.email])
+        end
+
+        it "FROM: contains the sender's name" do
+          expect(mail["From"].to_s).to eq("\"#{bob.name} (diaspora*)\" <#{AppConfig.mail.sender_address}>")
+        end
+
+        it "SUBJECT: does not show the limited post" do
+          expect(mail.subject).not_to include("Limited headline")
+        end
+
+        it "BODY: does not show limited message" do
+          expect(mail.body.encoded).not_to include("Limited headline")
+        end
+
+        it "BODY: does not show the comment" do
+          expect(mail.body.encoded).not_to include("Totally is")
+        end
+      end
+
+      describe ".comment_on_post" do
+        let(:comment) { bob.comment!(limited_post, "Totally is") }
+        let(:mail) { Notifier.comment_on_post(alice.id, bob.person.id, comment.id) }
+
+        it "TO: goes to the right person" do
+          expect(mail.to).to eq([alice.email])
+        end
+
+        it "FROM: contains the sender's name" do
+          expect(mail["From"].to_s).to eq("\"#{bob.name} (diaspora*)\" <#{AppConfig.mail.sender_address}>")
+        end
+
+        it "SUBJECT: does not show the limited post" do
+          expect(mail.subject).not_to include("Limited headline")
+        end
+
+        it "BODY: does not show the limited post" do
+          expect(mail.body.encoded).not_to include("Limited headline")
+        end
+
+        it "BODY: does not show the comment" do
+          expect(mail.body.encoded).not_to include("Totally is")
+        end
+      end
+    end
+
+    describe ".liked" do
+      let(:like) { bob.like!(limited_post) }
+      let(:mail) { Notifier.liked(alice.id, bob.person.id, like.id) }
+
+      it "TO: goes to the right person" do
+        expect(mail.to).to eq([alice.email])
+      end
+
+      it "FROM: contains the sender's name" do
+        expect(mail["From"].to_s).to eq("\"#{bob.name} (diaspora*)\" <#{AppConfig.mail.sender_address}>")
+      end
+
+      it "SUBJECT: does not show the limited post" do
+        expect(mail.subject).not_to include("Limited headline")
+      end
+
+      it "BODY: does not show the limited post" do
+        expect(mail.body.encoded).not_to include("Limited headline")
+      end
+
+      it "BODY: contains the name of person liking" do
+        expect(mail.body.encoded).to include(bob.name)
+      end
+
+      it "should not include translation fallback" do
+        expect(mail.body.encoded).not_to include(I18n.translate "notifier.a_post_you_shared")
+      end
+    end
+  end
 
   describe ".confirm_email" do
     before do
@@ -346,50 +418,29 @@ describe Notifier, :type => :mailer do
       @confirm_email = Notifier.confirm_email(bob.id)
     end
 
-      it 'goes to the right person' do
+    it "goes to the right person" do
       expect(@confirm_email.to).to eq([bob.unconfirmed_email])
     end
 
-      it 'has the unconfirmed emil in the subject' do
+    it "has the unconfirmed emil in the subject" do
       expect(@confirm_email.subject).to include(bob.unconfirmed_email)
     end
 
-      it 'has the unconfirmed emil in the body' do
+    it "has the unconfirmed emil in the body" do
       expect(@confirm_email.body.encoded).to include(bob.unconfirmed_email)
     end
 
-      it 'has the receivers name in the body' do
+    it "has the receivers name in the body" do
       expect(@confirm_email.body.encoded).to include(bob.person.profile.first_name)
     end
 
-      it 'has the activation link in the body' do
+    it "has the activation link in the body" do
-        expect(@confirm_email.body.encoded).to include(confirm_email_url(:token => bob.confirm_email_token))
+      expect(@confirm_email.body.encoded).to include(confirm_email_url(token: bob.confirm_email_token))
-      end
     end
   end
 
-  context "limited comments" do
+  describe "hashtags" do
-    let(:commented_limited_post) {
+    it "escapes hashtags" do
-      bob.post(:status_message, to: :all, public: false,
-        text: "### Limited headline \r\n It's **really** sunny outside today")
-    }
-    let(:limited_comment) { eve.comment!(commented_limited_post, "Totally is") }
-
-    describe ".comment_on_limited_post" do
-      let(:limited_comment_mail) { Notifier.comment_on_post(bob.id, person.id, limited_comment.id).deliver_now }
-
-      it "SUBJECT: does not show limited message" do
-        expect(limited_comment_mail.subject).not_to include("Limited headline")
-      end
-
-      it "BODY: does not show limited message" do
-        expect(limited_comment_mail.body.encoded).not_to include("Limited headline")
-      end
-    end
-  end
-
-  describe 'hashtags' do
-    it 'escapes hashtags' do
       mails = Notifier.admin("#Welcome to bureaucracy!", [bob])
       expect(mails.length).to eq(1)
       mail = mails.first
@@ -399,7 +450,6 @@ describe Notifier, :type => :mailer do
 
   describe "base" do
     it "handles idn addresses" do
-      # user = FactoryGirl.create(:user, email: "ŧoo@ŧexample.com")
       bob.update_attribute(:email, "ŧoo@ŧexample.com")
       expect {
         Notifier.started_sharing(bob.id, person.id)