Commit e61cce36 authored by Sean McGivern's avatar Sean McGivern

Merge branch '56864-reopen-locked-mr' into 'master'

Disallow reopening of locked merge request

Closes #56864

See merge request gitlab-org/gitlab-ce!24882
parents 66481881 eba4b940
...@@ -17,6 +17,7 @@ class IssuablePolicy < BasePolicy ...@@ -17,6 +17,7 @@ class IssuablePolicy < BasePolicy
enable :reopen_issue enable :reopen_issue
enable :read_merge_request enable :read_merge_request
enable :update_merge_request enable :update_merge_request
enable :reopen_merge_request
end end
rule { locked & ~is_project_member }.policy do rule { locked & ~is_project_member }.policy do
......
# frozen_string_literal: true # frozen_string_literal: true
class MergeRequestPolicy < IssuablePolicy class MergeRequestPolicy < IssuablePolicy
rule { locked }.policy do
prevent :reopen_merge_request
end
end end
...@@ -231,6 +231,7 @@ class ProjectPolicy < BasePolicy ...@@ -231,6 +231,7 @@ class ProjectPolicy < BasePolicy
enable :admin_merge_request enable :admin_merge_request
enable :admin_milestone enable :admin_milestone
enable :update_merge_request enable :update_merge_request
enable :reopen_merge_request
enable :create_commit_status enable :create_commit_status
enable :update_commit_status enable :update_commit_status
enable :create_build enable :create_build
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module MergeRequests module MergeRequests
class ReopenService < MergeRequests::BaseService class ReopenService < MergeRequests::BaseService
def execute(merge_request) def execute(merge_request)
return merge_request unless can?(current_user, :update_merge_request, merge_request) return merge_request unless can?(current_user, :reopen_merge_request, merge_request)
if merge_request.reopen if merge_request.reopen
create_event(merge_request) create_event(merge_request)
......
- can_update_merge_request = can?(current_user, :update_merge_request, @merge_request) - can_update_merge_request = can?(current_user, :update_merge_request, @merge_request)
- can_reopen_merge_request = can?(current_user, :reopen_merge_request, @merge_request)
- if @merge_request.closed_without_fork? - if @merge_request.closed_without_fork?
.alert.alert-danger .alert.alert-danger
...@@ -33,10 +34,11 @@ ...@@ -33,10 +34,11 @@
- if can_update_merge_request - if can_update_merge_request
%li{ class: [merge_request_button_visibility(@merge_request, true), 'js-close-item'] } %li{ class: [merge_request_button_visibility(@merge_request, true), 'js-close-item'] }
= link_to 'Close', merge_request_path(@merge_request, merge_request: { state_event: :close }), method: :put, title: 'Close merge request' = link_to 'Close', merge_request_path(@merge_request, merge_request: { state_event: :close }), method: :put, title: 'Close merge request'
- if can_reopen_merge_request
%li{ class: merge_request_button_visibility(@merge_request, false) } %li{ class: merge_request_button_visibility(@merge_request, false) }
= link_to 'Reopen', merge_request_path(@merge_request, merge_request: { state_event: :reopen }), method: :put, class: 'reopen-mr-link', title: 'Reopen merge request' = link_to 'Reopen', merge_request_path(@merge_request, merge_request: { state_event: :reopen }), method: :put, class: 'reopen-mr-link', title: 'Reopen merge request'
- if can_update_merge_request - if can_update_merge_request
= link_to 'Edit', edit_project_merge_request_path(@project, @merge_request), class: "d-none d-sm-none d-md-block btn btn-grouped js-issuable-edit qa-edit-button" = link_to 'Edit', edit_project_merge_request_path(@project, @merge_request), class: "d-none d-sm-none d-md-block btn btn-grouped js-issuable-edit qa-edit-button"
= render 'shared/issuable/close_reopen_button', issuable: @merge_request, can_update: can_update_merge_request, can_reopen: can_update_merge_request = render 'shared/issuable/close_reopen_button', issuable: @merge_request, can_update: can_update_merge_request, can_reopen: can_reopen_merge_request
---
title: Disallow reopening of a locked merge request
merge_request: 24882
author: Jan Beckmann
type: fixed
...@@ -276,7 +276,7 @@ edit existing comments. Non-team members are restricted from adding or editing c ...@@ -276,7 +276,7 @@ edit existing comments. Non-team members are restricted from adding or editing c
| :-----------: | :----------: | | :-----------: | :----------: |
| ![Comment form member](img/lock_form_member.png) | ![Comment form non-member](img/lock_form_non_member.png) | | ![Comment form member](img/lock_form_member.png) | ![Comment form non-member](img/lock_form_non_member.png) |
Additionally locked issues can not be reopened. Additionally, locked issues and merge requests can not be reopened.
## Filtering notes ## Filtering notes
......
...@@ -13,7 +13,7 @@ describe IssuablePolicy, models: true do ...@@ -13,7 +13,7 @@ describe IssuablePolicy, models: true do
context 'when user is able to read project' do context 'when user is able to read project' do
it 'enables user to read and update issuables' do it 'enables user to read and update issuables' do
expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request) expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request)
end end
end end
...@@ -24,12 +24,12 @@ describe IssuablePolicy, models: true do ...@@ -24,12 +24,12 @@ describe IssuablePolicy, models: true do
it 'enables user to read and update issuables' do it 'enables user to read and update issuables' do
project.add_maintainer(user) project.add_maintainer(user)
expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request) expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request)
end end
end end
it 'disallows user from reading and updating issuables from that project' do it 'disallows user from reading and updating issuables from that project' do
expect(policies).to be_disallowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request) expect(policies).to be_disallowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request)
end end
end end
end end
......
require 'spec_helper'
describe MergeRequestPolicy do
let(:guest) { create(:user) }
let(:author) { create(:user) }
let(:developer) { create(:user) }
let(:project) { create(:project, :public) }
def permissions(user, merge_request)
described_class.new(user, merge_request)
end
before do
project.add_guest(guest)
project.add_guest(author)
project.add_developer(developer)
end
context 'when merge request is unlocked' do
let(:merge_request) { create(:merge_request, :closed, source_project: project, target_project: project, author: author) }
it 'allows author to reopen merge request' do
expect(permissions(author, merge_request)).to be_allowed(:reopen_merge_request)
end
it 'allows developer to reopen merge request' do
expect(permissions(developer, merge_request)).to be_allowed(:reopen_merge_request)
end
it 'prevents guest from reopening merge request' do
expect(permissions(guest, merge_request)).to be_disallowed(:reopen_merge_request)
end
end
context 'when merge request is locked' do
let(:merge_request_locked) { create(:merge_request, :closed, discussion_locked: true, source_project: project, target_project: project, author: author) }
it 'prevents author from reopening merge request' do
expect(permissions(author, merge_request_locked)).to be_disallowed(:reopen_merge_request)
end
it 'prevents developer from reopening merge request' do
expect(permissions(developer, merge_request_locked)).to be_disallowed(:reopen_merge_request)
end
it 'prevents guests from reopening merge request' do
expect(permissions(guest, merge_request_locked)).to be_disallowed(:reopen_merge_request)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment