Merge branch '198562-revert-removal-of-close-comment-button' into 'master'

Reverts removal of Comment & close" buttons on MR/issues [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!54913

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

@@ -180,9 +180,6 @@ export default {
     trackingLabel() {
       return slugifyWithUnderscore(`${this.commentButtonTitle} button`);
     },
-    hasCloseAndCommentButton() {
-      return !this.glFeatures.removeCommentCloseReopen;
-    },
     confidentialNotesEnabled() {
       return Boolean(this.glFeatures.confidentialNotes);
     },
@@ -426,7 +423,7 @@ export default {
                 </gl-dropdown-item>
               </gl-dropdown>
               <gl-button
-                v-if="hasCloseAndCommentButton && canToggleIssueState"
+                v-if="canToggleIssueState"
                 :loading="isToggleStateButtonLoading"
                 category="secondary"
                 :variant="buttonVariant"

app/controllers/concerns/comment_and_close_flag.rb

-# frozen_string_literal: true
-
-module CommentAndCloseFlag
-  extend ActiveSupport::Concern
-
-  included do
-    before_action do
-      push_frontend_feature_flag(:remove_comment_close_reopen, @group || @project&.group)
-    end
-  end
-end

app/controllers/projects/issues_controller.rb

@@ -9,7 +9,6 @@ class Projects::IssuesController < Projects::ApplicationController
   include IssuesCalendar
   include SpammableActions
   include RecordUserLastActivity
-  include CommentAndCloseFlag
 
   ISSUES_EXCEPT_ACTIONS = %i[index calendar new create bulk_update import_csv export_csv service_desk].freeze
   SET_ISSUEABLES_INDEX_ONLY_ACTIONS = %i[index calendar service_desk].freeze

app/controllers/projects/merge_requests_controller.rb

@@ -11,7 +11,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   include RecordUserLastActivity
   include SourcegraphDecorator
   include DiffHelper
-  include CommentAndCloseFlag
 
   skip_before_action :merge_request, only: [:index, :bulk_update, :export_csv]
   before_action :apply_diff_view_cookie!, only: [:show]

config/feature_flags/development/remove_comment_close_reopen.yml

----
-name: remove_comment_close_reopen
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49614
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/198562
-milestone: '13.9'
-type: development
-group: group::code review
-default_enabled: false

ee/app/controllers/groups/epics_controller.rb

@@ -8,7 +8,6 @@ class Groups::EpicsController < Groups::ApplicationController
   include RendersNotes
   include EpicsActions
   include DescriptionDiffActions
-  include CommentAndCloseFlag
 
   before_action :check_epics_available!
   before_action :epic, except: [:index, :create, :new, :bulk_update]

ee/spec/features/epics/epic_show_spec.rb

@@ -28,8 +28,6 @@ RSpec.describe 'Epic show', :js do
   let_it_be(:child_issue_a) { create(:epic_issue, epic: epic, issue: public_issue, relative_position: 1) }
 
   before do
-    stub_feature_flags(remove_comment_close_reopen: false)
-
     group.add_developer(user)
     stub_licensed_features(epics: true, subepics: true)
     sign_in(user)
@@ -294,15 +292,9 @@ RSpec.describe 'Epic show', :js do
     end
 
     describe 'when open' do
-      let(:open_epic) { create(:epic, group: group) }
-
-      it_behaves_like 'page with comment and close button', 'Close epic' do
-        def setup
-          visit group_epic_path(group, open_epic)
-        end
-      end
-
       context 'when clicking the top `Close epic` button', :aggregate_failures do
+        let(:open_epic) { create(:epic, group: group) }
+
         before do
           visit group_epic_path(group, open_epic)
         end
@@ -311,6 +303,8 @@ RSpec.describe 'Epic show', :js do
       end
 
       context 'when clicking the bottom `Close epic` button', :aggregate_failures do
+        let(:open_epic) { create(:epic, group: group) }
+
         before do
           visit group_epic_path(group, open_epic)
         end
@@ -320,15 +314,9 @@ RSpec.describe 'Epic show', :js do
     end
 
     describe 'when closed' do
-      let(:closed_epic) { create(:epic, group: group, state: 'closed') }
-
-      it_behaves_like 'page with comment and close button', 'Reopen epic' do
-        def setup
-          visit group_epic_path(group, closed_epic)
-        end
-      end
-
       context 'when clicking the top `Reopen epic` button', :aggregate_failures do
+        let(:closed_epic) { create(:epic, group: group, state: 'closed') }
+
         before do
           visit group_epic_path(group, closed_epic)
         end
@@ -337,6 +325,8 @@ RSpec.describe 'Epic show', :js do
       end
 
       context 'when clicking the bottom `Reopen epic` button', :aggregate_failures do
+        let(:closed_epic) { create(:epic, group: group, state: 'closed') }
+
         before do
           visit group_epic_path(group, closed_epic)
         end

ee/spec/features/issues/related_issues_spec.rb

@@ -123,23 +123,11 @@ RSpec.describe 'Related issues', :js do
           expect(find('.js-related-issues-header-issue-count')).to have_content('1')
         end
 
-        it_behaves_like 'page with comment and close button', 'Close issue' do
-          def setup
-            visit project_issue_path(project, issue_a)
-
-            wait_for_requests
-          end
-        end
-
         context 'when clicking the top `Close issue` button in the issue header', :aggregate_failures do
           it_behaves_like 'issue closed by modal', '.detail-page-header'
         end
 
         context 'when clicking the bottom `Close issue` button below the comment textarea', :aggregate_failures do
-          before do
-            stub_feature_flags(remove_comment_close_reopen: false)
-          end
-
           it_behaves_like 'issue closed by modal', '.new-note'
         end
       end

ee/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb

@@ -20,12 +20,15 @@ RSpec.describe 'Merge request > User selects branches for new MR', :js do
   context 'create a merge request for the selected branches' do
     before do
       visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature_conflict' })
-      fill_in 'merge_request_title', with: 'A Test MR'
-      click_button 'Submit merge request'
     end
 
-    it 'shows the saved MR' do
-      expect(page).to have_content('A Test MR')
+    context 'saving the MR' do
+      it 'shows the saved MR' do
+        fill_in 'merge_request_title', with: 'Test'
+        click_button 'Submit merge request'
+
+        expect(page).to have_button('Close merge request')
+      end
     end
   end
 end

spec/features/discussion_comments/issue_spec.rb

@@ -8,8 +8,6 @@ RSpec.describe 'Thread Comments Issue', :js do
   let(:issue) { create(:issue, project: project) }
 
   before do
-    stub_feature_flags(remove_comment_close_reopen: false)
-
     project.add_maintainer(user)
     sign_in(user)
 

spec/features/discussion_comments/merge_request_spec.rb

@@ -9,7 +9,6 @@ RSpec.describe 'Thread Comments Merge Request', :js do
 
   before do
     stub_feature_flags(remove_resolve_note: false)
-    stub_feature_flags(remove_comment_close_reopen: false)
 
     project.add_maintainer(user)
     sign_in(user)

spec/features/issues/issue_state_spec.rb

@@ -42,15 +42,9 @@ RSpec.describe 'issue state', :js do
   end
 
   describe 'when open', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/297348' do
-    let(:open_issue) { create(:issue, project: project) }
-
-    it_behaves_like 'page with comment and close button', 'Close issue' do
-      def setup
-        visit project_issue_path(project, open_issue)
-      end
-    end
-
     context 'when clicking the top `Close issue` button', :aggregate_failures do
+      let(:open_issue) { create(:issue, project: project) }
+
       before do
         visit project_issue_path(project, open_issue)
       end
@@ -59,8 +53,9 @@ RSpec.describe 'issue state', :js do
     end
 
     context 'when clicking the bottom `Close issue` button', :aggregate_failures do
+      let(:open_issue) { create(:issue, project: project) }
+
       before do
-        stub_feature_flags(remove_comment_close_reopen: false)
         visit project_issue_path(project, open_issue)
       end
 
@@ -69,15 +64,9 @@ RSpec.describe 'issue state', :js do
   end
 
   describe 'when closed', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/297201' do
-    let(:closed_issue) { create(:issue, project: project, state: 'closed') }
-
-    it_behaves_like 'page with comment and close button', 'Reopen issue' do
-      def setup
-        visit project_issue_path(project, closed_issue)
-      end
-    end
-
     context 'when clicking the top `Reopen issue` button', :aggregate_failures do
+      let(:closed_issue) { create(:issue, project: project, state: 'closed') }
+
       before do
         visit project_issue_path(project, closed_issue)
       end
@@ -86,8 +75,9 @@ RSpec.describe 'issue state', :js do
     end
 
     context 'when clicking the bottom `Reopen issue` button', :aggregate_failures do
+      let(:closed_issue) { create(:issue, project: project, state: 'closed') }
+
       before do
-        stub_feature_flags(remove_comment_close_reopen: false)
         visit project_issue_path(project, closed_issue)
       end
 

spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb

@@ -12,15 +12,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
   end
 
   describe 'when open' do
-    let(:open_merge_request) { create(:merge_request, source_project: project, target_project: project) }
-
-    it_behaves_like 'page with comment and close button', 'Close merge request' do
-      def setup
-        visit merge_request_path(open_merge_request)
-      end
-    end
-
     context 'when clicking the top `Close merge request` link', :aggregate_failures do
+      let(:open_merge_request) { create(:merge_request, source_project: project, target_project: project) }
+
       before do
         visit merge_request_path(open_merge_request)
       end
@@ -40,8 +34,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
     end
 
     context 'when clicking the bottom `Close merge request` button', :aggregate_failures do
+      let(:open_merge_request) { create(:merge_request, source_project: project, target_project: project) }
+
       before do
-        stub_feature_flags(remove_comment_close_reopen: false)
         visit merge_request_path(open_merge_request)
       end
 
@@ -61,22 +56,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
   end
 
   describe 'when closed' do
-    let(:closed_merge_request) { create(:merge_request, source_project: project, target_project: project, state: 'closed') }
-
-    it_behaves_like 'page with comment and close button', 'Close merge request' do
-      def setup
-        visit merge_request_path(closed_merge_request)
-
-        within '.detail-page-header' do
-          click_button 'Toggle dropdown'
-          click_link 'Reopen merge request'
-        end
-
-        wait_for_requests
-      end
-    end
-
     context 'when clicking the top `Reopen merge request` link', :aggregate_failures do
+      let(:closed_merge_request) { create(:merge_request, source_project: project, target_project: project, state: 'closed') }
+
       before do
         visit merge_request_path(closed_merge_request)
       end
@@ -96,8 +78,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
     end
 
     context 'when clicking the bottom `Reopen merge request` button', :aggregate_failures do
+      let(:closed_merge_request) { create(:merge_request, source_project: project, target_project: project, state: 'closed') }
+
       before do
-        stub_feature_flags(remove_comment_close_reopen: false)
         visit merge_request_path(closed_merge_request)
       end
 

spec/features/task_lists_spec.rb

@@ -69,13 +69,7 @@ RSpec.describe 'Task Lists', :js do
         wait_for_requests
 
         expect(page).to have_selector(".md .task-list .task-list-item .task-list-item-checkbox")
-      end
-
-      it_behaves_like 'page with comment and close button', 'Close issue' do
-        def setup
-          visit_issue(project, issue)
-          wait_for_requests
-        end
+        expect(page).to have_selector('.btn-close')
       end
 
       it 'is only editable by author' do

spec/support/shared_examples/features/comment_and_close_button_shared_examples.rb

-# frozen_string_literal: true
-
-RSpec.shared_examples 'page with comment and close button' do |button_text|
-  context 'when remove_comment_close_reopen feature flag is enabled' do
-    before do
-      stub_feature_flags(remove_comment_close_reopen: true)
-      setup
-    end
-
-    it "does not show #{button_text} button" do
-      within '.note-form-actions' do
-        expect(page).not_to have_button(button_text)
-      end
-    end
-  end
-
-  context 'when remove_comment_close_reopen feature flag is disabled' do
-    before do
-      stub_feature_flags(remove_comment_close_reopen: false)
-      setup
-    end
-
-    it "shows #{button_text} button" do
-      within '.note-form-actions' do
-        expect(page).to have_button(button_text)
-      end
-    end
-  end
-end