Merge branch 'notification_refactoring'

app/controllers/admin/users_controller.rb

@@ -27,7 +27,6 @@ class Admin::UsersController < ApplicationController
 
     respond_to do |format|
       if @admin_user.save
-        Notify.new_user_email(@admin_user, params[:user][:password]).deliver
         format.html { redirect_to [:admin, @admin_user], notice: 'User was successfully created.' }
         format.json { render json: @admin_user, status: :created, location: @admin_user }
       else

app/controllers/application_controller.rb

@@ -1,5 +1,6 @@
 class ApplicationController < ActionController::Base
   before_filter :authenticate_user!
+  before_filter :set_current_user_for_mailer
   protect_from_forgery
   helper_method :abilities, :can?
 
@@ -19,6 +20,10 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  def set_current_user_for_mailer
+    MailerObserver.current_user = current_user
+  end
+
   def abilities
     @abilities ||= Six.new
   end

app/controllers/issues_controller.rb

@@ -67,10 +67,7 @@ class IssuesController < ApplicationController
   def create
     @issue = @project.issues.new(params[:issue])
     @issue.author = current_user
-
+    @issue.save
-    if @issue.save && @issue.assignee != current_user
-      Notify.new_issue_email(@issue).deliver
-    end
 
     respond_with(@issue)
   end

app/controllers/notes_controller.rb

@@ -12,10 +12,8 @@ class NotesController < ApplicationController
   def create
     @note = @project.notes.new(params[:note])
     @note.author = current_user
-
+    @note.notify = true if params[:notify] == '1'
-    if @note.save
+    @note.save
-      notify if params[:notify] == '1'
-    end
 
     respond_to do |format|
       format.html {redirect_to :back}
@@ -35,22 +33,4 @@ class NotesController < ApplicationController
     end
   end
 
-  protected
-
-  def notify
-    @project.users.reject { |u| u.id == current_user.id } .each do |u|
-      case @note.noteable_type
-      when "Commit" then
-        Notify.note_commit_email(u, @note).deliver
-      when "Issue" then
-        Notify.note_issue_email(u, @note).deliver
-      when "MergeRequest"
-        true # someone should write email notification
-      when "Snippet"
-        true
-      else
-        Notify.note_wall_email(u, @note).deliver
-      end
-    end
-  end
 end

app/mailers/notify.rb

@@ -31,6 +31,14 @@ class Notify < ActionMailer::Base
     mail(:to => @user.email, :subject => "gitlab | #{@note.project.name} ")
   end
   
+  def note_merge_request_email(user, note)
+    @user = user
+    @note = note
+    @project = note.project
+    @merge_request = note.noteable
+    mail(:to => @user.email, :subject => "gitlab | #{@note.project.name} ")
+  end
+
   def note_issue_email(user, note)
     @user = user
     @note = note
@@ -38,4 +46,27 @@ class Notify < ActionMailer::Base
     @issue = note.noteable
     mail(:to => @user.email, :subject => "gitlab | #{@note.project.name} ")
   end
+  
+  def new_merge_request_email(merge_request)
+    @user = merge_request.assignee
+    @merge_request = merge_request
+    @project = merge_request.project
+    mail(:to => @user.email, :subject => "gitlab | #{@merge_request.title} ")
+  end
+  
+  def changed_merge_request_email(user, merge_request)
+    @user = user
+    @assignee_was ||= User.find(merge_request.assignee_id_was)
+    @merge_request = merge_request
+    @project = merge_request.project
+    mail(:to => @user.email, :subject => "gitlab | #{@merge_request.title} ")
+  end
+  
+  def changed_issue_email(user, issue)
+    @user = user
+    @assignee_was ||= User.find(issue.assignee_id_was)
+    @issue = issue
+    @project = issue.project
+    mail(:to => @user.email, :subject => "gitlab | #{@issue.title} ")
+  end
 end

app/models/issue.rb

@@ -59,5 +59,6 @@ end
 #  closed      :boolean         default(FALSE), not null
 #  position    :integer         default(0)
 #  critical    :boolean         default(FALSE), not null
+#  branch_name :string(255)
 #
 

app/models/mailer_observer.rb

