Merge branch 'epic-link-destroy' into 'master'

Allow to remove subepics if epics are available

See merge request gitlab-org/gitlab!36346

ee/app/controllers/groups/epic_links_controller.rb

@@ -20,6 +20,12 @@ class Groups::EpicLinksController < Groups::ApplicationController
 
   private
 
+  def authorize_admin!
+    return super unless action_name == 'destroy'
+
+    render_403 unless can?(current_user, 'destroy_epic_link', epic)
+  end
+
   def create_service
     EpicLinks::CreateService.new(epic, current_user, create_params)
   end

ee/app/policies/ee/group_policy.rb

@@ -146,6 +146,7 @@ module EE
         enable :admin_epic
         enable :update_epic
         enable :read_confidential_epic
+        enable :destroy_epic_link
       end
 
       rule { reporter & subepics_available }.policy do

ee/changelogs/unreleased/epic-link-destroy.yml

+---
+title: Allow to remove subepics when user downgrades its license.
+merge_request:
+author:
+type: fixed

ee/lib/api/epic_links.rb

@@ -95,8 +95,7 @@ module API
         use :child_epic_id
       end
       delete ':id/(-/)epics/:epic_iid/epics/:child_epic_id' do
-        authorize_subepics_feature!
-        authorize_can_admin_epic_link!
+        authorize_can_destroy_epic_link!
 
         updated_epic = ::Epics::UpdateService.new(user_group, current_user, { parent: nil }).execute(child_epic)
 

ee/lib/api/helpers/epics_helpers.rb

@@ -23,6 +23,10 @@ module API
         authorize!(:admin_epic_link, epic)
       end
 
+      def authorize_can_destroy_epic_link!
+        authorize!(:destroy_epic_link, epic)
+      end
+
       def authorize_can_create!
         authorize!(:admin_epic, user_group)
       end

ee/spec/controllers/groups/epic_links_controller_spec.rb

@@ -182,9 +182,9 @@ RSpec.describe Groups::EpicLinksController do
 
     it_behaves_like 'unlicensed subepics action'
 
-    context 'when subepics are enabled' do
+    context 'when epics are enabled' do
       before do
-        stub_licensed_features(epics: true, subepics: true)
+        stub_licensed_features(epics: true)
       end
 
       context 'when user has permissions to update the parent epic' do
@@ -223,5 +223,18 @@ RSpec.describe Groups::EpicLinksController do
         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

ee/spec/policies/group_policy_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe GroupPolicy do
   context 'when epics feature is disabled' do
     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
 
   context 'when epics feature is enabled' do
@@ -19,27 +19,27 @@ RSpec.describe GroupPolicy do
     context 'when user is owner' do
       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
 
     context 'when user is maintainer' do
       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) }
     end
 
     context 'when user is developer' do
       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) }
     end
 
     context 'when user is reporter' do
       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) }
     end
 
@@ -47,19 +47,19 @@ RSpec.describe GroupPolicy do
       let(:current_user) { guest }
 
       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
 
     context 'when user is not member' do
       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
 
     context 'when user is anonymous' do
       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
 

ee/spec/requests/api/epic_links_spec.rb

@@ -214,14 +214,15 @@ RSpec.describe API::EpicLinks do
   describe 'DELETE /groups/:id/epics/:epic_iid/epics' do
     let!(:child_epic) { create(:epic, group: group, parent: epic)}
     let(:url) { "/groups/#{group.path}/epics/#{epic.iid}/epics/#{child_epic.id}" }
+    let(:features_when_forbidden) { { epics: false } }
 
     subject { delete api(url, user) }
 
     it_behaves_like 'user does not have access'
 
-    context 'when subepics feature is enabled' do
+    context 'when epics feature is enabled' do
       before do
-        stub_licensed_features(epics: true, subepics: true)
+        stub_licensed_features(epics: true)
       end
 
       context 'when user is guest' do
@@ -246,5 +247,23 @@ RSpec.describe API::EpicLinks do
         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