Auto resolve new notes of resolved discussions
**Why?** The previous behavior had resolved discussions being unresolved when commented on. This was strange UX, especially since there is a separate button for "Comment & unresolve discussion". https://gitlab.com/gitlab-org/gitlab-ce/issues/24128 **Note:** - Also adds changelog
This commit is contained in:
parent
ce27f74afa
commit
370f07361c
|
@ -3,6 +3,7 @@
|
|||
module Notes
|
||||
class BuildService < ::BaseService
|
||||
def execute
|
||||
should_resolve = false
|
||||
in_reply_to_discussion_id = params.delete(:in_reply_to_discussion_id)
|
||||
|
||||
if in_reply_to_discussion_id.present?
|
||||
|
@ -15,12 +16,17 @@ module Notes
|
|||
end
|
||||
|
||||
params.merge!(discussion.reply_attributes)
|
||||
should_resolve = discussion.resolved?
|
||||
end
|
||||
|
||||
note = Note.new(params)
|
||||
note.project = project
|
||||
note.author = current_user
|
||||
|
||||
if should_resolve
|
||||
note.resolve_without_save(current_user)
|
||||
end
|
||||
|
||||
note
|
||||
end
|
||||
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Fix resolved discussions being unresolved when commented on
|
||||
merge_request: 21881
|
||||
author:
|
||||
type: fixed
|
|
@ -149,6 +149,20 @@ describe 'Merge request > User resolves diff notes and discussions', :js do
|
|||
end
|
||||
end
|
||||
|
||||
it 'allows user to comment' do
|
||||
page.within '.diff-content' do
|
||||
find('.js-note-text').set 'testing'
|
||||
|
||||
click_button 'Comment'
|
||||
|
||||
wait_for_requests
|
||||
end
|
||||
|
||||
page.within '.line-resolve-all-container' do
|
||||
expect(page).to have_content('1/1 discussion resolved')
|
||||
end
|
||||
end
|
||||
|
||||
it 'allows user to unresolve from reply form without a comment' do
|
||||
page.within '.diff-content' do
|
||||
click_button 'Unresolve discussion'
|
||||
|
|
|
@ -4,6 +4,8 @@ describe Notes::BuildService do
|
|||
let(:note) { create(:discussion_note_on_issue) }
|
||||
let(:project) { note.project }
|
||||
let(:author) { note.author }
|
||||
let(:merge_request) { create(:merge_request, source_project: project) }
|
||||
let(:mr_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: author) }
|
||||
|
||||
describe '#execute' do
|
||||
context 'when in_reply_to_discussion_id is specified' do
|
||||
|
@ -12,6 +14,19 @@ describe Notes::BuildService do
|
|||
new_note = described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute
|
||||
expect(new_note).to be_valid
|
||||
expect(new_note.in_reply_to?(note)).to be_truthy
|
||||
expect(new_note.resolved?).to be_falsey
|
||||
end
|
||||
|
||||
context 'when discussion is resolved' do
|
||||
before do
|
||||
mr_note.resolve!(author)
|
||||
end
|
||||
|
||||
it 'resolves the note' do
|
||||
new_note = described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: mr_note.discussion_id).execute
|
||||
expect(new_note).to be_valid
|
||||
expect(new_note.resolved?).to be_truthy
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in New Issue