PMs and comments contain author name in "From" header

refactor notifier

app/helpers/notifier_helper.rb

@@ -19,7 +19,7 @@ module NotifierHelper
   # @return [String] The truncated and formatted comment.
   def comment_message(comment, opts={})
     opts[:length] ||= 600
-    text = truncate(@comment.text, :length => opts[:length])
+    text = truncate(comment.text, :length => opts[:length])
     text = process_newlines(text) if opts[:process_newlines]
     text
   end

app/mailers/notification_mailers/also_commented.rb

@@ -0,0 +1,21 @@
+module NotificationMailers
+  class AlsoCommented < NotificationMailers::Base
+    include ActionView::Helpers::TextHelper
+
+    attr_accessor :comment
+
+    def set_headers(comment_id)
+      @comment  = Comment.find_by_id(comment_id)
+
+      if mail?
+        @headers[:from] = "[#{@comment.author.name} (Diaspora)] <#{AppConfig[:smtp_sender_address]}>"
+        @headers[:subject] = truncate(@comment.parent.comment_email_subject, :length => TRUNCATION_LEN)
+        @headers[:subject] = "Re: #{@headers[:subject]}"
+      end
+    end
+
+    def mail?
+      @recipient && @sender && @comment
+    end
+  end
+end

app/mailers/notification_mailers/base.rb

@@ -0,0 +1,52 @@
+module NotificationMailers
+  TRUNCATION_LEN = 70
+
+  class Base
+    attr_accessor :recipient, :recipient_name, :sender
+
+    def initialize(recipient_id, sender_id=nil, *args)
+      @headers = {}
+      @recipient = User.find_by_id(recipient_id)
+      @recipient_name = @recipient.profile.first_name
+      @sender = Person.find_by_id(sender_id) if sender_id.present?
+
+      log_mail(recipient_id, sender_id, self.class.to_s.underscore)
+
+      with_locale do
+        set_headers(*args)
+      end
+    end
+
+    def headers
+      default_headers.merge(@headers)
+    end
+
+    private
+
+    def default_headers
+      headers = {
+        :host => "#{AppConfig[:pod_uri]}",
+        :to => "\"#{@recipient.name}\" <#{@recipient.email}>"
+      }
+
+      headers[:from] = "\"#{@sender.name} (Diaspora)\" <#{AppConfig[:smtp_sender_address]}>" if @sender.present?
+
+      headers
+    end
+
+    def with_locale(&block)
+      I18n.with_locale(@recipient.language, &block)
+    end
+
+    def log_mail(recipient_id, sender_id, type)
+      log_string = "event=mail mail_type=#{type} recipient_id=#{recipient_id} sender_id=#{sender_id}"
+      if @recipient && @sender
+        log_string << "models_found=true sender_handle=#{@sender.diaspora_handle} recipient_handle=#{@recipient.diaspora_handle}"
+      else
+        log_string << "models_found=false"
+      end
+
+      Rails.logger.info(log_string)
+    end
+  end
+end

app/mailers/notification_mailers/comment_on_post.rb

@@ -0,0 +1,15 @@
+module NotificationMailers
+  class CommentOnPost < NotificationMailers::Base
+    include ActionView::Helpers::TextHelper
+
+    attr_accessor :comment
+
+    def set_headers(comment_id)
+      @comment = Comment.find(comment_id)
+
+      @headers[:from] = "[#{@comment.author.name} (Diaspora)] <#{AppConfig[:smtp_sender_address]}>"
+      @headers[:subject] = truncate(@comment.parent.comment_email_subject, :length => TRUNCATION_LEN)
+      @headers[:subject] = "Re: #{@headers[:subject]}"
+    end
+  end
+end

app/mailers/notification_mailers/confirm_email.rb

@@ -0,0 +1,8 @@
+module NotificationMailers
+  class ConfirmEmail < NotificationMailers::Base
+    def set_headers
+      @headers[:to] = "\"#{recipient_name}\" <#{@recipient.unconfirmed_email}>"
+      @headers[:subject] = I18n.t('notifier.confirm_email.subject', :unconfirmed_email => @recipient.unconfirmed_email)
+    end
+  end
+end

app/mailers/notification_mailers/liked.rb

@@ -0,0 +1,11 @@
+module NotificationMailers
+  class Liked < NotificationMailers::Base
+    attr_accessor :like
+
+    def set_headers(like_id)
+      @like = Like.find(like_id)
+
+      @headers[:subject] = I18n.t('notifier.liked.liked', :name => @sender.name)
+    end
+  end
+end

