summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJared Deckard <jared.deckard@gmail.com>2017-06-08 12:56:41 -0500
committerJared Deckard <jared.deckard@gmail.com>2017-06-12 11:51:19 -0500
commit2e7db162fa1e1d375688c8eb66fabac2f67e8eb3 (patch)
treec96e6d731ad82bbcbc0b6ac204f68fbbf458fec7
parent9cf6adc111181d0ddc299d7a56e4dbd615607717 (diff)
downloadgitlab-ce-2e7db162fa1e1d375688c8eb66fabac2f67e8eb3.tar.gz
Only add a description change note when no tasks are updated
-rw-r--r--app/services/issuable_base_service.rb12
-rw-r--r--spec/services/issues/update_service_spec.rb27
-rw-r--r--spec/services/merge_requests/update_service_spec.rb13
3 files changed, 42 insertions, 10 deletions
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index e77a3e3eac1..a65d6e11c47 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -313,11 +313,13 @@ class IssuableBaseService < BaseService
end
if issuable.previous_changes.include?('description')
- create_description_change_note(issuable)
- end
-
- if issuable.previous_changes.include?('description') && issuable.tasks?
- create_task_status_note(issuable)
+ if issuable.tasks? && issuable.updated_tasks.any?
+ create_task_status_note(issuable)
+ else
+ # TODO: Show this note if non-task content was modified.
+ # https://gitlab.com/gitlab-org/gitlab-ce/issues/33577
+ create_description_change_note(issuable)
+ end
end
if issuable.previous_changes.include?('time_estimate')
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 5184c1d5f19..a78866a2c32 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -31,6 +31,13 @@ describe Issues::UpdateService, services: true do
end
end
+ def find_notes(action)
+ issue
+ .notes
+ .joins(:system_note_metadata)
+ .where(system_note_metadata: { action: action })
+ end
+
def update_issue(opts)
described_class.new(project, user, opts).execute(issue)
end
@@ -330,6 +337,9 @@ describe Issues::UpdateService, services: true do
expect(note1).not_to be_nil
expect(note2).not_to be_nil
+
+ description_notes = find_notes('description')
+ expect(description_notes.length).to eq(1)
end
end
@@ -345,6 +355,9 @@ describe Issues::UpdateService, services: true do
expect(note1).not_to be_nil
expect(note2).not_to be_nil
+
+ description_notes = find_notes('description')
+ expect(description_notes.length).to eq(1)
end
end
@@ -354,10 +367,12 @@ describe Issues::UpdateService, services: true do
update_issue(description: "- [x] Task 1\n- [ ] Task 3\n- [ ] Task 2")
end
- it 'does not create a system note' do
- note = find_note('marked the task **Task 2** as incomplete')
+ it 'does not create a system note for the task' do
+ task_note = find_note('marked the task **Task 2** as incomplete')
+ description_notes = find_notes('description')
- expect(note).to be_nil
+ expect(task_note).to be_nil
+ expect(description_notes.length).to eq(2)
end
end
@@ -368,9 +383,11 @@ describe Issues::UpdateService, services: true do
end
it 'does not create a system note referencing the position the old item' do
- note = find_note('marked the task **Two** as incomplete')
+ task_note = find_note('marked the task **Two** as incomplete')
+ description_notes = find_notes('description')
- expect(note).to be_nil
+ expect(task_note).to be_nil
+ expect(description_notes.length).to eq(2)
end
it 'does not generate a new note at all' do
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index d371fc68312..091c193aaa6 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -30,6 +30,13 @@ describe MergeRequests::UpdateService, services: true do
end
end
+ def find_notes(action)
+ @merge_request
+ .notes
+ .joins(:system_note_metadata)
+ .where(system_note_metadata: { action: action })
+ end
+
def update_merge_request(opts)
@merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
@merge_request.reload
@@ -394,6 +401,9 @@ describe MergeRequests::UpdateService, services: true do
expect(note1).not_to be_nil
expect(note2).not_to be_nil
+
+ description_notes = find_notes('description')
+ expect(description_notes.length).to eq(1)
end
end
@@ -409,6 +419,9 @@ describe MergeRequests::UpdateService, services: true do
expect(note1).not_to be_nil
expect(note2).not_to be_nil
+
+ description_notes = find_notes('description')
+ expect(description_notes.length).to eq(1)
end
end
end