Merge branch '50161-hide-close-mr-button-when-merged' into 'master'

Hides Close MR button on merged MR

Closes #50161

See merge request gitlab-org/gitlab-ce!21840

app/helpers/issuables_helper.rb

@@ -327,11 +327,15 @@ module IssuablesHelper
   end
 
   def issuable_button_visibility(issuable, closed)
+    return 'hidden' if issuable_button_hidden?(issuable, closed)
+  end
+
+  def issuable_button_hidden?(issuable, closed)
     case issuable
     when Issue
-      issue_button_visibility(issuable, closed)
+      issue_button_hidden?(issuable, closed)
     when MergeRequest
-      merge_request_button_visibility(issuable, closed)
+      merge_request_button_hidden?(issuable, closed)
     end
   end
 

app/helpers/issues_helper.rb

@@ -64,7 +64,11 @@ module IssuesHelper
   end
 
   def issue_button_visibility(issue, closed)
-    return 'hidden' if issue.closed? == closed
+    return 'hidden' if issue_button_hidden?(issue, closed)
+  end
+
+  def issue_button_hidden?(issue, closed)
+    issue.closed? == closed || (!closed && issue.discussion_locked)
   end
 
   def confidential_icon(issue)

app/helpers/merge_requests_helper.rb

@@ -80,7 +80,11 @@ module MergeRequestsHelper
   end
 
   def merge_request_button_visibility(merge_request, closed)
-    return 'hidden' if merge_request.closed? == closed || (merge_request.merged? == closed && !merge_request.closed?) || merge_request.closed_without_fork?
+    return 'hidden' if merge_request_button_hidden?(merge_request, closed)
+  end
+
+  def merge_request_button_hidden?(merge_request, closed)
+    merge_request.closed? == closed || (merge_request.merged? == closed && !merge_request.closed?) || merge_request.closed_without_fork?
   end
 
   def merge_request_version_path(project, merge_request, merge_request_diff, start_sha = nil)

app/views/shared/issuable/_close_reopen_button.html.haml

@@ -1,16 +1,18 @@
 - is_current_user = issuable_author_is_current_user(issuable)
 - display_issuable_type = issuable_display_type(issuable)
 - button_method = issuable_close_reopen_button_method(issuable)
+- are_close_and_open_buttons_hidden = issuable_button_hidden?(issuable, true) && issuable_button_hidden?(issuable, false)
 
-- if can_update
-  - if is_current_user
+- if is_current_user
+  - if can_update
     = link_to "Close #{display_issuable_type}", close_issuable_path(issuable), method: button_method,
               class: "d-none d-sm-none d-md-block btn btn-grouped btn-close js-btn-issue-action #{issuable_button_visibility(issuable, true)}", title: "Close #{display_issuable_type}"
-  - else
-    = render 'shared/issuable/close_reopen_report_toggle', issuable: issuable
-  - if can_reopen && is_current_user
+  - if can_reopen
     = link_to "Reopen #{display_issuable_type}", reopen_issuable_path(issuable), method: button_method,
               class: "d-none d-sm-none d-md-block btn btn-grouped btn-reopen js-btn-issue-action #{issuable_button_visibility(issuable, false)}", title: "Reopen #{display_issuable_type}"
 - else
-  = link_to 'Report abuse', new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)),
-            class: 'd-none d-sm-none d-md-block btn btn-grouped btn-close-color', title: 'Report abuse'
+  - if can_update && !are_close_and_open_buttons_hidden
+    = render 'shared/issuable/close_reopen_report_toggle', issuable: issuable
+  - else
+    = link_to 'Report abuse', new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)),
+              class: 'd-none d-sm-none d-md-block btn btn-grouped btn-close-color', title: 'Report abuse'

changelogs/unreleased/50161-hide-close-mr-button-when-merged.yml

@@ -0,0 +1,5 @@
+---
+title: Hides Close Merge request btn on merged Merge request
+merge_request: 21840
+author: Jacopo Beschi @jacopo-beschi
+type: fixed

spec/factories/issues.rb

@@ -13,6 +13,10 @@ FactoryBot.define do
       state :opened
     end
 
+    trait :locked do
+      discussion_locked true
+    end
+
     trait :closed do
       state :closed
       closed_at { Time.now }

spec/features/issuables/close_reopen_report_toggle_spec.rb

@@ -56,6 +56,24 @@ describe 'Issuables Close/Reopen/Report toggle' do
       end
 
       it_behaves_like 'an issuable close/reopen/report toggle'
+
+      context 'when the issue is closed and locked' do
+        let(:issuable) { create(:issue, :closed, :locked, project: project) }
+
+        it 'hides the reopen button' do
+          expect(page).not_to have_link('Reopen issue')
+        end
+
+        context 'when the issue author is the current user' do
+          before do
+            issuable.update(author: user)
+          end
+
+          it 'hides the reopen button' do
+            expect(page).not_to have_link('Reopen issue')
+          end
+        end
+      end
     end
 
     context 'when user doesnt have permission to update' do
@@ -93,6 +111,28 @@ describe 'Issuables Close/Reopen/Report toggle' do
       end
 
       it_behaves_like 'an issuable close/reopen/report toggle'
+
+      context 'when the merge request is merged' do
+        let(:issuable) { create(:merge_request, :merged, source_project: project) }
+
+        it 'shows only the `Report abuse` and `Edit` button' do
+          expect(page).to have_link('Report abuse')
+          expect(page).to have_link('Edit')
+          expect(page).not_to have_link('Close merge request')
+          expect(page).not_to have_link('Reopen merge request')
+        end
+
+        context 'when the merge request author is the current user' do
+          let(:issuable) { create(:merge_request, :merged, source_project: project, author: user) }
+
+          it 'shows only the `Edit` button' do
+            expect(page).to have_link('Edit')
+            expect(page).not_to have_link('Report abuse')
+            expect(page).not_to have_link('Close merge request')
+            expect(page).not_to have_link('Reopen merge request')
+          end
+        end
+      end
     end
 
     context 'when user doesnt have permission to update' do

spec/policies/issue_policy_spec.rb

@@ -112,7 +112,7 @@ describe IssuePolicy do
     let(:project) { create(:project, :public) }
     let(:issue) { create(:issue, project: project, assignees: [assignee], author: author) }
     let(:issue_no_assignee) { create(:issue, project: project) }
-    let(:issue_locked) { create(:issue, project: project, discussion_locked: true, author: author, assignees: [assignee]) }
+    let(:issue_locked) { create(:issue, :locked, project: project, author: author, assignees: [assignee]) }
 
     before do
       project.add_guest(guest)