Commit 8d7951d8 authored by Sean McGivern's avatar Sean McGivern

Merge branch '32834-task-note-only' into 'master'

Prevent Description Change Notes When Toggling Tasks

Closes #32834

See merge request !12057
parents e889b4e8 7360703a
...@@ -313,11 +313,13 @@ class IssuableBaseService < BaseService ...@@ -313,11 +313,13 @@ class IssuableBaseService < BaseService
end end
if issuable.previous_changes.include?('description') if issuable.previous_changes.include?('description')
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) create_description_change_note(issuable)
end end
if issuable.previous_changes.include?('description') && issuable.tasks?
create_task_status_note(issuable)
end end
if issuable.previous_changes.include?('time_estimate') if issuable.previous_changes.include?('time_estimate')
......
---
title: Prevent description change notes when toggling tasks
merge_request: 12057
author: Jared Deckard <jared.deckard@gmail.com>
...@@ -31,6 +31,13 @@ describe Issues::UpdateService, services: true do ...@@ -31,6 +31,13 @@ describe Issues::UpdateService, services: true do
end end
end end
def find_notes(action)
issue
.notes
.joins(:system_note_metadata)
.where(system_note_metadata: { action: action })
end
def update_issue(opts) def update_issue(opts)
described_class.new(project, user, opts).execute(issue) described_class.new(project, user, opts).execute(issue)
end end
...@@ -330,6 +337,9 @@ describe Issues::UpdateService, services: true do ...@@ -330,6 +337,9 @@ describe Issues::UpdateService, services: true do
expect(note1).not_to be_nil expect(note1).not_to be_nil
expect(note2).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 end
...@@ -345,6 +355,9 @@ describe Issues::UpdateService, services: true do ...@@ -345,6 +355,9 @@ describe Issues::UpdateService, services: true do
expect(note1).not_to be_nil expect(note1).not_to be_nil
expect(note2).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 end
...@@ -354,10 +367,12 @@ describe Issues::UpdateService, services: true do ...@@ -354,10 +367,12 @@ describe Issues::UpdateService, services: true do
update_issue(description: "- [x] Task 1\n- [ ] Task 3\n- [ ] Task 2") update_issue(description: "- [x] Task 1\n- [ ] Task 3\n- [ ] Task 2")
end end
it 'does not create a system note' do it 'does not create a system note for the task' do
note = find_note('marked the task **Task 2** as incomplete') 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
end end
...@@ -368,9 +383,11 @@ describe Issues::UpdateService, services: true do ...@@ -368,9 +383,11 @@ describe Issues::UpdateService, services: true do
end end
it 'does not create a system note referencing the position the old item' do 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 end
it 'does not generate a new note at all' do it 'does not generate a new note at all' do
......
...@@ -30,6 +30,13 @@ describe MergeRequests::UpdateService, services: true do ...@@ -30,6 +30,13 @@ describe MergeRequests::UpdateService, services: true do
end end
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) def update_merge_request(opts)
@merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) @merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
@merge_request.reload @merge_request.reload
...@@ -394,6 +401,9 @@ describe MergeRequests::UpdateService, services: true do ...@@ -394,6 +401,9 @@ describe MergeRequests::UpdateService, services: true do
expect(note1).not_to be_nil expect(note1).not_to be_nil
expect(note2).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 end
...@@ -409,6 +419,9 @@ describe MergeRequests::UpdateService, services: true do ...@@ -409,6 +419,9 @@ describe MergeRequests::UpdateService, services: true do
expect(note1).not_to be_nil expect(note1).not_to be_nil
expect(note2).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 end
end end
......
...@@ -1052,7 +1052,7 @@ describe SystemNoteService, services: true do ...@@ -1052,7 +1052,7 @@ describe SystemNoteService, services: true do
let(:action) { 'task' } let(:action) { 'task' }
end end
it "posts the 'marked as a Work In Progress from commit' system note" do it "posts the 'marked the task as complete' system note" do
expect(subject.note).to eq("marked the task **task** as completed") expect(subject.note).to eq("marked the task **task** as completed")
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment