summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/models/event.rb8
-rw-r--r--app/observers/activity_observer.rb35
-rw-r--r--app/observers/base_observer.rb4
-rw-r--r--app/observers/issue_observer.rb3
-rw-r--r--app/observers/merge_request_observer.rb23
-rw-r--r--app/observers/milestone_observer.rb13
-rw-r--r--app/observers/note_observer.rb6
-rw-r--r--app/services/event_create_service.rb64
-rw-r--r--app/services/merge_requests/base_merge_service.rb8
-rw-r--r--config/application.rb2
-rw-r--r--spec/observers/activity_observer_spec.rb61
-rw-r--r--spec/requests/api/issues_spec.rb12
-rw-r--r--spec/services/event_create_service_spec.rb103
-rw-r--r--spec/support/test_env.rb2
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