Commit 7713f7fefb4601b7ddede29d61fbf80e2b2929b8
1 parent
f7859ec1
Exists in
master
and in
4 other branches
Notification refactoring
Showing
7 changed files
with
55 additions
and
28 deletions
Show diff stats
app/controllers/admin/users_controller.rb
| @@ -27,7 +27,6 @@ class Admin::UsersController < ApplicationController | @@ -27,7 +27,6 @@ class Admin::UsersController < ApplicationController | ||
| 27 | 27 | ||
| 28 | respond_to do |format| | 28 | respond_to do |format| |
| 29 | if @admin_user.save | 29 | if @admin_user.save |
| 30 | - Notify.new_user_email(@admin_user, params[:user][:password]).deliver | ||
| 31 | format.html { redirect_to [:admin, @admin_user], notice: 'User was successfully created.' } | 30 | format.html { redirect_to [:admin, @admin_user], notice: 'User was successfully created.' } |
| 32 | format.json { render json: @admin_user, status: :created, location: @admin_user } | 31 | format.json { render json: @admin_user, status: :created, location: @admin_user } |
| 33 | else | 32 | else |
app/controllers/application_controller.rb
| 1 | class ApplicationController < ActionController::Base | 1 | class ApplicationController < ActionController::Base |
| 2 | before_filter :authenticate_user! | 2 | before_filter :authenticate_user! |
| 3 | + before_filter :set_current_user_for_mailer | ||
| 3 | protect_from_forgery | 4 | protect_from_forgery |
| 4 | helper_method :abilities, :can? | 5 | helper_method :abilities, :can? |
| 5 | 6 | ||
| @@ -19,6 +20,10 @@ class ApplicationController < ActionController::Base | @@ -19,6 +20,10 @@ class ApplicationController < ActionController::Base | ||
| 19 | end | 20 | end |
| 20 | end | 21 | end |
| 21 | 22 | ||
| 23 | + def set_current_user_for_mailer | ||
| 24 | + MailerObserver.current_user = current_user | ||
| 25 | + end | ||
| 26 | + | ||
| 22 | def abilities | 27 | def abilities |
| 23 | @abilities ||= Six.new | 28 | @abilities ||= Six.new |
| 24 | end | 29 | end |
app/controllers/issues_controller.rb
| @@ -67,10 +67,7 @@ class IssuesController < ApplicationController | @@ -67,10 +67,7 @@ class IssuesController < ApplicationController | ||
| 67 | def create | 67 | def create |
| 68 | @issue = @project.issues.new(params[:issue]) | 68 | @issue = @project.issues.new(params[:issue]) |
| 69 | @issue.author = current_user | 69 | @issue.author = current_user |
| 70 | - | ||
| 71 | - if @issue.save && @issue.assignee != current_user | ||
| 72 | - Notify.new_issue_email(@issue).deliver | ||
| 73 | - end | 70 | + @issue.save |
| 74 | 71 | ||
| 75 | respond_with(@issue) | 72 | respond_with(@issue) |
| 76 | end | 73 | end |
app/controllers/notes_controller.rb
| @@ -12,10 +12,8 @@ class NotesController < ApplicationController | @@ -12,10 +12,8 @@ class NotesController < ApplicationController | ||
| 12 | def create | 12 | def create |
| 13 | @note = @project.notes.new(params[:note]) | 13 | @note = @project.notes.new(params[:note]) |
| 14 | @note.author = current_user | 14 | @note.author = current_user |
| 15 | - | ||
| 16 | - if @note.save | ||
| 17 | - notify if params[:notify] == '1' | ||
| 18 | - end | 15 | + @note.notify = true if params[:notify] == '1' |
| 16 | + @note.save | ||
| 19 | 17 | ||
| 20 | respond_to do |format| | 18 | respond_to do |format| |
| 21 | format.html {redirect_to :back} | 19 | format.html {redirect_to :back} |
| @@ -35,22 +33,4 @@ class NotesController < ApplicationController | @@ -35,22 +33,4 @@ class NotesController < ApplicationController | ||
| 35 | end | 33 | end |
| 36 | end | 34 | end |
| 37 | 35 | ||
| 38 | - protected | ||
| 39 | - | ||
| 40 | - def notify | ||
| 41 | - @project.users.reject { |u| u.id == current_user.id } .each do |u| | ||
| 42 | - case @note.noteable_type | ||
| 43 | - when "Commit" then | ||
| 44 | - Notify.note_commit_email(u, @note).deliver | ||
| 45 | - when "Issue" then | ||
| 46 | - Notify.note_issue_email(u, @note).deliver | ||
| 47 | - when "MergeRequest" | ||
| 48 | - true # someone should write email notification | ||
| 49 | - when "Snippet" | ||
| 50 | - true | ||
| 51 | - else | ||
| 52 | - Notify.note_wall_email(u, @note).deliver | ||
| 53 | - end | ||
| 54 | - end | ||
| 55 | - end | ||
| 56 | end | 36 | end |
| @@ -0,0 +1,41 @@ | @@ -0,0 +1,41 @@ | ||
| 1 | +class MailerObserver < ActiveRecord::Observer | ||
| 2 | + observe :issue, :user, :note, :snippet | ||
| 3 | + cattr_accessor :current_user | ||
| 4 | + | ||
| 5 | + def after_create(model) | ||
| 6 | + new_issue(model) if model.kind_of?(Issue) | ||
| 7 | + new_user(model) if model.kind_of?(User) | ||
| 8 | + new_note(model) if model.kind_of?(Note) | ||
| 9 | + end | ||
| 10 | + | ||
| 11 | + protected | ||
| 12 | + | ||
| 13 | + def new_issue(issue) | ||
| 14 | + if issue.assignee != current_user | ||
| 15 | + Notify.new_issue_email(issue).deliver | ||
| 16 | + end | ||
| 17 | + end | ||
| 18 | + | ||
| 19 | + def new_user(user) | ||
| 20 | + Notify.new_user_email(user, user.password).deliver | ||
| 21 | + end | ||
| 22 | + | ||
| 23 | + def new_note(note) | ||
| 24 | + return unless note.notify | ||
| 25 | + note.project.users.reject { |u| u.id == current_user.id } .each do |u| | ||
| 26 | + case note.noteable_type | ||
| 27 | + when "Commit" then | ||
| 28 | + Notify.note_commit_email(u, note).deliver | ||
| 29 | + when "Issue" then | ||
| 30 | + Notify.note_issue_email(u, note).deliver | ||
| 31 | + when "MergeRequest" | ||
| 32 | + true # someone should write email notification | ||
| 33 | + when "Snippet" | ||
| 34 | + true | ||
| 35 | + else | ||
| 36 | + Notify.note_wall_email(u, note).deliver | ||
| 37 | + end | ||
| 38 | + end | ||
| 39 | + end | ||
| 40 | + | ||
| 41 | +end |
app/models/note.rb
| @@ -13,6 +13,7 @@ class Note < ActiveRecord::Base | @@ -13,6 +13,7 @@ class Note < ActiveRecord::Base | ||
| 13 | :prefix => true | 13 | :prefix => true |
| 14 | 14 | ||
| 15 | attr_protected :author, :author_id | 15 | attr_protected :author, :author_id |
| 16 | + attr_accessor :notify | ||
| 16 | 17 | ||
| 17 | validates_presence_of :project | 18 | validates_presence_of :project |
| 18 | 19 | ||
| @@ -35,6 +36,10 @@ class Note < ActiveRecord::Base | @@ -35,6 +36,10 @@ class Note < ActiveRecord::Base | ||
| 35 | scope :inc_author, includes(:author) | 36 | scope :inc_author, includes(:author) |
| 36 | 37 | ||
| 37 | mount_uploader :attachment, AttachmentUploader | 38 | mount_uploader :attachment, AttachmentUploader |
| 39 | + | ||
| 40 | + def notify | ||
| 41 | + @notify ||= false | ||
| 42 | + end | ||
| 38 | end | 43 | end |
| 39 | # == Schema Information | 44 | # == Schema Information |
| 40 | # | 45 | # |
config/application.rb
| @@ -23,7 +23,7 @@ module Gitlab | @@ -23,7 +23,7 @@ module Gitlab | ||
| 23 | # config.plugins = [ :exception_notification, :ssl_requirement, :all ] | 23 | # config.plugins = [ :exception_notification, :ssl_requirement, :all ] |
| 24 | 24 | ||
| 25 | # Activate observers that should always be running. | 25 | # Activate observers that should always be running. |
| 26 | - # config.active_record.observers = :cacher, :garbage_collector, :forum_observer | 26 | + config.active_record.observers = :mailer_observer |
| 27 | 27 | ||
| 28 | # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone. | 28 | # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone. |
| 29 | # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC. | 29 | # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC. |