summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJacopo <beschi.jacopo@gmail.com>2018-09-20 16:41:15 +0200
committerJacopo <beschi.jacopo@gmail.com>2018-10-02 16:17:55 +0200
commite2056f08f072805a132bf18879749e401d8ad620 (patch)
treeb6f7011f7df982cb6c70d604beeef3c656250f7a
parentba66e0cc9cc26df686ed47d926a3edcde497baa1 (diff)
downloadgitlab-ce-50161-hide-close-mr-button-when-merged.tar.gz
Hides Close MR button on merged MR50161-hide-close-mr-button-when-merged
When a Merge request is merged, shows only the Report abuse menu item in the dropdown menu instead of showing the close_reopen_report toggle with an unusable Close button. The Report abuse is still hidden when the author of the Merge request is the current_user. Hides the Reopen button on a closed and locked issue when the issue.author is not the current_user
-rw-r--r--app/helpers/issuables_helper.rb8
-rw-r--r--app/helpers/issues_helper.rb6
-rw-r--r--app/helpers/merge_requests_helper.rb6
-rw-r--r--app/views/shared/issuable/_close_reopen_button.html.haml16
-rw-r--r--changelogs/unreleased/50161-hide-close-mr-button-when-merged.yml5
-rw-r--r--spec/factories/issues.rb4
-rw-r--r--spec/features/issuables/close_reopen_report_toggle_spec.rb40
-rw-r--r--spec/policies/issue_policy_spec.rb2
8 files changed, 75 insertions, 12 deletions
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 56f6686da57..97406fefd43 100644
--- a/app/helpers/issuables_helper.rb
+++ b/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
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index f7d448ea3a7..957ab06b0ca 100644
--- a/app/helpers/issues_helper.rb
+++ b/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)
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index 87af6fb08f0..23d7aa427bb 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/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)
diff --git a/app/views/shared/issuable/_close_reopen_button.html.haml b/app/views/shared/issuable/_close_reopen_button.html.haml
index 70e05eb1c8c..4f6a71b6071 100644
--- a/app/views/shared/issuable/_close_reopen_button.html.haml
+++ b/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'
diff --git a/changelogs/unreleased/50161-hide-close-mr-button-when-merged.yml b/changelogs/unreleased/50161-hide-close-mr-button-when-merged.yml
new file mode 100644
index 00000000000..e6dd78a7a19
--- /dev/null
+++ b/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
diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb
index 4d4f74e101e..ab27fee33dc 100644
--- a/spec/factories/issues.rb
+++ b/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 }
diff --git a/spec/features/issuables/close_reopen_report_toggle_spec.rb b/spec/features/issuables/close_reopen_report_toggle_spec.rb
index de6f5fe1560..26c44baeb21 100644
--- a/spec/features/issuables/close_reopen_report_toggle_spec.rb
+++ b/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
diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb
index 93e85b3a6fa..008d118b557 100644
--- a/spec/policies/issue_policy_spec.rb
+++ b/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)