Merge branch 'refactor_notify' of https://github.com/tsigo/gitlabhq into tsigo-refactor_notify

app/mailers/notify.rb

@@ -12,74 +12,102 @@ class Notify < ActionMailer::Base
   def new_user_email(user_id, password)
     @user = User.find(user_id)
     @password = password
-    mail(to: @user.email, subject: "gitlab | Account was created for you")
+    mail(to: @user.email, subject: subject("Account was created for you"))
   end
 
   def new_issue_email(issue_id)
     @issue = Issue.find(issue_id)
     @project = @issue.project
-    mail(to: @issue.assignee_email, subject: "gitlab | new issue ##{@issue.id} | #{@issue.title} | #{@project.name}")
+    mail(to: @issue.assignee_email, subject: subject("new issue ##{@issue.id}", @issue.title))
   end
 
   def note_wall_email(recipient_id, note_id)
-    recipient = User.find(recipient_id)
     @note = Note.find(note_id)
     @project = @note.project
-    mail(to: recipient.email, subject: "gitlab | #{@project.name}")
+    mail(to: recipient(recipient_id), subject: subject)
   end
 
   def note_commit_email(recipient_id, note_id)
-    recipient = User.find(recipient_id)
     @note = Note.find(note_id)
     @commit = @note.target
     @commit = CommitDecorator.decorate(@commit)
     @project = @note.project
-    mail(to: recipient.email, subject: "gitlab | note for commit #{@commit.short_id} | #{@commit.title} | #{@project.name}")
+    mail(to: recipient(recipient_id), subject: subject("note for commit #{@commit.short_id}", @commit.title))
   end
 
   def note_merge_request_email(recipient_id, note_id)
-    recipient = User.find(recipient_id)
     @note = Note.find(note_id)
     @merge_request = @note.noteable
     @project = @note.project
-    mail(to: recipient.email, subject: "gitlab | note for merge request !#{@merge_request.id} | #{@project.name}")
+    mail(to: recipient(recipient_id), subject: subject("note for merge request !#{@merge_request.id}"))
   end
 
   def note_issue_email(recipient_id, note_id)
-    recipient = User.find(recipient_id)
     @note = Note.find(note_id)
     @issue = @note.noteable
     @project = @note.project
-    mail(to: recipient.email, subject: "gitlab | note for issue ##{@issue.id} | #{@project.name}")
+    mail(to: recipient(recipient_id), subject: subject("note for issue ##{@issue.id}"))
   end
 
   def note_wiki_email(recipient_id, note_id)
-    recipient = User.find(recipient_id)
     @note = Note.find(note_id)
     @wiki = @note.noteable
     @project = @note.project
-    mail(to: recipient.email, subject: "gitlab | note for wiki | #{@project.name}")
+    mail(to: recipient(recipient_id), subject: subject("note for wiki"))
   end
 
   def new_merge_request_email(merge_request_id)
     @merge_request = MergeRequest.find(merge_request_id)
     @project = @merge_request.project
-    mail(to: @merge_request.assignee_email, subject: "gitlab | new merge request !#{@merge_request.id} | #{@merge_request.title} | #{@project.name}")
+    mail(to: @merge_request.assignee_email, subject: subject("new merge request !#{@merge_request.id}", @merge_request.title))
   end
 
   def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id)
-    recipient = User.find(recipient_id)
     @merge_request = MergeRequest.find(merge_request_id)
     @previous_assignee ||= User.find(previous_assignee_id)
     @project = @merge_request.project
-    mail(to: recipient.email, subject: "gitlab | changed merge request !#{@merge_request.id} | #{@merge_request.title} | #{@project.name}")
+    mail(to: recipient(recipient_id), subject: subject("changed merge request !#{@merge_request.id}", @merge_request.title))
   end
 
   def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id)
-    recipient = User.find(recipient_id)
     @issue = Issue.find(issue_id)
     @previous_assignee ||= User.find(previous_assignee_id)
     @project = @issue.project
-    mail(to: recipient.email, subject: "gitlab | changed issue ##{@issue.id} | #{@issue.title} | #{@project.name}")
+    mail(to: recipient(recipient_id), subject: subject("changed issue ##{@issue.id}", @issue.title))
+  end
+
+  private
+
+  # Look up a User by their ID and return their email address
+  #
+  # recipient_id - User ID
+  #
+  # Returns a String containing the User's email address.
+  def recipient(recipient_id)
+    if recipient = User.find(recipient_id)
+      recipient.email
+    end
+  end
+
+  # Formats arguments into a String suitable for use as an email subject
+  #
+  # extra - Extra Strings to be inserted into the subject
+  #
+  # Examples
+  #
+  #   >> subject('Lorem ipsum')
+  #   => "gitlab | Lorem ipsum"
+  #
+  #   # Automatically inserts Project name when @project is set
+  #   >> @project = Project.last
+  #   => #<Project id: 1, name: "Ruby on Rails", path: "ruby_on_rails", ...>
+  #   >> subject('Lorem ipsum')
+  #   => "gitlab | Lorem ipsum | Ruby on Rails"
+  #
+  #   # Accepts multiple arguments
+  #   >> subject('Lorem ipsum', 'Dolor sit amet')
+  #   => "gitlab | Lorem ipsum | Dolor sit amet"
+  def subject(*extra)
+    "gitlab | " << extra.join(' | ') << (@project ? " | #{@project.name}" : "")
   end
 end

spec/mailers/notify_spec.rb

@@ -24,7 +24,7 @@ describe Notify do
     end
 
     it 'has the correct subject' do
-      should have_subject /Account was created for you/
+      should have_subject /^gitlab \| Account was created for you$/
     end
 
     it 'contains the new user\'s login name' do
@@ -60,7 +60,7 @@ describe Notify do
           it_behaves_like 'an assignee email'
 
           it 'has the correct subject' do
-            should have_subject /new issue ##{issue.id}/
+            should have_subject /new issue ##{issue.id} \| #{issue.title} \| #{project.name}/
           end
 
           it 'contains a link to the new issue' do
@@ -76,7 +76,7 @@ describe Notify do
           it_behaves_like 'a multiple recipients email'
 
           it 'has the correct subject' do
-            should have_subject /changed issue/
+            should have_subject /changed issue ##{issue.id} \| #{issue.title}/
           end
 
           it 'contains the name of the previous assignee' do