diff options
| -rw-r--r-- | app/helpers/issuables_helper.rb | 8 | ||||
| -rw-r--r-- | app/helpers/issues_helper.rb | 6 | ||||
| -rw-r--r-- | app/helpers/merge_requests_helper.rb | 6 | ||||
| -rw-r--r-- | app/views/shared/issuable/_close_reopen_button.html.haml | 16 | ||||
| -rw-r--r-- | changelogs/unreleased/50161-hide-close-mr-button-when-merged.yml | 5 | ||||
| -rw-r--r-- | spec/factories/issues.rb | 4 | ||||
| -rw-r--r-- | spec/features/issuables/close_reopen_report_toggle_spec.rb | 40 | ||||
| -rw-r--r-- | spec/policies/issue_policy_spec.rb | 2 |
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) |