From 6149dba5189b9f32b3a9caf0c4c585a973ec99fb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 17 Jun 2014 22:09:01 +0300 Subject: [PATCH] Remove NotesObserver --- app/controllers/projects/notes_controller.rb | 2 +- app/models/note.rb | 5 ++ app/observers/note_observer.rb | 20 ------- app/services/notes/create_service.rb | 19 ++++++- config/application.rb | 3 +- spec/observers/note_observer_spec.rb | 56 -------------------- spec/services/notes/create_service_spec.rb | 27 ++++++++++ 7 files changed, 51 insertions(+), 81 deletions(-) delete mode 100644 app/observers/note_observer.rb delete mode 100644 spec/observers/note_observer_spec.rb create mode 100644 spec/services/notes/create_service_spec.rb diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index b5b0446b43f..5df92b29eda 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -21,7 +21,7 @@ class Projects::NotesController < Projects::ApplicationController end def create - @note = Notes::CreateService.new(project, current_user, params).execute + @note = Notes::CreateService.new(project, current_user, params[:note]).execute respond_to do |format| format.json { render_note_json(@note) } diff --git a/app/models/note.rb b/app/models/note.rb index 659cd752071..01026cd3994 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -57,6 +57,7 @@ class Note < ActiveRecord::Base serialize :st_diff before_create :set_diff, if: ->(n) { n.line_code.present? } + after_update :set_references class << self def create_status_change_note(noteable, project, author, status, source) @@ -314,4 +315,8 @@ class Note < ActiveRecord::Base order('id DESC').limit(100). update_all(updated_at: Time.now) end + + def set_references + notice_added_references(project, author) + end end diff --git a/app/observers/note_observer.rb b/app/observers/note_observer.rb deleted file mode 100644 index bb0b0876eca..00000000000 --- a/app/observers/note_observer.rb +++ /dev/null @@ -1,20 +0,0 @@ -class NoteObserver < BaseObserver - def after_create(note) - notification.new_note(note) - - # Skip system notes, like status changes and cross-references. - unless note.system - event_service.leave_note(note, note.author) - - # Create a cross-reference note if this Note contains GFM that names an - # issue, merge request, or commit. - note.references.each do |mentioned| - Note.create_cross_reference_note(mentioned, note.noteable, note.author, note.project) - end - end - end - - def after_update(note) - note.notice_added_references(note.project, note.author) - end -end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index ff6dfb61139..f64006a4edc 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -1,10 +1,25 @@ module Notes class CreateService < BaseService def execute - note = project.notes.new(params[:note]) + note = project.notes.new(params) note.author = current_user note.system = false - note.save + + if note.save + notification_service.new_note(note) + + # Skip system notes, like status changes and cross-references. + unless note.system + event_service.leave_note(note, note.author) + + # Create a cross-reference note if this Note contains GFM that names an + # issue, merge request, or commit. + note.references.each do |mentioned| + Note.create_cross_reference_note(mentioned, note.noteable, note.author, note.project) + end + end + end + note end end diff --git a/config/application.rb b/config/application.rb index d0ed85d7cf6..9e09f654001 100644 --- a/config/application.rb +++ b/config/application.rb @@ -19,8 +19,7 @@ module Gitlab # config.plugins = [ :exception_notification, :ssl_requirement, :all ] # Activate observers that should always be running. - config.active_record.observers = :note_observer, - :system_hook_observer, + config.active_record.observers = :system_hook_observer, :user_observer, :users_project_observer diff --git a/spec/observers/note_observer_spec.rb b/spec/observers/note_observer_spec.rb deleted file mode 100644 index f8693355b23..00000000000 --- a/spec/observers/note_observer_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -require 'spec_helper' - -describe NoteObserver do - subject { NoteObserver.instance } - before { subject.stub(notification: double('NotificationService').as_null_object) } - - let(:team_without_author) { (1..2).map { |n| double :user, id: n } } - let(:note) { double(:note).as_null_object } - - describe '#after_create' do - - it 'is called after a note is created' do - subject.should_receive :after_create - - Note.observers.enable :note_observer do - create(:note) - end - end - - it 'sends out notifications' do - subject.should_receive(:notification) - - subject.after_create(note) - end - - it 'creates cross-reference notes as appropriate' do - @p = create(:project) - @referenced = create(:issue, project: @p) - @referencer = create(:issue, project: @p) - @author = create(:user) - - Note.should_receive(:create_cross_reference_note).with(@referenced, @referencer, @author, @p) - - Note.observers.enable :note_observer do - create(:note, project: @p, author: @author, noteable: @referencer, - note: "Duplicate of ##{@referenced.iid}") - end - end - - it "doesn't cross-reference system notes" do - Note.should_receive(:create_cross_reference_note).once - - Note.observers.enable :note_observer do - Note.create_cross_reference_note(create(:issue), create(:issue)) - end - end - end - - describe '#after_update' do - it 'checks for new cross-references' do - note.should_receive(:notice_added_references) - - subject.after_update(note) - end - end -end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb new file mode 100644 index 00000000000..106c14bc015 --- /dev/null +++ b/spec/services/notes/create_service_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe Notes::CreateService do + let(:project) { create(:empty_project) } + let(:issue) { create(:issue, project: project) } + let(:user) { create(:user) } + + describe :execute do + context "valid params" do + before do + project.team << [user, :master] + opts = { + note: 'Awesome comment', + description: 'please fix', + noteable_type: 'Issue', + noteable_id: issue.id + } + + @note = Notes::CreateService.new(project, user, opts).execute + end + + it { @note.should be_valid } + it { @note.note.should == 'Awesome comment' } + end + end +end +