diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 1c008d90..033332a7 100644 --- a/app/controllers/admin/users_controller.rb +++ b/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 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6f7ed9a3..00ab93a1 100644 --- a/app/controllers/application_controller.rb +++ b/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 diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 9bf22d8c..143bc191 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -67,10 +67,7 @@ class IssuesController < ApplicationController def create @issue = @project.issues.new(params[:issue]) @issue.author = current_user - - if @issue.save && @issue.assignee != current_user - Notify.new_issue_email(@issue).deliver - end + @issue.save respond_with(@issue) end diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index b8e04f1c..5d99d1e2 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -12,10 +12,8 @@ class NotesController < ApplicationController def create @note = @project.notes.new(params[:note]) @note.author = current_user - - if @note.save - notify if params[:notify] == '1' - end + @note.notify = true if params[:notify] == '1' + @note.save 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 diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index bcbe82c2..64f17232 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -30,6 +30,14 @@ class Notify < ActionMailer::Base @commit = @project.repo.commits(note.noteable_id).first 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 @@ -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 diff --git a/app/models/issue.rb b/app/models/issue.rb index 3da595d9..aa0a2acc 100644 --- a/app/models/issue.rb +++ b/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) # diff --git a/app/models/mailer_observer.rb b/app/models/mailer_observer.rb new file mode 100644 index 00000000..159f6554 --- /dev/null +++ b/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 diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index d726d2d0..67f61235 100644 --- a/app/models/merge_request.rb +++ b/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 +# + diff --git a/app/models/note.rb b/app/models/note.rb index 946f5062..9a38cd77 100644 --- a/app/models/note.rb +++ b/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 # diff --git a/app/models/project.rb b/app/models/project.rb index 56d55fa2..d7c9515e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -247,14 +247,15 @@ end # # Table name: projects # -# id :integer not null, primary key -# name :string(255) -# path :string(255) -# description :text -# created_at :datetime -# updated_at :datetime -# private_flag :boolean default(TRUE), not null -# code :string(255) -# owner_id :integer +# id :integer not null, primary key +# name :string(255) +# path :string(255) +# description :text +# created_at :datetime +# updated_at :datetime +# private_flag :boolean default(TRUE), not null +# code :string(255) +# owner_id :integer +# default_branch :string(255) default("master"), not null # diff --git a/app/models/users_project.rb b/app/models/users_project.rb index 84d84cf3..6fd067c3 100644 --- a/app/models/users_project.rb +++ b/app/models/users_project.rb @@ -23,13 +23,12 @@ end # # Table name: users_projects # -# 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 +# id :integer not null, primary key +# user_id :integer not null +# project_id :integer not null +# created_at :datetime +# updated_at :datetime +# repo_access :integer default(0), not null +# project_access :integer default(0), not null # diff --git a/app/views/notify/changed_issue_email.html.haml b/app/views/notify/changed_issue_email.html.haml new file mode 100644 index 00000000..fe046e40 --- /dev/null +++ b/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 + diff --git a/app/views/notify/changed_merge_request_email.html.haml b/app/views/notify/changed_merge_request_email.html.haml new file mode 100644 index 00000000..403d5123 --- /dev/null +++ b/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 + diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml index c411d9dc..64c5aa61 100644 --- a/app/views/notify/new_issue_email.html.haml +++ b/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"} diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml new file mode 100644 index 00000000..57c35db5 --- /dev/null +++ b/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 + diff --git a/app/views/notify/note_merge_request_email.html.haml b/app/views/notify/note_merge_request_email.html.haml new file mode 100644 index 00000000..27854e23 --- /dev/null +++ b/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"} + diff --git a/config/application.rb b/config/application.rb index 3481c6d6..bdd5bbf3 100644 --- a/config/application.rb +++ b/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. diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index ac678633..74eb5b93 100644 --- a/spec/models/issue_spec.rb +++ b/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) # diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index e6868779..f6b4cbc8 100644 --- a/spec/models/merge_request_spec.rb +++ b/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 +# + diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index eda20a0c..a5edc408 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -168,14 +168,15 @@ end # # Table name: projects # -# id :integer not null, primary key -# name :string(255) -# path :string(255) -# description :text -# created_at :datetime -# updated_at :datetime -# private_flag :boolean default(TRUE), not null -# code :string(255) -# owner_id :integer +# id :integer not null, primary key +# name :string(255) +# path :string(255) +# description :text +# created_at :datetime +# updated_at :datetime +# private_flag :boolean default(TRUE), not null +# code :string(255) +# owner_id :integer +# default_branch :string(255) default("master"), not null # diff --git a/spec/models/users_project_spec.rb b/spec/models/users_project_spec.rb index c1539161..41e36b57 100644 --- a/spec/models/users_project_spec.rb +++ b/spec/models/users_project_spec.rb @@ -20,13 +20,12 @@ end # # Table name: users_projects # -# 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 +# id :integer not null, primary key +# user_id :integer not null +# project_id :integer not null +# created_at :datetime +# updated_at :datetime +# repo_access :integer default(0), not null +# project_access :integer default(0), not null # diff --git a/spec/requests/issues_spec.rb b/spec/requests/issues_spec.rb index 732723c1..62daf168 100644 --- a/spec/requests/issues_spec.rb +++ b/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