app/mailers/notification_mailers/mentioned.rb

@@ -0,0 +1,11 @@
+module NotificationMailers
+  class Mentioned < NotificationMailers::Base
+    attr_accessor :post
+
+    def set_headers(target_id)
+      @post = Mention.find_by_id(target_id).post
+
+      @headers[:subject] = I18n.t('notifier.mentioned.subject', :name => @sender.name)
+    end
+  end
+end

app/mailers/notification_mailers/private_message.rb

@@ -0,0 +1,15 @@
+module NotificationMailers
+  class PrivateMessage < NotificationMailers::Base
+    attr_accessor :message, :conversation, :participants
+
+    def set_headers(message_id)
+      @message  = Message.find_by_id(message_id)
+      @conversation = @message.conversation
+      @participants = @conversation.participants
+
+      @headers[:from] = "[#{@message.author.name} (Diaspora)] <#{AppConfig[:smtp_sender_address]}>"
+      @headers[:subject] = @conversation.subject.strip
+      @headers[:subject] = "Re: #{@headers[:subject]}" if @conversation.messages.size > 1
+    end
+  end
+end

app/mailers/notification_mailers/reshared.rb

@@ -0,0 +1,11 @@
+module NotificationMailers
+  class Reshared < NotificationMailers::Base
+    attr_accessor :reshare
+
+    def set_headers(reshare_id)
+      @reshare = Reshare.find(reshare_id)
+
+      @headers[:subject] = I18n.t('notifier.reshared.reshared', :name => @sender.name)
+    end
+  end
+end

app/mailers/notification_mailers/started_sharing.rb

@@ -0,0 +1,7 @@
+module NotificationMailers
+  class StartedSharing < NotificationMailers::Base
+    def set_headers
+      @headers[:subject] = I18n.t('notifier.started_sharing.subject', :name => @sender.name)
+    end
+  end
+end

app/mailers/notifier.rb

@@ -5,11 +5,8 @@ class Notifier < ActionMailer::Base
 
   default :from => AppConfig[:smtp_sender_address]
 
-  include ActionView::Helpers::TextHelper
   include NotifierHelper
 
-  TRUNCATION_LEN = 70
-
   def self.admin(string, recipients, opts = {})
     mails = []
     recipients.each do |rec|
@@ -27,133 +24,71 @@ class Notifier < ActionMailer::Base
   end
 
   def started_sharing(recipient_id, sender_id)
-    @receiver = User.find_by_id(recipient_id)
-    @sender = Person.find_by_id(sender_id)
+    @notification = NotificationMailers::StartedSharing.new(recipient_id, sender_id)
 
-    log_mail(recipient_id, sender_id, 'started_sharing')
-
-    I18n.with_locale(@receiver.language) do
-      mail(:to => "\"#{@receiver.name}\" <#{@receiver.email}>",
-           :subject => I18n.t('notifier.started_sharing.subject', :name => @sender.name), :host => AppConfig[:pod_uri].host)
+    with_recipient_locale do
+      mail(@notification.headers)
     end
   end
 
   def liked(recipient_id, sender_id, like_id)
-    @receiver = User.find_by_id(recipient_id)
-    @sender = Person.find_by_id(sender_id)
-    @like = Like.find(like_id)
+    @notification = NotificationMailers::Liked.new(recipient_id, sender_id, like_id)
 
-    log_mail(recipient_id, sender_id, 'liked')
-
-    I18n.with_locale(@receiver.language) do
-      mail(:to => "\"#{@receiver.name}\" <#{@receiver.email}>",
-           :subject => I18n.t('notifier.liked.liked', :name => @sender.name), :host => AppConfig[:pod_uri].host)
+    with_recipient_locale do
+      mail(@notification.headers)
     end
   end
 
   def reshared(recipient_id, sender_id, reshare_id)
-    @receiver = User.find_by_id(recipient_id)
-    @sender = Person.find_by_id(sender_id)
-    @reshare = Reshare.find(reshare_id)
+    @notification = NotificationMailers::Reshared.new(recipient_id, sender_id, reshare_id)
 
-    log_mail(recipient_id, sender_id, 'reshared')
-
-    I18n.with_locale(@receiver.language) do
-      mail(:to => "\"#{@receiver.name}\" <#{@receiver.email}>",
-           :subject => I18n.t('notifier.reshared.reshared', :name => @sender.name), :host => AppConfig[:pod_uri].host)
+    with_recipient_locale do
+      mail(@notification.headers)
     end
   end
 
   def mentioned(recipient_id, sender_id, target_id)
