Merge branch '320756-remove-remove_resolve_note-feature-flag' into 'master'

Remove `remove_resolve_note` feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!57757

app/assets/javascripts/batch_comments/mixins/resolved_status.js

 import { mapGetters } from 'vuex';
 import { sprintf, s__, __ } from '~/locale';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 export default {
-  mixins: [glFeatureFlagsMixin()],
   props: {
     discussionId: {
       type: String,
@@ -54,11 +52,7 @@ export default {
     resolveButtonTitle() {
       if (this.isDraft || this.discussionId) return this.resolvedStatusMessage;
 
-      let title = __('Mark as resolved');
-
-      if (this.glFeatures.removeResolveNote) {
-        title = __('Resolve thread');
-      }
+      let title = __('Resolve thread');
 
       if (this.resolvedBy) {
         title = sprintf(__('Resolved by %{name}'), { name: this.resolvedBy.name });

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

@@ -86,7 +86,7 @@ export default {
       isRequesting: false,
       isResolving: false,
       commentLineStart: {},
-      resolveAsThread: this.glFeatures.removeResolveNote,
+      resolveAsThread: true,
     };
   },
   computed: {
@@ -139,14 +139,9 @@ export default {
       return this.note.isDraft;
     },
     canResolve() {
-      if (this.glFeatures.removeResolveNote && !this.discussionRoot) return false;
+      if (!this.discussionRoot) return false;
 
-      if (this.glFeatures.removeResolveNote) return this.note.current_user.can_resolve_discussion;
-
-      return (
-        this.note.current_user.can_resolve ||
-        (this.note.isDraft && this.note.discussion_id !== null)
-      );
+      return this.note.current_user.can_resolve_discussion;
     },
     lineRange() {
       return this.note.position?.line_range;

app/assets/javascripts/notes/mixins/resolvable.js

 import { deprecatedCreateFlash as Flash } from '~/flash';
 import { __ } from '~/locale';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 export default {
-  mixins: [glFeatureFlagsMixin()],
   computed: {
     discussionResolved() {
       if (this.discussion) {
-        const { notes, resolved } = this.discussion;
-
-        if (this.glFeatures.removeResolveNote) {
-          return Boolean(resolved);
-        }
-
-        if (notes) {
-          // Decide resolved state using store. Only valid for discussions.
-          return notes.filter((note) => !note.system).every((note) => note.resolved);
-        }
-
-        return resolved;
+        return Boolean(this.discussion.resolved);
       }
 
       return this.note.resolved;
@@ -47,7 +34,7 @@ export default {
       let endpoint =
         discussion && this.discussion ? this.discussion.resolve_path : `${this.note.path}/resolve`;
 
-      if (this.glFeatures.removeResolveNote && this.discussionResolvePath) {
+      if (this.discussionResolvePath) {
         endpoint = this.discussionResolvePath;
       }
 

app/controllers/projects/merge_requests_controller.rb

@@ -36,7 +36,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     push_frontend_feature_flag(:unified_diff_components, @project, default_enabled: true)
     push_frontend_feature_flag(:default_merge_ref_for_diffs, @project, default_enabled: :yaml)
     push_frontend_feature_flag(:core_security_mr_widget_counts, @project)
-    push_frontend_feature_flag(:remove_resolve_note, @project, default_enabled: true)
     push_frontend_feature_flag(:diffs_gradual_load, @project, default_enabled: true)
     push_frontend_feature_flag(:codequality_backend_comparison, @project, default_enabled: :yaml)
     push_frontend_feature_flag(:local_file_reviews, default_enabled: :yaml)

changelogs/unreleased/320756-remove-remove_resolve_note-feature-flag.yml

+---
+title: Remove remove_resolve_note feature flag
+merge_request: 57757
+author:
+type: other

config/feature_flags/development/remove_resolve_note.yml

----
-name: remove_resolve_note
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45549
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320756
-milestone: '13.6'
-type: development
-group: group::code review
-default_enabled: true

locale/gitlab.pot

@@ -18804,9 +18804,6 @@ msgstr ""
 msgid "Mark as ready"
 msgstr ""
 
-msgid "Mark as resolved"
-msgstr ""
-
 msgid "Mark this issue as a duplicate of another issue"
 msgstr ""
 

spec/features/discussion_comments/merge_request_spec.rb

@@ -8,8 +8,6 @@ RSpec.describe 'Thread Comments Merge Request', :js do
   let(:merge_request) { create(:merge_request, source_project: project) }
 
   before do
-    stub_feature_flags(remove_resolve_note: false)
-
     project.add_maintainer(user)
     sign_in(user)
 

spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb

@@ -18,10 +18,6 @@ RSpec.describe 'Resolving all open threads in a merge request from an issue', :j
     end
   end
 
-  before do
-    stub_feature_flags(remove_resolve_note: false)
-  end
-
   describe 'as a user with access to the project' do
     before do
       project.add_maintainer(user)
@@ -37,7 +33,7 @@ RSpec.describe 'Resolving all open threads in a merge request from an issue', :j
 
     context 'resolving the thread' do
       before do
-        click_button 'Resolve thread'
+        find('button[data-qa-selector="resolve_discussion_button"]').click
       end
 
       it 'hides the link for creating a new issue' do

spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb

@@ -14,10 +14,6 @@ RSpec.describe 'Resolve an open thread in a merge request by creating an issue',
     "a[title=\"#{title}\"][href=\"#{url}\"]"
   end
 
-  before do
-    stub_feature_flags(remove_resolve_note: false)
-  end
-
   describe 'As a user with access to the project' do
     before do
       project.add_maintainer(user)
@@ -39,7 +35,7 @@ RSpec.describe 'Resolve an open thread in a merge request by creating an issue',
 
     context 'resolving the thread' do
       before do
-        click_button 'Resolve thread'
+        find('button[data-qa-selector="resolve_discussion_button"]').click
       end
 
       it 'hides the link for creating a new issue' do

spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb

@@ -15,10 +15,6 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
           diff_refs: merge_request.diff_refs)
   end
 
-  before do
-    stub_feature_flags(remove_resolve_note: false)
-  end
-
   context 'no threads' do
     before do
       project.add_maintainer(user)
@@ -67,7 +63,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to mark thread as resolved' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
         end
 
         expect(page).to have_selector('.discussion-body', visible: false)
@@ -84,7 +80,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to unresolve thread' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
           click_button 'Unresolve thread'
         end
 
@@ -96,7 +92,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
       describe 'resolved thread' do
         before do
           page.within '.diff-content' do
-            click_button 'Resolve thread'
+            find('button[data-qa-selector="resolve_discussion_button"]').click
           end
 
           visit_merge_request
@@ -197,7 +193,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to resolve from reply form without a comment' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
         end
 
         page.within '.line-resolve-all-container' do
@@ -234,7 +230,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'hides jump to next button when all resolved' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
         end
 
         expect(page).to have_selector('.discussion-next-btn', visible: false)
@@ -264,7 +260,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
         visit_merge_request
       end
 
-      it 'does not mark thread as resolved when resolving single note' do
+      it 'marks thread as resolved when resolving single note' do
         page.within("#note_#{note.id}") do
           first('.line-resolve-btn').click
 
@@ -273,15 +269,13 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
           expect(first('.line-resolve-btn')['aria-label']).to eq("Resolved by #{user.name}")
         end
 
-        expect(page).to have_content('Last updated')
-
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1 unresolved thread')
+          expect(page).to have_content('All threads resolved')
         end
       end
 
       it 'resolves thread' do
-        resolve_buttons = page.all('.note .line-resolve-btn', count: 2)
+        resolve_buttons = page.all('.note .line-resolve-btn', count: 1)
         resolve_buttons.each do |button|
           button.click
         end
@@ -332,7 +326,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
       it 'allows user to mark all threads as resolved' do
         page.all('.discussion-reply-holder', count: 2).each do |reply_holder|
           page.within reply_holder do
-            click_button 'Resolve thread'
+            find('button[data-qa-selector="resolve_discussion_button"]').click
           end
         end
 
@@ -344,7 +338,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to quickly scroll to next unresolved thread' do
         page.within('.discussion-reply-holder', match: :first) do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
         end
 
         page.within '.line-resolve-all-container' do
@@ -416,7 +410,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to mark thread as resolved' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
         end
 
         page.within '.diff-content .note' do
@@ -431,7 +425,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to unresolve thread' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
           click_button 'Unresolve thread'
         end
 
@@ -459,7 +453,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to comment & unresolve thread' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
 
           find_field('Reply…').click
 

spec/frontend/notes/components/noteable_discussion_spec.js

@@ -124,14 +124,7 @@ describe('noteable_discussion component', () => {
         ...getJSONFixture(discussionWithTwoUnresolvedNotes)[0],
         expanded: true,
       };
-      discussion.notes = discussion.notes.map((note) => ({
-        ...note,
-        resolved: false,
-        current_user: {
-          ...note.current_user,
-          can_resolve: true,
-        },
-      }));
+      discussion.resolved = false;
 
       wrapper.setProps({ discussion });
 

spec/support/shared_examples/features/discussion_comments_shared_example.rb

@@ -304,7 +304,7 @@ RSpec.shared_examples 'thread comments for issue, epic and merge request' do |re
           let(:reply_id) { find("#{comments_selector} .note:last-of-type", match: :first)['data-note-id'] }
 
           it 'can be replied to after resolving' do
-            click_button "Resolve thread"
+            find('button[data-qa-selector="resolve_discussion_button"]').click
             wait_for_requests
 
             refresh
@@ -316,7 +316,7 @@ RSpec.shared_examples 'thread comments for issue, epic and merge request' do |re
           it 'shows resolved thread when toggled' do
             submit_reply('a')
 
-            click_button "Resolve thread"
+            find('button[data-qa-selector="resolve_discussion_button"]').click
             wait_for_requests
 
             expect(page).to have_selector(".note-row-#{note_id}", visible: true)