diff options
-rw-r--r-- | app/models/event.rb | 8 | ||||
-rw-r--r-- | app/observers/activity_observer.rb | 35 | ||||
-rw-r--r-- | app/observers/base_observer.rb | 4 | ||||
-rw-r--r-- | app/observers/issue_observer.rb | 3 | ||||
-rw-r--r-- | app/observers/merge_request_observer.rb | 23 | ||||
-rw-r--r-- | app/observers/milestone_observer.rb | 13 | ||||
-rw-r--r-- | app/observers/note_observer.rb | 6 | ||||
-rw-r--r-- | app/services/event_create_service.rb | 64 | ||||
-rw-r--r-- | app/services/merge_requests/base_merge_service.rb | 8 | ||||
-rw-r--r-- | config/application.rb | 2 | ||||
-rw-r--r-- | spec/observers/activity_observer_spec.rb | 61 | ||||
-rw-r--r-- | spec/requests/api/issues_spec.rb | 12 | ||||
-rw-r--r-- | spec/services/event_create_service_spec.rb | 103 | ||||
-rw-r--r-- | spec/support/test_env.rb | 2 |
14 files changed, 200 insertions, 144 deletions
diff --git a/app/models/event.rb b/app/models/event.rb index d43d6eb682f..5c156856d79 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -47,14 +47,6 @@ class Event < ActiveRecord::Base scope :in_projects, ->(project_ids) { where(project_id: project_ids).recent } class << self - def determine_action(record) - if [Issue, MergeRequest].include? record.class - Event::CREATED - elsif record.kind_of? Note - Event::COMMENTED - end - end - def create_ref_event(project, user, ref, action = 'add', prefix = 'refs/heads') commit = project.repository.commit(ref.target) diff --git a/app/observers/activity_observer.rb b/app/observers/activity_observer.rb deleted file mode 100644 index c5420c275ae..00000000000 --- a/app/observers/activity_observer.rb +++ /dev/null @@ -1,35 +0,0 @@ -class ActivityObserver < BaseObserver - observe :issue, :note, :milestone - - def after_create(record) - if record.kind_of?(Note) - # Skip system notes, like status changes and cross-references. - return true if record.system? - - # Skip wall notes to prevent spamming of dashboard - return true if record.noteable_type.blank? - end - - create_event(record, Event.determine_action(record)) if current_user - end - - def after_close(record, transition) - create_event(record, Event::CLOSED) - end - - def after_reopen(record, transition) - create_event(record, Event::REOPENED) - end - - protected - - def create_event(record, status) - Event.create( - project: record.project, - target_id: record.id, - target_type: record.class.name, - action: status, - author_id: current_user.id - ) - end -end diff --git a/app/observers/base_observer.rb b/app/observers/base_observer.rb index f9a0242ce77..d685bd5d819 100644 --- a/app/observers/base_observer.rb +++ b/app/observers/base_observer.rb @@ -3,6 +3,10 @@ class BaseObserver < ActiveRecord::Observer NotificationService.new end + def event_service + EventCreateService.new + end + def log_info message Gitlab::AppLogger.info message end diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb index 6ef13eb5d5e..30da1f83da7 100644 --- a/app/observers/issue_observer.rb +++ b/app/observers/issue_observer.rb @@ -1,17 +1,20 @@ class IssueObserver < BaseObserver def after_create(issue) notification.new_issue(issue, current_user) + event_service.open_issue(issue, current_user) issue.create_cross_references!(issue.project, current_user) execute_hooks(issue) end def after_close(issue, transition) notification.close_issue(issue, current_user) + event_service.close_issue(issue, current_user) create_note(issue) execute_hooks(issue) end def after_reopen(issue, transition) + event_service.reopen_issue(issue, current_user) create_note(issue) execute_hooks(issue) end diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb index f2e2d16c943..04ee30b4976 100644 --- a/app/observers/merge_request_observer.rb +++ b/app/observers/merge_request_observer.rb @@ -1,25 +1,20 @@ -class MergeRequestObserver < ActivityObserver - observe :merge_request - +class MergeRequestObserver < BaseObserver def after_create(merge_request) - if merge_request.author_id - create_event(merge_request, Event.determine_action(merge_request)) - end - + event_service.open_mr(merge_request, current_user) notification.new_merge_request(merge_request, current_user) merge_request.create_cross_references!(merge_request.project, current_user) execute_hooks(merge_request) end def after_close(merge_request, transition) - create_event(merge_request, Event::CLOSED) + event_service.close_mr(merge_request, current_user) notification.close_mr(merge_request, current_user) create_note(merge_request) execute_hooks(merge_request) end def after_reopen(merge_request, transition) - create_event(merge_request, Event::REOPENED) + event_service.reopen_mr(merge_request, current_user) create_note(merge_request) execute_hooks(merge_request) merge_request.reload_code @@ -33,16 +28,6 @@ class MergeRequestObserver < ActivityObserver execute_hooks(merge_request) end - def create_event(record, status) - Event.create( - project: record.target_project, - target_id: record.id, - target_type: record.class.name, - action: status, - author_id: current_user.id - ) - end - private # Create merge request note with service comment like 'Status changed to closed' diff --git a/app/observers/milestone_observer.rb b/app/observers/milestone_observer.rb new file mode 100644 index 00000000000..a1a62a99a8f --- /dev/null +++ b/app/observers/milestone_observer.rb @@ -0,0 +1,13 @@ +class MilestoneObserver < BaseObserver + def after_create(milestone) + event_service.open_milestone(milestone, current_user) + end + + def after_close(milestone, transition) + event_service.close_milestone(milestone, current_user) + end + + def after_reopen(milestone, transition) + event_service.reopen_milestone(milestone, current_user) + end +end diff --git a/app/observers/note_observer.rb b/app/observers/note_observer.rb index d31b6e8d7ce..337bb1dc5ae 100644 --- a/app/observers/note_observer.rb +++ b/app/observers/note_observer.rb @@ -2,6 +2,12 @@ class NoteObserver < BaseObserver def after_create(note) notification.new_note(note) + # Skip system notes, like status changes and cross-references. + # Skip wall notes to prevent spamming of dashboard + if note.noteable_type.present? && !note.system + event_service.leave_note(note, current_user) + end + unless note.system? # Create a cross-reference note if this Note contains GFM that names an # issue, merge request, or commit. diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb new file mode 100644 index 00000000000..8d8a5873e62 --- /dev/null +++ b/app/services/event_create_service.rb @@ -0,0 +1,64 @@ +# EventCreateService class +# +# Used for creating events feed on dashboard after certain user action +# +# Ex. +# EventCreateService.new.new_issue(issue, current_user) +# +class EventCreateService + def open_issue(issue, current_user) + create_event(issue, current_user, Event::CREATED) + end + + def close_issue(issue, current_user) + create_event(issue, current_user, Event::CLOSED) + end + + def reopen_issue(issue, current_user) + create_event(issue, current_user, Event::REOPENED) + end + + def open_mr(merge_request, current_user) + create_event(merge_request, current_user, Event::CREATED) + end + + def close_mr(merge_request, current_user) + create_event(merge_request, current_user, Event::CLOSED) + end + + def reopen_mr(merge_request, current_user) + create_event(merge_request, current_user, Event::REOPENED) + end + + def merge_mr(merge_request, current_user) + create_event(merge_request, current_user, Event::MERGED) + end + + def open_milestone(milestone, current_user) + create_event(milestone, current_user, Event::CREATED) + end + + def close_milestone(milestone, current_user) + create_event(milestone, current_user, Event::CLOSED) + end + + def reopen_milestone(milestone, current_user) + create_event(milestone, current_user, Event::REOPENED) + end + + def leave_note(note, current_user) + create_event(note, current_user, Event::COMMENTED) + end + + private + + def create_event(record, current_user, status) + Event.create( + project: record.project, + target_id: record.id, + target_type: record.class.name, + action: status, + author_id: current_user.id + ) + end +end diff --git a/app/services/merge_requests/base_merge_service.rb b/app/services/merge_requests/base_merge_service.rb index d0f777d50ec..9bc50d3d16c 100644 --- a/app/services/merge_requests/base_merge_service.rb +++ b/app/services/merge_requests/base_merge_service.rb @@ -8,13 +8,7 @@ module MergeRequests end def create_merge_event(merge_request, current_user) - Event.create( - project: merge_request.target_project, - target_id: merge_request.id, - target_type: merge_request.class.name, - action: Event::MERGED, - author_id: current_user.id - ) + EventCreateService.new.merge_mr(merge_request, current_user) end def execute_project_hooks(merge_request) diff --git a/config/application.rb b/config/application.rb index 4d7c1415c8e..a782dd1d01e 100644 --- a/config/application.rb +++ b/config/application.rb @@ -19,7 +19,7 @@ module Gitlab # config.plugins = [ :exception_notification, :ssl_requirement, :all ] # Activate observers that should always be running. - config.active_record.observers = :activity_observer, + config.active_record.observers = :milestone_observer, :project_activity_cache_observer, :issue_observer, :key_observer, diff --git a/spec/observers/activity_observer_spec.rb b/spec/observers/activity_observer_spec.rb deleted file mode 100644 index dc14ab86b6d..00000000000 --- a/spec/observers/activity_observer_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -require 'spec_helper' - -describe ActivityObserver do - let(:project) { create(:project) } - - before { Thread.current[:current_user] = create(:user) } - - def self.it_should_be_valid_event - it { @event.should_not be_nil } - it { @event.project.should == project } - end - - describe "Issue created" do - before do - Issue.observers.enable :activity_observer do - @issue = create(:issue, project: project) - @event = Event.last - end - end - - it_should_be_valid_event - it { @event.action.should == Event::CREATED } - it { @event.target.should == @issue } - end - - describe "Issue commented" do - before do - Note.observers.enable :activity_observer do - @issue = create(:issue, project: project) - @note = create(:note, noteable: @issue, project: project, author: @issue.author) - @event = Event.last - end - end - - it_should_be_valid_event - it { @event.action.should == Event::COMMENTED } - it { @event.target.should == @note } - end - - describe "Ignore system notes" do - let(:author) { create(:user) } - let!(:issue) { create(:issue, project: project) } - let!(:other) { create(:issue) } - - it "should not create events for status change notes" do - expect do - Note.observers.enable :activity_observer do - Note.create_status_change_note(issue, project, author, 'reopened', nil) - end - end.to_not change { Event.count } - end - - it "should not create events for cross-reference notes" do - expect do - Note.observers.enable :activity_observer do - Note.create_cross_reference_note(issue, other, author, issue.project) - end - end.to_not change { Event.count } - end - end -end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index c55025d72b5..e9422cd2643 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -100,16 +100,4 @@ describe API::API do response.status.should == 405 end end - - describe "PUT /projects/:id/issues/:issue_id to test observer on close" do - before { enable_observers } - after { disable_observers } - - it "should create an activity event when an issue is closed" do - Event.should_receive(:create) - - put api("/projects/#{project.id}/issues/#{issue.id}", user), - state_event: "close" - end - end end diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb new file mode 100644 index 00000000000..713aa3e7e74 --- /dev/null +++ b/spec/services/event_create_service_spec.rb @@ -0,0 +1,103 @@ +require 'spec_helper' + +describe EventCreateService do + let(:service) { EventCreateService.new } + + describe 'Issues' do + describe :open_issue do + let(:issue) { create(:issue) } + + it { service.open_issue(issue, issue.author).should be_true } + + it "should create new event" do + expect { service.open_issue(issue, issue.author) }.to change { Event.count } + end + end + + describe :close_issue do + let(:issue) { create(:issue) } + + it { service.close_issue(issue, issue.author).should be_true } + + it "should create new event" do + expect { service.close_issue(issue, issue.author) }.to change { Event.count } + end + end + + describe :reopen_issue do + let(:issue) { create(:issue) } + + it { service.reopen_issue(issue, issue.author).should be_true } + + it "should create new event" do + expect { service.reopen_issue(issue, issue.author) }.to change { Event.count } + end + end + end + + describe 'Merge Requests' do + describe :open_mr do + let(:merge_request) { create(:merge_request) } + + it { service.open_mr(merge_request, merge_request.author).should be_true } + + it "should create new event" do + expect { service.open_mr(merge_request, merge_request.author) }.to change { Event.count } + end + end + + describe :close_mr do + let(:merge_request) { create(:merge_request) } + + it { service.close_mr(merge_request, merge_request.author).should be_true } + + it "should create new event" do + expect { service.close_mr(merge_request, merge_request.author) }.to change { Event.count } + end + end + + describe :merge_mr do + let(:merge_request) { create(:merge_request) } + + it { service.merge_mr(merge_request, merge_request.author).should be_true } + + it "should create new event" do + expect { service.merge_mr(merge_request, merge_request.author) }.to change { Event.count } + end + end + + describe :reopen_mr do + let(:merge_request) { create(:merge_request) } + + it { service.reopen_mr(merge_request, merge_request.author).should be_true } + + it "should create new event" do + expect { service.reopen_mr(merge_request, merge_request.author) }.to change { Event.count } + end + end + end + + describe 'Milestone' do + let(:user) { create :user } + + describe :open_milestone do + let(:milestone) { create(:milestone) } + + it { service.open_milestone(milestone, user).should be_true } + + it "should create new event" do + expect { service.open_milestone(milestone, user) }.to change { Event.count } + end + end + + describe :close_mr do + let(:milestone) { create(:milestone) } + + it { service.close_milestone(milestone, user).should be_true } + + it "should create new event" do + expect { service.close_milestone(milestone, user) }.to change { Event.count } + end + end + end +end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index d00decf6121..d237f7ad094 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -90,7 +90,7 @@ module TestEnv size: 12.45 ) - ActivityObserver.any_instance.stub( + BaseObserver.any_instance.stub( current_user: double("current_user", id: 1) ) end |