-    @receiver = User.find_by_id(recipient_id)
-    @sender = Person.find_by_id(sender_id)
-    @post = Mention.find_by_id(target_id).post
+    @notification = NotificationMailers::Mentioned.new(recipient_id, sender_id, target_id)
 
-    log_mail(recipient_id, sender_id, 'mentioned')
-
-    I18n.with_locale(@receiver.language) do
-      mail(:to => "\"#{@receiver.name}\" <#{@receiver.email}>",
-           :subject => I18n.t('notifier.mentioned.subject', :name => @sender.name), :host => AppConfig[:pod_uri].host)
+    with_recipient_locale do
+      mail(@notification.headers)
     end
   end
 
   def comment_on_post(recipient_id, sender_id, comment_id)
-    @receiver = User.find_by_id(recipient_id)
-    @sender   = Person.find_by_id(sender_id)
-    @comment  = Comment.find_by_id(comment_id)
+    @notification = NotificationMailers::CommentOnPost.new(recipient_id, sender_id, comment_id)
 
-    log_mail(recipient_id, sender_id, 'comment_on_post')
-
-    I18n.with_locale(@receiver.language) do
-      mail(:from => "\"#{@sender.name} (Diaspora)\" <#{AppConfig[:smtp_sender_address]}>",
-           :to => "\"#{@receiver.name}\" <#{@receiver.email}>",
-           :subject => "Re: #{comment_email_subject}")
+    with_recipient_locale do
+      mail(@notification.headers)
     end
   end
 
   def also_commented(recipient_id, sender_id, comment_id)
-    @receiver = User.find_by_id(recipient_id)
-    @sender   = Person.find_by_id(sender_id)
-    @comment  = Comment.find_by_id(comment_id)
+    @notification = NotificationMailers::AlsoCommented.new(recipient_id, sender_id, comment_id)
 
-    if @receiver && @sender && @comment
-      @post_author_name = @comment.post.author.name
-
-
-      log_mail(recipient_id, sender_id, 'comment_on_post')
-
-      I18n.with_locale(@receiver.language) do
-        mail(:from => "\"#{@sender.name} (Diaspora)\" <#{AppConfig[:smtp_sender_address]}>",
-             :to => "\"#{@receiver.name}\" <#{@receiver.email}>",
-             :subject => "Re: #{comment_email_subject}")
-      end
+    with_recipient_locale do
+      mail(@notification.headers) if @notification.mail?
     end
   end
 
-  def comment_email_subject
-    truncate(@comment.parent.comment_email_subject, :length => TRUNCATION_LEN)
-  end
-
   def private_message(recipient_id, sender_id, message_id)
-    @receiver = User.find_by_id(recipient_id)
-    @sender   = Person.find_by_id(sender_id)
-    @message  = Message.find_by_id(message_id)
-    @conversation = @message.conversation
-    @participants = @conversation.participants
+    @notification = NotificationMailers::PrivateMessage.new(recipient_id, sender_id, message_id)
 
-
-    log_mail(recipient_id, sender_id, 'private_message')
-
-    subject = @conversation.subject.strip
-    subject = "Re: #{subject}" if @conversation.messages.size > 1
-
-
-    I18n.with_locale(@receiver.language) do
-      mail(:from => "\"#{@sender.name} (Diaspora)\" <#{AppConfig[:smtp_sender_address]}>",
-           :to => "\"#{@receiver.name}\" <#{@receiver.email}>",
-           :subject => subject)
+    with_recipient_locale do
+      mail(@notification.headers)
     end
   end
 
-  def confirm_email(receiver_id)
-    @receiver = User.find_by_id(receiver_id)
+  def confirm_email(recipient_id)
+    @notification = NotificationMailers::ConfirmEmail.new(recipient_id)
 
-    I18n.with_locale(@receiver.language) do
-      mail(:to => "\"#{@receiver.name}\" <#{@receiver.unconfirmed_email}>",
-           :subject => I18n.t('notifier.confirm_email.subject', :unconfirmed_email => @receiver.unconfirmed_email),
-           :host => AppConfig[:pod_uri].host)
+    with_recipient_locale do
+      mail(@notification.headers)
     end
   end
 
-
   private