@@ -0,0 +1,75 @@
+class MailerObserver < ActiveRecord::Observer
+  observe :issue, :user, :note, :merge_request
+  cattr_accessor :current_user
+
+  def after_create(model)
+    new_issue(model) if model.kind_of?(Issue)
+    new_user(model) if model.kind_of?(User)
+    new_note(model) if model.kind_of?(Note)
+    new_merge_request(model) if model.kind_of?(MergeRequest)
+  end
+
+  def after_update(model)
+    changed_merge_request(model) if model.kind_of?(MergeRequest)
+    changed_issue(model) if model.kind_of?(Issue)
+  end
+
+  protected
+
+    def new_issue(issue)
+      if issue.assignee != current_user
+        Notify.new_issue_email(issue).deliver
+      end
+    end
+
+    def new_user(user)
+      Notify.new_user_email(user, user.password).deliver
+    end
+
+    def new_note(note)
+      return unless note.notify
+      note.project.users.reject { |u| u.id == current_user.id } .each do |u|
+        case note.noteable_type
+        when "Commit" then
+          Notify.note_commit_email(u, note).deliver
+        when "Issue" then
+          Notify.note_issue_email(u, note).deliver
+        when "MergeRequest" then
+          Notify.note_merge_request_email(u, note).deliver
+        when "Snippet"
+          true
+        else
+          Notify.note_wall_email(u, note).deliver
+        end
+      end
+    end
+
+    def new_merge_request(merge_request)
+      if merge_request.assignee != current_user
+        Notify.new_merge_request_email(merge_request).deliver
+      end
+    end
+
+    def changed_merge_request(merge_request)
+      if merge_request.assignee_id_changed?
+        recipients_ids = merge_request.assignee_id_was, merge_request.assignee_id
+        recipients_ids.delete current_user.id
+
+        User.find(recipients_ids).each do |user|
+          Notify.changed_merge_request_email(user, merge_request).deliver
+        end
+      end
+    end
+
+    def changed_issue(issue)
+      if issue.assignee_id_changed?
+        recipients_ids = issue.assignee_id_was, issue.assignee_id
+        recipients_ids.delete current_user.id
+
+        User.find(recipients_ids).each do |user|
+          Notify.changed_issue_email(user, issue).deliver
+        end
+      end
+
+    end
+end

app/models/merge_request.rb

@@ -44,3 +44,19 @@ class MergeRequest < ActiveRecord::Base
     project.commit(source_branch)
   end
 end
+# == Schema Information
+#
+# Table name: merge_requests
+#
+#  id            :integer         not null, primary key
+#  target_branch :string(255)     not null
+#  source_branch :string(255)     not null
+#  project_id    :integer         not null
+#  author_id     :integer
+#  assignee_id   :integer
+#  title         :string(255)
+#  closed        :boolean         default(FALSE), not null
+#  created_at    :datetime
+#  updated_at    :datetime
+#
+

app/models/note.rb

@@ -13,6 +13,7 @@ class Note < ActiveRecord::Base
            :prefix => true
 
   attr_protected :author, :author_id
+  attr_accessor :notify
 
   validates_presence_of :project
 
@@ -35,6 +36,10 @@ class Note < ActiveRecord::Base
   scope :inc_author, includes(:author)
 
   mount_uploader :attachment, AttachmentUploader
+
+  def notify
+    @notify ||= false
+  end
 end
 # == Schema Information
 #

app/models/project.rb

@@ -256,5 +256,6 @@ end
 #  private_flag   :boolean         default(TRUE), not null
 #  code           :string(255)
 #  owner_id       :integer
+#  default_branch :string(255)     default("master"), not null
 #
 

app/models/users_project.rb

@@ -26,10 +26,9 @@ end
 #  id             :integer         not null, primary key
 #  user_id        :integer         not null
 #  project_id     :integer         not null
-#  read       :boolean         default(FALSE)
-#  write      :boolean         default(FALSE)
-#  admin      :boolean         default(FALSE)
 #  created_at     :datetime
 #  updated_at     :datetime
+#  repo_access    :integer         default(0), not null
+#  project_access :integer         default(0), not null
 #
 

app/views/notify/changed_issue_email.html.haml

