Merge branch '3525_allow_removal_for_cloud_licenses' into 'master'

Allow removal of cloud licenses

See merge request gitlab-org/gitlab!71481

ee/app/assets/javascripts/admin/subscriptions/show/components/subscription_breakdown.vue

@@ -93,7 +93,7 @@ export default {
       return this.licenseUploadPath && this.isLicenseFileType;
     },
     canRemoveLicense() {
-      return this.licenseRemovePath && this.isLicenseFileType;
+      return this.licenseRemovePath;
     },
     hasSubscription() {
       return Boolean(Object.keys(this.subscription).length);

ee/app/controllers/admin/licenses_controller.rb

@@ -43,10 +43,6 @@ class Admin::LicensesController < Admin::ApplicationController
       flash[:alert] = _('The license was removed. GitLab now no longer has a valid license.')
     end
 
-    redirect_to admin_subscription_path, status: :found
-  rescue Licenses::DestroyService::DestroyCloudLicenseError => e
-    flash[:error] = e.message
-
     redirect_to admin_subscription_path, status: :found
   end
 

ee/app/services/licenses/destroy_service.rb

@@ -4,13 +4,10 @@ module Licenses
   class DestroyService < ::Licenses::BaseService
     extend ::Gitlab::Utils::Override
 
-    DestroyCloudLicenseError = Class.new(StandardError)
-
     override :execute
     def execute
       raise ActiveRecord::RecordNotFound unless license
       raise Gitlab::Access::AccessDeniedError unless can?(user, :destroy_licenses)
-      raise DestroyCloudLicenseError, _('Cloud licenses can not be removed.') if license.cloud_license?
 
       license.destroy
     end

ee/lib/api/license.rb

@@ -6,10 +6,6 @@ module API
 
     feature_category :license
 
-    rescue_from Licenses::DestroyService::DestroyCloudLicenseError do |e|
-      render_api_error!(e.message, 422)
-    end
-
     resource :license do
       desc 'Get information on the currently active license' do
         success EE::API::Entities::GitlabLicenseWithActiveUsers

ee/spec/controllers/admin/licenses_controller_spec.rb

@@ -124,28 +124,26 @@ RSpec.describe Admin::LicensesController do
       allow(License).to receive(:current).and_return(create(:license, cloud: is_cloud_license))
     end
 
-    context 'with a cloud license' do
-      let(:is_cloud_license) { true }
-
-      it 'is can not be removed' do
+    shared_examples 'license removal' do
+      it 'removes the license' do
         delete :destroy
 
         expect(response).to redirect_to(admin_subscription_path)
-        expect(flash[:error]).to match('Cloud licenses can not be removed.')
-        expect(cloud_licenses).to be_present
+        expect(flash[:notice]).to match('The license was removed. GitLab has fallen back on the previous license.')
+        expect(cloud_licenses).to be_empty
       end
     end
 
+    context 'with a cloud license' do
+      let(:is_cloud_license) { true }
+
+      it_behaves_like 'license removal'
+    end
+
     context 'with a legacy license' do
       let(:is_cloud_license) { false }
 
-      it 'is can be removed' do
-        delete :destroy
-
-        expect(response).to redirect_to(admin_subscription_path)
-        expect(flash[:notice]).to match('The license was removed. GitLab has fallen back on the previous license.')
-        expect(cloud_licenses).to be_empty
-      end
+      it_behaves_like 'license removal'
     end
   end
 end

ee/spec/features/admin/subscriptions/admin_views_subscription_spec.rb

@@ -16,6 +16,22 @@ RSpec.describe 'Admin views Subscription', :js do
     end
   end
 
+  shared_examples 'license removal' do
+    context 'when removing a license file' do
+      before do
+        accept_alert do
+          click_on 'Remove license'
+        end
+      end
+
+      it 'shows a message saying the license was correctly removed' do
+        page.within(find('#content-body', match: :first)) do
+          expect(page).to have_content('The license was removed.')
+        end
+      end
+    end
+  end
+
   context 'with a cloud license' do
     let!(:license) { create_current_license(cloud_licensing_enabled: true, plan: License::ULTIMATE_PLAN) }
 
@@ -51,6 +67,7 @@ RSpec.describe 'Admin views Subscription', :js do
       end
 
       it_behaves_like 'an "Export license usage file" button'
+      it_behaves_like 'license removal'
     end
   end
 
@@ -62,20 +79,7 @@ RSpec.describe 'Admin views Subscription', :js do
     end
 
     it_behaves_like 'an "Export license usage file" button'
-
-    context 'when removing a license file' do
-      before do
-        accept_alert do
-          click_on 'Remove license'
-        end
-      end
-
-      it 'shows a message saying the license was correctly removed' do
-        page.within(find('#content-body', match: :first)) do
-          expect(page).to have_content('The license was removed.')
-        end
-      end
-    end
+    it_behaves_like 'license removal'
 
     context 'when activating another subscription' do
       before do

ee/spec/frontend/admin/subscriptions/components/subscription_breakdown_spec.js

@@ -255,7 +255,7 @@ describe('Subscription Breakdown', () => {
       it.each`
         url                  | type                              | shouldShow
         ${licenseRemovePath} | ${subscriptionTypes.LICENSE_FILE} | ${true}
-        ${licenseRemovePath} | ${subscriptionTypes.CLOUD}        | ${false}
+        ${licenseRemovePath} | ${subscriptionTypes.CLOUD}        | ${true}
         ${''}                | ${subscriptionTypes.LICENSE_FILE} | ${false}
         ${''}                | ${subscriptionTypes.CLOUD}        | ${false}
         ${undefined}         | ${subscriptionTypes.LICENSE_FILE} | ${false}

ee/spec/requests/api/license_spec.rb

@@ -86,14 +86,18 @@ RSpec.describe API::License, api: true do
 
     let(:endpoint) { "/license/#{license.id}" }
 
-    it 'destroys a license and returns 204' do
-      delete api(endpoint, admin)
+    shared_examples 'license removal' do
+      it 'destroys a license and returns 204' do
+        delete api(endpoint, admin)
 
-      expect(response).to have_gitlab_http_status(:no_content)
-      expect(response.message).to eq('No Content')
-      expect(License.where(id: license.id)).not_to exist
+        expect(response).to have_gitlab_http_status(:no_content)
+        expect(response.message).to eq('No Content')
+        expect(License.where(id: license.id)).not_to exist
+      end
     end
 
+    it_behaves_like 'license removal'
+
     it "returns an error if the license doesn't exist" do
       delete api("/license/0", admin)
 
@@ -111,12 +115,7 @@ RSpec.describe API::License, api: true do
     context 'with a cloud license' do
       let(:cloud_licensing_enabled) { true }
 
-      it 'returns 422 and does not delete the license' do
-        delete api(endpoint, admin)
-
-        expect(response).to have_gitlab_http_status(:unprocessable_entity)
-        expect(License.where(id: license.id)).to exist
-      end
+      it_behaves_like 'license removal'
     end
   end
 

ee/spec/services/licenses/destroy_service_spec.rb

@@ -16,6 +16,16 @@ RSpec.describe Licenses::DestroyService do
 
       expect(License.where(id: license.id)).not_to exist
     end
+
+    context 'with cloud license' do
+      let(:license) { create(:license, cloud_licensing_enabled: true, plan: License::ULTIMATE_PLAN) }
+
+      it 'destroys a license' do
+        destroy_with(user)
+
+        expect(License.where(id: license.id)).not_to exist
+      end
+    end
   end
 
   context 'when admin mode is disabled' do

locale/gitlab.pot

@@ -7204,9 +7204,6 @@ msgstr ""
 msgid "Cloud Run description and apps that are suitable for this deployment target"
 msgstr ""
 
-msgid "Cloud licenses can not be removed."
-msgstr ""
-
 msgid "Cluster"
 msgstr ""