Merge branch '7921-fix-batch-comments-resolution' into 'master'

Resolves "Reviews: Resolving or unresolving discussions with “Add comment now” does not work"

Closes #7921

See merge request gitlab-org/gitlab-ee!7866

app/assets/javascripts/notes/components/note_form.vue

@@ -106,6 +106,18 @@ export default {
   },
   methods: {
     ...mapActions(['toggleResolveNote']),
+    shouldToggleResolved(shouldResolve, beforeSubmitDiscussionState) {
+      // shouldBeResolved() checks the actual resolution state,
+      // considering batchComments (EEP), if applicable/enabled.
+      const newResolvedStateAfterUpdate =
+        this.shouldBeResolved && this.shouldBeResolved(shouldResolve);
+
+      const shouldToggleState =
+        newResolvedStateAfterUpdate !== undefined &&
+        beforeSubmitDiscussionState !== newResolvedStateAfterUpdate;
+
+      return shouldResolve || shouldToggleState;
+    },
     handleUpdate(shouldResolve) {
       const beforeSubmitDiscussionState = this.discussionResolved;
       this.isSubmitting = true;
@@ -113,7 +125,7 @@ export default {
       this.$emit('handleFormUpdate', this.updatedNoteBody, this.$refs.editNoteForm, () => {
         this.isSubmitting = false;
 
-        if (shouldResolve) {
+        if (this.shouldToggleResolved(shouldResolve, beforeSubmitDiscussionState)) {
           this.resolveHandler(beforeSubmitDiscussionState);
         }
       });

ee/changelogs/unreleased/7921-fix-batch-comments-resolution.yml

+---
+title: Fix bug when resolving a discussion via a batch comment published right away
+merge_request:
+author:
+type: fixed

ee/spec/features/merge_request/batch_comments_spec.rb

@@ -130,6 +130,84 @@ describe 'Merge request > Batch comments', :js do
         expect(find('.review-bar-content .btn-success')).to have_content('2')
       end
     end
+
+    context 'discussion is unresolved' do
+      let!(:active_discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion }
+
+      before do
+        visit_diffs
+      end
+
+      it 'publishes comment right away and resolves the discussion' do
+        expect(active_discussion.resolved?).to eq(false)
+
+        write_reply_to_discussion(button_text: 'Add comment now', resolve: true)
+
+        page.within '.line-resolve-all-container' do
+          expect(page).to have_content('1/1 discussion resolved')
+          expect(page).to have_selector('.line-resolve-btn.is-active')
+        end
+      end
+
+      it 'publishes review and resolves the discussion' do
+        expect(active_discussion.resolved?).to eq(false)
+
+        write_reply_to_discussion(resolve: true)
+
+        page.within('.review-bar-content') do
+          click_button 'Submit review'
+        end
+
+        wait_for_requests
+
+        page.within '.line-resolve-all-container' do
+          expect(page).to have_content('1/1 discussion resolved')
+          expect(page).to have_selector('.line-resolve-btn.is-active')
+        end
+      end
+    end
+
+    context 'discussion is resolved' do
+      let!(:active_discussion) { create(:diff_note_on_merge_request, :resolved, noteable: merge_request, project: project).to_discussion }
+
+      before do
+        active_discussion.resolve!(@current_user)
+
+        visit_diffs
+
+        page.find('.js-diff-comment-avatar').click
+      end
+
+      it 'publishes comment right away and unresolves the discussion' do
+        expect(active_discussion.resolved?).to eq(true)
+
+        write_reply_to_discussion(button_text: 'Add comment now', unresolve: true)
+
+        page.within '.line-resolve-all-container' do
+          expect(page).to have_content('0/1 discussion resolved')
+          expect(page).to have_selector('.line-resolve-btn.is-disabled')
+        end
+      end
+
+      it 'publishes review and unresolves the discussion' do
+        expect(active_discussion.resolved?).to eq(true)
+
+        wait_for_requests
+
+        write_reply_to_discussion(button_text: 'Start a review', unresolve: true)
+
+        page.within('.review-bar-content') do
+          click_button 'Submit review'
+        end
+
+        wait_for_requests
+
+        page.within '.line-resolve-all-container' do
+          expect(page).to have_content('0/1 discussion resolved')
+          expect(page).to have_selector('.line-resolve-btn.is-disabled')
+        end
+      end
+    end
   end
 
   def visit_diffs
@@ -161,3 +239,23 @@ describe 'Merge request > Batch comments', :js do
     wait_for_requests
   end
 end
+
+def write_reply_to_discussion(button_text: 'Start a review', text: 'Line is wrong', resolve: false, unresolve: false)
+  page.within('.discussion-reply-holder') do
+    click_button('Reply...')
+
+    fill_in('note_note', with: text)
+
+    if resolve
+      page.check('Resolve discussion')
+    end
+
+    if unresolve
+      page.check('Unresolve discussion')
+    end
+
+    click_button(button_text)
+  end
+
+  wait_for_requests
+end