Commit 95a7555e authored by Jan Provaznik's avatar Jan Provaznik

Allow to remove subepics if epics are available

If user downgrades its license, it should be still possible to
to remove existing subepics.
parent 97083e16
...@@ -20,6 +20,12 @@ class Groups::EpicLinksController < Groups::ApplicationController ...@@ -20,6 +20,12 @@ class Groups::EpicLinksController < Groups::ApplicationController
private private
def authorize_admin!
return super unless action_name == 'destroy'
render_403 unless can?(current_user, 'destroy_epic_link', epic)
end
def create_service def create_service
EpicLinks::CreateService.new(epic, current_user, create_params) EpicLinks::CreateService.new(epic, current_user, create_params)
end end
......
...@@ -146,6 +146,7 @@ module EE ...@@ -146,6 +146,7 @@ module EE
enable :admin_epic enable :admin_epic
enable :update_epic enable :update_epic
enable :read_confidential_epic enable :read_confidential_epic
enable :destroy_epic_link
end end
rule { reporter & subepics_available }.policy do rule { reporter & subepics_available }.policy do
......
---
title: Allow to remove subepics when user downgrades its license.
merge_request:
author:
type: fixed
...@@ -95,8 +95,7 @@ module API ...@@ -95,8 +95,7 @@ module API
use :child_epic_id use :child_epic_id
end end
delete ':id/(-/)epics/:epic_iid/epics/:child_epic_id' do delete ':id/(-/)epics/:epic_iid/epics/:child_epic_id' do
authorize_subepics_feature! authorize_can_destroy_epic_link!
authorize_can_admin_epic_link!
updated_epic = ::Epics::UpdateService.new(user_group, current_user, { parent: nil }).execute(child_epic) updated_epic = ::Epics::UpdateService.new(user_group, current_user, { parent: nil }).execute(child_epic)
......
...@@ -23,6 +23,10 @@ module API ...@@ -23,6 +23,10 @@ module API
authorize!(:admin_epic_link, epic) authorize!(:admin_epic_link, epic)
end end
def authorize_can_destroy_epic_link!
authorize!(:destroy_epic_link, epic)
end
def authorize_can_create! def authorize_can_create!
authorize!(:admin_epic, user_group) authorize!(:admin_epic, user_group)
end end
......
...@@ -182,9 +182,9 @@ RSpec.describe Groups::EpicLinksController do ...@@ -182,9 +182,9 @@ RSpec.describe Groups::EpicLinksController do
it_behaves_like 'unlicensed subepics action' it_behaves_like 'unlicensed subepics action'
context 'when subepics are enabled' do context 'when epics are enabled' do
before do before do
stub_licensed_features(epics: true, subepics: true) stub_licensed_features(epics: true)
end end
context 'when user has permissions to update the parent epic' do context 'when user has permissions to update the parent epic' do
...@@ -223,5 +223,18 @@ RSpec.describe Groups::EpicLinksController do ...@@ -223,5 +223,18 @@ RSpec.describe Groups::EpicLinksController do
end end
end end
end end
context 'when user has permissions to update the parent epic but epics feature is disabled' do
before do
stub_licensed_features(epics: false)
group.add_developer(user)
end
it 'does not destroy the link' do
expect { subject }.not_to change { epic1.reload.parent }.from(parent_epic)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end end
end end
...@@ -8,7 +8,7 @@ RSpec.describe GroupPolicy do ...@@ -8,7 +8,7 @@ RSpec.describe GroupPolicy do
context 'when epics feature is disabled' do context 'when epics feature is disabled' do
let(:current_user) { owner } let(:current_user) { owner }
it { is_expected.to be_disallowed(:read_epic, :create_epic, :admin_epic, :destroy_epic, :read_confidential_epic) } it { is_expected.to be_disallowed(:read_epic, :create_epic, :admin_epic, :destroy_epic, :read_confidential_epic, :destroy_epic_link) }
end end
context 'when epics feature is enabled' do context 'when epics feature is enabled' do
...@@ -19,27 +19,27 @@ RSpec.describe GroupPolicy do ...@@ -19,27 +19,27 @@ RSpec.describe GroupPolicy do
context 'when user is owner' do context 'when user is owner' do
let(:current_user) { owner } let(:current_user) { owner }
it { is_expected.to be_allowed(:read_epic, :create_epic, :admin_epic, :destroy_epic, :read_confidential_epic) } it { is_expected.to be_allowed(:read_epic, :create_epic, :admin_epic, :destroy_epic, :read_confidential_epic, :destroy_epic_link) }
end end
context 'when user is maintainer' do context 'when user is maintainer' do
let(:current_user) { maintainer } let(:current_user) { maintainer }
it { is_expected.to be_allowed(:read_epic, :create_epic, :admin_epic, :read_confidential_epic) } it { is_expected.to be_allowed(:read_epic, :create_epic, :admin_epic, :read_confidential_epic, :destroy_epic_link) }
it { is_expected.to be_disallowed(:destroy_epic) } it { is_expected.to be_disallowed(:destroy_epic) }
end end
context 'when user is developer' do context 'when user is developer' do
let(:current_user) { developer } let(:current_user) { developer }
it { is_expected.to be_allowed(:read_epic, :create_epic, :admin_epic, :read_confidential_epic) } it { is_expected.to be_allowed(:read_epic, :create_epic, :admin_epic, :read_confidential_epic, :destroy_epic_link) }
it { is_expected.to be_disallowed(:destroy_epic) } it { is_expected.to be_disallowed(:destroy_epic) }
end end
context 'when user is reporter' do context 'when user is reporter' do
let(:current_user) { reporter } let(:current_user) { reporter }
it { is_expected.to be_allowed(:read_epic, :create_epic, :admin_epic, :read_confidential_epic) } it { is_expected.to be_allowed(:read_epic, :create_epic, :admin_epic, :read_confidential_epic, :destroy_epic_link) }
it { is_expected.to be_disallowed(:destroy_epic) } it { is_expected.to be_disallowed(:destroy_epic) }
end end
...@@ -47,19 +47,19 @@ RSpec.describe GroupPolicy do ...@@ -47,19 +47,19 @@ RSpec.describe GroupPolicy do
let(:current_user) { guest } let(:current_user) { guest }
it { is_expected.to be_allowed(:read_epic) } it { is_expected.to be_allowed(:read_epic) }
it { is_expected.to be_disallowed(:create_epic, :admin_epic, :destroy_epic, :read_confidential_epic) } it { is_expected.to be_disallowed(:create_epic, :admin_epic, :destroy_epic, :read_confidential_epic, :destroy_epic_link) }
end end
context 'when user is not member' do context 'when user is not member' do
let(:current_user) { create(:user) } let(:current_user) { create(:user) }
it { is_expected.to be_disallowed(:read_epic, :create_epic, :admin_epic, :destroy_epic, :read_confidential_epic) } it { is_expected.to be_disallowed(:read_epic, :create_epic, :admin_epic, :destroy_epic, :read_confidential_epic, :destroy_epic_link) }
end end
context 'when user is anonymous' do context 'when user is anonymous' do
let(:current_user) { nil } let(:current_user) { nil }
it { is_expected.to be_disallowed(:read_epic, :create_epic, :admin_epic, :destroy_epic, :read_confidential_epic) } it { is_expected.to be_disallowed(:read_epic, :create_epic, :admin_epic, :destroy_epic, :read_confidential_epic, :destroy_epic_link) }
end end
end end
......
...@@ -214,14 +214,15 @@ RSpec.describe API::EpicLinks do ...@@ -214,14 +214,15 @@ RSpec.describe API::EpicLinks do
describe 'DELETE /groups/:id/epics/:epic_iid/epics' do describe 'DELETE /groups/:id/epics/:epic_iid/epics' do
let!(:child_epic) { create(:epic, group: group, parent: epic)} let!(:child_epic) { create(:epic, group: group, parent: epic)}
let(:url) { "/groups/#{group.path}/epics/#{epic.iid}/epics/#{child_epic.id}" } let(:url) { "/groups/#{group.path}/epics/#{epic.iid}/epics/#{child_epic.id}" }
let(:features_when_forbidden) { { epics: false } }
subject { delete api(url, user) } subject { delete api(url, user) }
it_behaves_like 'user does not have access' it_behaves_like 'user does not have access'
context 'when subepics feature is enabled' do context 'when epics feature is enabled' do
before do before do
stub_licensed_features(epics: true, subepics: true) stub_licensed_features(epics: true)
end end
context 'when user is guest' do context 'when user is guest' do
...@@ -246,5 +247,23 @@ RSpec.describe API::EpicLinks do ...@@ -246,5 +247,23 @@ RSpec.describe API::EpicLinks do
end end
end end
end end
context 'when epics feature is disabled' do
before do
stub_licensed_features(epics: false)
end
context 'when user is developer' do
before do
group.add_developer(user)
end
it 'returns 403 status' do
subject
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
end 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