-  def log_mail recipient_id, sender_id, type
-    log_string = "event=mail mail_type=#{type} recipient_id=#{recipient_id} sender_id=#{sender_id}"
-    if @receiver && @sender
-      log_string << "models_found=true sender_handle=#{@sender.diaspora_handle} recipient_handle=#{@receiver.diaspora_handle}"
-    else
-      log_string << "models_found=false"
-    end
-    Rails.logger.info log_string
+  def with_recipient_locale(&block)
+    I18n.with_locale(@notification.recipient.language, &block)
   end
 end

app/views/notifier/also_commented.html.haml

@@ -1,4 +1,4 @@
 %p
-  = comment_message(@comment.text, :process_newlines => true)
+  = comment_message(@notification.comment, :process_newlines => true)
 %p
-  = link_to t('notifier.comment_on_post.reply', :name => @comment.post.author.first_name), post_url(@comment.post)
+  = link_to t('notifier.comment_on_post.reply', :name => @notification.comment.post.author.first_name), post_url(@notification.comment.post)

app/views/notifier/also_commented.text.haml

@@ -1 +1 @@
-!= comment_message(@comment.text)
+!= comment_message(@notification.comment)

app/views/notifier/comment_on_post.html.haml

@@ -1,4 +1,4 @@
 %p
-  = comment_message(@comment.text, :process_newlines => true)
+  = comment_message(@notification.comment, :process_newlines => true)
 %p
-  = link_to t('notifier.comment_on_post.reply', :name => @comment.post.author.name), post_url(@comment.post)
+  = link_to t('notifier.comment_on_post.reply', :name => @notification.comment.post.author.name), post_url(@notification.comment.post)

app/views/notifier/comment_on_post.text.haml

@@ -1 +1 @@
-!= comment_message(@comment.text)
+!= comment_message(@notification.comment)

app/views/notifier/confirm_email.html.haml

@@ -1,6 +1,7 @@
 %p
-  = t('notifier.hello', :name => @receiver.profile.first_name)
+  = t('notifier.hello', :name => @notification.recipient_name)
 %p
-  != t('notifier.confirm_email.click_link', :unconfirmed_email => @receiver.unconfirmed_email)
+  != t('notifier.confirm_email.click_link', :unconfirmed_email => @notification.recipient.unconfirmed_email)
   %br
-  = link_to confirm_email_url(:token => @receiver.confirm_email_token), confirm_email_url(:token => @receiver.confirm_email_token)
+  = link_to confirm_email_url(:token => @notification.recipient.confirm_email_token),
+            confirm_email_url(:token => @notification.recipient.confirm_email_token)

app/views/notifier/confirm_email.text.haml

@@ -1,4 +1,4 @@
-!= t('notifier.hello', :name => @receiver.profile.first_name)
+!= t('notifier.hello', :name => @notification.recipient_name)
 
-!= t('notifier.confirm_email.click_link', :unconfirmed_email => @receiver.unconfirmed_email)
-!= confirm_email_url(:token => @receiver.confirm_email_token)
+!= t('notifier.confirm_email.click_link', :unconfirmed_email => @notification.recipient.unconfirmed_email)
+!= confirm_email_url(:token => @notification.recipient.confirm_email_token)

app/views/notifier/liked.html.haml

@@ -1,8 +1,8 @@
 %p
-  = "#{t('.liked', :name => "#{@sender.name}")}:"
+  = "#{t('.liked', :name => "#{@notification.sender.name}")}:"
 
 %p{:style => "font-style:italic;color:#666"}
-  = post_message(@like.target, :process_newlines => true)
+  = post_message(@notification.like.target, :process_newlines => true)
 
 %p
-  = link_to t('.view_post'), post_url(@like.target)
+  = link_to t('.view_post'), post_url(@notification.like.target)

app/views/notifier/liked.text.haml

@@ -1,3 +1,3 @@
-!= "#{t('notifier.liked.liked', :name => "#{@sender.name}")}:"
-!=post_message(@like.target)
+!= "#{t('notifier.liked.liked', :name => "#{@notification.sender.name}")}:"
+!=post_message(@notification.like.target)
 

app/views/notifier/mentioned.html.haml

@@ -1,4 +1,4 @@
 %p
-  = post_message(@post, :process_newlines => true, :length => 600)
+  = post_message(@notification.post, :process_newlines => true, :length => 600)
 %p
