summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2015-08-30 21:51:34 -0700
committerStan Hu <stanhu@gmail.com>2015-09-15 05:51:11 -0700
commitd3d03d1362e576d194782a655cdfe9bc6ed5c596 (patch)
tree0b720ea7ac0b5df7e968df83ce25d8e571617a56
parent080a086d7644285af6cd4fb4b51c8f1c9b3aec95 (diff)
downloadgitlab-ce-d3d03d1362e576d194782a655cdfe9bc6ed5c596.tar.gz
Create a "destroyed Milestone" event and keep Milestone events around in the DB
for posterity. Also fix issue where destroying a Milestone would cause odd, transient messages like "created milestone" or "imported milestone". Add "in" preposition when creating and destroying milestones Closes #2382
-rw-r--r--app/controllers/projects/milestones_controller.rb7
-rw-r--r--app/helpers/events_helper.rb11
-rw-r--r--app/models/event.rb12
-rw-r--r--app/models/milestone.rb2
-rw-r--r--app/services/event_create_service.rb4
-rw-r--r--app/services/milestones/destroy_service.rb22
-rw-r--r--app/views/events/event/_common.html.haml5
-rw-r--r--features/project/issues/milestones.feature8
-rw-r--r--features/steps/project/issues/milestones.rb7
-rw-r--r--features/steps/shared/project.rb5
-rw-r--r--spec/controllers/projects/milestones_controller_spec.rb4
-rw-r--r--spec/services/event_create_service_spec.rb10
12 files changed, 83 insertions, 14 deletions
diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb
index 9efe9704d1e..86f4a02a6e9 100644
--- a/app/controllers/projects/milestones_controller.rb
+++ b/app/controllers/projects/milestones_controller.rb
@@ -66,12 +66,7 @@ class Projects::MilestonesController < Projects::ApplicationController
def destroy
return access_denied! unless can?(current_user, :admin_milestone, @project)
- update_params = { milestone: nil }
- @milestone.issues.each do |issue|
- Issues::UpdateService.new(@project, current_user, update_params).execute(issue)
- end
-
- @milestone.destroy
+ Milestones::DestroyService.new(project, current_user).execute(milestone)
respond_to do |format|
format.html { redirect_to namespace_project_milestones_path }
diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb
index 76602614bcd..6f69c2a9f32 100644
--- a/app/helpers/events_helper.rb
+++ b/app/helpers/events_helper.rb
@@ -46,6 +46,14 @@ module EventsHelper
}
end
+ def event_preposition(event)
+ if event.push? || event.commented? || event.target
+ "at"
+ elsif event.milestone?
+ "in"
+ end
+ end
+
def event_feed_title(event)
words = []
words << event.author_name
@@ -62,6 +70,9 @@ module EventsHelper
words << "##{truncate event.note_target_iid}"
end
words << "at"
+ elsif event.milestone?
+ words << "##{event.target_iid}" if event.target_iid
+ words << "in"
elsif event.target
words << "##{event.target_iid}:"
words << event.target.title if event.target.respond_to?(:title)
diff --git a/app/models/event.rb b/app/models/event.rb
index 78f16c6304e..47600c57e35 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -27,6 +27,7 @@ class Event < ActiveRecord::Base
MERGED = 7
JOINED = 8 # User joined project
LEFT = 9 # User left project
+ DESTROYED = 10
delegate :name, :email, to: :author, prefix: true, allow_nil: true
delegate :title, to: :issue, prefix: true, allow_nil: true
@@ -48,6 +49,7 @@ class Event < ActiveRecord::Base
scope :code_push, -> { where(action: PUSHED) }
scope :in_projects, ->(project_ids) { where(project_id: project_ids).recent }
scope :with_associations, -> { includes(project: :namespace) }
+ scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) }
class << self
def reset_event_cache_for(target)
@@ -71,7 +73,7 @@ class Event < ActiveRecord::Base
elsif created_project?
true
else
- (issue? || merge_request? || note? || milestone?) && target
+ ((issue? || merge_request? || note?) && target) || milestone?
end
end
@@ -115,6 +117,10 @@ class Event < ActiveRecord::Base
action == LEFT
end
+ def destroyed?
+ action == DESTROYED
+ end
+
def commented?
action == COMMENTED
end
@@ -124,7 +130,7 @@ class Event < ActiveRecord::Base
end
def created_project?
- created? && !target
+ created? && !target && target_type.nil?
end
def created_target?
@@ -180,6 +186,8 @@ class Event < ActiveRecord::Base
'joined'
elsif left?
'left'
+ elsif destroyed?
+ 'destroyed'
elsif commented?
"commented on"
elsif created_project?
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index c6aff6f709f..d979a35084b 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -61,7 +61,7 @@ class Milestone < ActiveRecord::Base
false
end
end
-
+
def open_items_count
self.issues.opened.count + self.merge_requests.opened.count
end
diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb
index 103d6b0a08b..07fc77001a5 100644
--- a/app/services/event_create_service.rb
+++ b/app/services/event_create_service.rb
@@ -46,6 +46,10 @@ class EventCreateService
create_record_event(milestone, current_user, Event::REOPENED)
end
+ def destroy_milestone(milestone, current_user)
+ create_record_event(milestone, current_user, Event::DESTROYED)
+ end
+
def leave_note(note, current_user)
create_record_event(note, current_user, Event::COMMENTED)
end
diff --git a/app/services/milestones/destroy_service.rb b/app/services/milestones/destroy_service.rb
new file mode 100644
index 00000000000..7ce7d259d0b
--- /dev/null
+++ b/app/services/milestones/destroy_service.rb
@@ -0,0 +1,22 @@
+module Milestones
+ class DestroyService < Milestones::BaseService
+ def execute(milestone)
+
+ Milestone.transaction do
+ update_params = { milestone: nil }
+ milestone.issues.each do |issue|
+ Issues::UpdateService.new(project, current_user, update_params).execute(issue)
+ end
+
+ event_service.destroy_milestone(milestone, current_user)
+
+ Event.for_milestone_id(milestone.id).each do |event|
+ event.target_id = nil
+ event.save
+ end
+
+ milestone.destroy
+ end
+ end
+ end
+end
diff --git a/app/views/events/event/_common.html.haml b/app/views/events/event/_common.html.haml
index a39e62e9dac..4ecf1c33d2a 100644
--- a/app/views/events/event/_common.html.haml
+++ b/app/views/events/event/_common.html.haml
@@ -5,13 +5,14 @@
- if event.target
%strong= link_to "##{event.target_iid}", [event.project.namespace.becomes(Namespace), event.project, event.target]
- at
+
+ = event_preposition(event)
- if event.project
= link_to_project event.project
- else
= event.project_name
-
+
- if event.target.respond_to?(:title)
.event-body
.event-note
diff --git a/features/project/issues/milestones.feature b/features/project/issues/milestones.feature
index bfbaaec5a35..c1a20e9b488 100644
--- a/features/project/issues/milestones.feature
+++ b/features/project/issues/milestones.feature
@@ -12,13 +12,17 @@ Feature: Project Issues Milestones
Given I click link "v2.2"
Then I should see milestone "v2.2"
- Scenario: I create new milestone
+ @javascript
+ Scenario: I create and delete new milestone
Given I click link "New Milestone"
And I submit new milestone "v2.3"
Then I should see milestone "v2.3"
+ Given I click link to remove milestone
+ When I visit project "Shop" activity page
+ Then I should see deleted milestone activity
Scenario: I delete new milestone
- Given I click link to remove milestone "v2.2"
+ Given I click link to remove milestone
And I should see no milestones
@javascript
diff --git a/features/steps/project/issues/milestones.rb b/features/steps/project/issues/milestones.rb
index 61e62c2adbd..c8708572ec6 100644
--- a/features/steps/project/issues/milestones.rb
+++ b/features/steps/project/issues/milestones.rb
@@ -49,6 +49,11 @@ class Spinach::Features::ProjectIssuesMilestones < Spinach::FeatureSteps
create(:closed_issue, project: project, milestone: milestone)
end
+ step 'I should see deleted milestone activity' do
+ expect(page).to have_content('opened milestone in')
+ expect(page).to have_content('destroyed milestone in')
+ end
+
When 'I click link "All Issues"' do
click_link 'All Issues'
end
@@ -57,7 +62,7 @@ class Spinach::Features::ProjectIssuesMilestones < Spinach::FeatureSteps
expect(page).to have_selector('#tab-issues li.issue-row', count: 4)
end
- step 'I click link to remove milestone "v2.2"' do
+ step 'I click link to remove milestone' do
click_link 'Remove'
end
diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb
index ccbe8f96a4c..a9cf426852e 100644
--- a/features/steps/shared/project.rb
+++ b/features/steps/shared/project.rb
@@ -51,6 +51,11 @@ module SharedProject
visit namespace_project_path(project.namespace, project)
end
+ step 'I visit project "Shop" activity page' do
+ project = Project.find_by(name: 'Shop')
+ visit namespace_project_path(project.namespace, project)
+ end
+
step 'project "Shop" has push event' do
@project = Project.find_by(name: "Shop")
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
index d3868c13202..35446640929 100644
--- a/spec/controllers/projects/milestones_controller_spec.rb
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -15,8 +15,12 @@ describe Projects::MilestonesController do
describe "#destroy" do
it "should remove milestone" do
expect(issue.milestone_id).to eq(milestone.id)
+
delete :destroy, namespace_id: project.namespace.id, project_id: project.id, id: milestone.id, format: :js
expect(response).to be_success
+
+ expect(Event.first.action).to eq(Event::DESTROYED)
+
expect { Milestone.find(milestone.id) }.to raise_exception(ActiveRecord::RecordNotFound)
issue.reload
expect(issue.milestone_id).to eq(nil)
diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb
index 007a9eed192..7756b973ecd 100644
--- a/spec/services/event_create_service_spec.rb
+++ b/spec/services/event_create_service_spec.rb
@@ -99,5 +99,15 @@ describe EventCreateService do
expect { service.close_milestone(milestone, user) }.to change { Event.count }
end
end
+
+ describe :destroy_mr do
+ let(:milestone) { create(:milestone) }
+
+ it { expect(service.destroy_milestone(milestone, user)).to be_truthy }
+
+ it "should create new event" do
+ expect { service.destroy_milestone(milestone, user) }.to change { Event.count }
+ end
+ end
end
end