@@ -0,0 +1,16 @@
+%td.content{:align => "left", :style => "font-family: Helvetica, Arial, sans-serif; padding: 20px 0 0;", :valign => "top", :width => "600"}
+  %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :style => "color: #717171; font: normal 11px Helvetica, Arial, sans-serif; margin: 0; padding: 0;", :width => "600"}
+    %tr
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+      %td{:align => "left", :style => "padding: 20px 0 0;"}
+        %h2{:style => "color:#646464; font-weight: bold; margin: 0; padding: 0; line-height: 26px; font-size: 18px; font-family: Helvetica, Arial, sans-serif; "}
+          Reassigned Issue
+          = link_to truncate(@issue.title, :length => 16), project_issue_url(@project, @issue)
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+    %tr
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+      %td{:style => "padding: 15px 0 15px;", :valign => "top"}
+        %p{:style => "color:#767676; font-weight: normal; margin: 0; padding: 0; line-height: 20px; font-size: 12px;font-family: Helvetica, Arial, sans-serif; "}
+          Assignee changed from #{@assignee_was.name} to #{@issue.assignee.name}
+      %td
+

app/views/notify/changed_merge_request_email.html.haml

@@ -0,0 +1,16 @@
+%td.content{:align => "left", :style => "font-family: Helvetica, Arial, sans-serif; padding: 20px 0 0;", :valign => "top", :width => "600"}
+  %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :style => "color: #717171; font: normal 11px Helvetica, Arial, sans-serif; margin: 0; padding: 0;", :width => "600"}
+    %tr
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+      %td{:align => "left", :style => "padding: 20px 0 0;"}
+        %h2{:style => "color:#646464; font-weight: bold; margin: 0; padding: 0; line-height: 26px; font-size: 18px; font-family: Helvetica, Arial, sans-serif; "}
+          Reassigned Merge Request
+          = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@project, @merge_request)
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+    %tr
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+      %td{:style => "padding: 15px 0 15px;", :valign => "top"}
+        %p{:style => "color:#767676; font-weight: normal; margin: 0; padding: 0; line-height: 20px; font-size: 12px;font-family: Helvetica, Arial, sans-serif; "}
+          Assignee changed from #{@assignee_was.name} to #{@merge_request.assignee.name}
+      %td
+

app/views/notify/new_issue_email.html.haml

@@ -4,7 +4,7 @@
       %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
       %td{:align => "left", :style => "padding: 20px 0 0;"}
         %h2{:style => "color:#646464; font-weight: bold; margin: 0; padding: 0; line-height: 26px; font-size: 18px; font-family: Helvetica, Arial, sans-serif; "}
-          Hi #{@user.name}! New Issue was created and assigned to you.
+          New Issue was created and assigned to you.
       %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
     %tr
       %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}

app/views/notify/new_merge_request_email.html.haml

@@ -0,0 +1,20 @@
+%td.content{:align => "left", :style => "font-family: Helvetica, Arial, sans-serif; padding: 20px 0 0;", :valign => "top", :width => "600"}
+  %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :style => "color: #717171; font: normal 11px Helvetica, Arial, sans-serif; margin: 0; padding: 0;", :width => "600"}
+    %tr
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+      %td{:align => "left", :style => "padding: 20px 0 0;"}
+        %h2{:style => "color:#646464; font-weight: bold; margin: 0; padding: 0; line-height: 26px; font-size: 18px; font-family: Helvetica, Arial, sans-serif; "}
+          New Merge Request
+          = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@project, @merge_request)
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+    %tr
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+      %td{:style => "padding: 15px 0 15px;", :valign => "top"}
+        %p{:style => "color:#767676; font-weight: normal; margin: 0; padding: 0; line-height: 20px; font-size: 12px;font-family: Helvetica, Arial, sans-serif; "}
+          Branches: #{@merge_request.source_branch} → #{@merge_request.target_branch}
+        
+        %p{:style => "color:#767676; font-weight: normal; margin: 0; padding: 0; line-height: 20px; font-size: 12px;font-family: Helvetica, Arial, sans-serif; "}
+          Asignee: #{@merge_request.author.name} → #{@merge_request.assignee.name}      
+      
+      %td
+

app/views/notify/note_merge_request_email.html.haml