-  = link_to t('notifier.comment_on_post.reply', :name => @post.author.name), post_url(@post)
+  = link_to t('notifier.comment_on_post.reply', :name => @notification.post.author.name), post_url(@notification.post)

app/views/notifier/mentioned.text.haml

@@ -1 +1 @@
-!= post_message(@post, :process_newlines => true, :length => 600)
+!= post_message(@notification.post, :process_newlines => true, :length => 600)

app/views/notifier/private_message.html.haml

@@ -1,4 +1,4 @@
 %p
-  = post_message(@message, :process_newlines => true, :length => 2000)
+  = post_message(@notification.message, :process_newlines => true, :length => 2000)
 %p
-  = link_to t('.reply_to_or_view'), conversations_url(:conversation_id => @conversation)
+  = link_to t('.reply_to_or_view'), conversations_url(:conversation_id => @notification.conversation)

app/views/notifier/private_message.text.haml

@@ -1 +1 @@
-!= @message.text
+!= @notification.message.text

app/views/notifier/reshared.html.haml

@@ -1,8 +1,8 @@
 %p
-  = "#{t('.reshared', :name => "#{@sender.name}")}:"
+  = "#{t('.reshared', :name => "#{@notification.sender.name}")}:"
 
 %p{:style => "font-style:italic;color:#666"}
-  = post_message(@reshare.root, :process_newlines => true)
+  = post_message(@notification.reshare.root, :process_newlines => true)
 
 %p
-  = link_to t('.view_post'), post_url(@reshare)
+  = link_to t('.view_post'), post_url(@notification.reshare)

app/views/notifier/reshared.text.haml

@@ -1,3 +1,3 @@
-!= "#{t('notifier.reshared.reshared', :name => "#{@sender.name}")}:"
-!=post_message(@reshare.root)
+!= "#{t('notifier.reshared.reshared', :name => "#{@notification.sender.name}")}:"
+!=post_message(@notification.reshare.root)
 

app/views/notifier/started_sharing.html.haml

@@ -1,5 +1,5 @@
 %p
-  = @sender.name
+  = @notification.sender.name
   = t('.sharing')
 %p
-  = link_to t('.view_profile', :name => @sender.first_name), person_url(@sender)
+  = link_to t('.view_profile', :name => @notification.sender.first_name), person_url(@notification.sender)

app/views/notifier/started_sharing.text.haml

@@ -1,4 +1,4 @@
-!= "#{@sender.name}"
+!= "#{@notification.sender.name}"
 != t('notifier.started_sharing.sharing')
-!= t('.view_profile', :name => @sender.first_name)
-!= person_url(@sender)
+!= t('.view_profile', :name => @notification.sender.first_name)
+!= person_url(@notification.sender)

spec/mailers/notifier_spec.rb

@@ -172,8 +172,7 @@ describe Notifier do
     end
 
     it "FROM: contains the sender's name" do
-      pending
-      @mail.from.should == "\"#{person.name} (Diaspora)\" <#{AppConfig[:smtp_sender_address]}>"
+      @mail.from.should == "[#{@cnv.author.name} (Diaspora)] <#{AppConfig[:smtp_sender_address]}>"
     end
 
     it 'SUBJECT: has a snippet of the post contents' do
@@ -208,8 +207,7 @@ describe Notifier do
       end
 
       it "FROM: contains the sender's name" do
-        pending
-        comment_mail.from.should == "\"#{person.name} (Diaspora)\" <#{AppConfig[:smtp_sender_address]}>"
+        comment_mail.from.should == "[#{eve.name} (Diaspora)] <#{AppConfig[:smtp_sender_address]}>"
       end
 
       it 'SUBJECT: has a snippet of the post contents' do
@@ -243,15 +241,14 @@ describe Notifier do
     end
 
     describe ".also_commented" do
-      let(:comment_mail) {Notifier.also_commented(bob.id, person.id, comment.id)}
+      let(:comment_mail) { Notifier.also_commented(bob.id, person.id, comment.id) }
 
       it 'TO: goes to the right person' do
         comment_mail.to.should == [bob.email]
       end
 
       it 'FROM: has the name of person commenting as the sender' do
-        pending
-        comment_mail.from.should == "\"#{person.name} (Diaspora)\" <#{AppConfig[:smtp_sender_address]}>"
+        comment_mail.from.should == "[#{eve.name} (Diaspora)] <#{AppConfig[:smtp_sender_address]}>"
       end
 
       it 'SUBJECT: has a snippet of the post contents' do