@@ -0,0 +1,23 @@
+%td.content{:align => "left", :style => "font-family: Helvetica, Arial, sans-serif; padding: 20px 0 0;", :valign => "top", :width => "600"}
+  %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :style => "color: #717171; font: normal 11px Helvetica, Arial, sans-serif; margin: 0; padding: 0;", :width => "600"}
+    %tr
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+      %td{:align => "left", :style => "padding: 20px 0 0;"}
+        %h2{:style => "color:#646464; font-weight: bold; margin: 0; padding: 0; line-height: 26px; font-size: 18px; font-family: Helvetica, Arial, sans-serif; "}
+          New comment for Merge Request
+          = link_to truncate(@merge_request.title, :length => 16), project_merge_request_url(@project, @merge_request, :anchor => "note_#{@note.id}")
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+    %tr
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+      %td{:style => "padding: 15px 0 15px;", :valign => "top"}
+        %p{:style => "color:#767676; font-weight: normal; margin: 0; padding: 0; line-height: 20px; font-size: 12px;font-family: Helvetica, Arial, sans-serif; "}
+          %a{:href => "#", :style => "color: #0eb6ce; text-decoration: none;"} #{@note.author.name}
+          left next message:
+        %br
+          %table{:border => "0", :cellpadding => "0", :cellspacing => "0", :width => "558"}
+            %tr
+              %td{:valign => "top"}
+                %cite{:style => "color:#767676; font-weight: normal; margin: 0; padding: 0; line-height: 20px; font-size: 12px;font-family: Helvetica, Arial, sans-serif; "}
+                  = @note.note
+      %td{:style => "font-size: 1px; line-height: 1px;", :width => "21"}
+

config/application.rb

@@ -23,7 +23,7 @@ module Gitlab
     # config.plugins = [ :exception_notification, :ssl_requirement, :all ]
 
     # Activate observers that should always be running.
-    # config.active_record.observers = :cacher, :garbage_collector, :forum_observer
+    config.active_record.observers = :mailer_observer
 
     # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone.
     # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC.

spec/models/issue_spec.rb

@@ -39,5 +39,6 @@ end
 #  closed      :boolean         default(FALSE), not null
 #  position    :integer         default(0)
 #  critical    :boolean         default(FALSE), not null
+#  branch_name :string(255)
 #
 

spec/models/merge_request_spec.rb

@@ -26,3 +26,19 @@ describe MergeRequest do
                       :assignee => Factory(:user),
                       :project => Factory.create(:project)).should be_valid }
 end
+# == Schema Information
+#
+# Table name: merge_requests
+#
+#  id            :integer         not null, primary key
+#  target_branch :string(255)     not null
+#  source_branch :string(255)     not null
+#  project_id    :integer         not null
+#  author_id     :integer
+#  assignee_id   :integer
+#  title         :string(255)
+#  closed        :boolean         default(FALSE), not null
+#  created_at    :datetime
+#  updated_at    :datetime
+#
+

spec/models/project_spec.rb

@@ -177,5 +177,6 @@ end
 #  private_flag   :boolean         default(TRUE), not null
 #  code           :string(255)
 #  owner_id       :integer
+#  default_branch :string(255)     default("master"), not null
 #
 

spec/models/users_project_spec.rb

@@ -23,10 +23,9 @@ end
 #  id             :integer         not null, primary key
 #  user_id        :integer         not null
 #  project_id     :integer         not null
-#  read       :boolean         default(FALSE)
-#  write      :boolean         default(FALSE)
-#  admin      :boolean         default(FALSE)
 #  created_at     :datetime
 #  updated_at     :datetime
+#  repo_access    :integer         default(0), not null
+#  project_access :integer         default(0), not null
 #
 

spec/requests/issues_spec.rb

@@ -147,13 +147,12 @@ describe "Issues" do
           click_button "Save"
         end
 
-        it "should send valid email to user with email & password" do
+        it "should send valid email to user" do
           click_button "Save"
           issue = Issue.last
           email = ActionMailer::Base.deliveries.last
           email.subject.should have_content("New Issue was created")
           email.body.should have_content(issue.title)
-          email.body.should have_content(issue.assignee.name)
         end
 
       end