Merge branch 'change-delete-service' into 'master'

Do not delete instance_administrators_group

See merge request gitlab-org/gitlab!22768

lib/gitlab/database_importers/self_monitoring/project/delete_service.rb

@@ -9,8 +9,7 @@ module Gitlab
           include SelfMonitoring::Helpers
 
           steps :validate_self_monitoring_project_exists,
-            :destroy_project_owner,
-            :delete_project_id
+            :destroy_project
 
           def initialize
             super(nil)
@@ -30,29 +29,16 @@ module Gitlab
             success(result)
           end
 
-          def destroy_project_owner(result)
+          def destroy_project(result)
             return success(result) unless project_created?
 
-            if self_monitoring_project.owner.destroy
+            if self_monitoring_project.destroy
               success(result)
             else
               log_error(self_monitoring_project.errors.full_messages)
               error(_('Error deleting project. Check logs for error details.'))
             end
           end
-
-          def delete_project_id(result)
-            update_result = application_settings.update(
-              instance_administration_project_id: nil
-            )
-
-            if update_result
-              success(result)
-            else
-              log_error("Could not delete self monitoring project ID, errors: %{errors}" % { errors: application_settings.errors.full_messages })
-              error(_('Could not delete project ID'))
-            end
-          end
         end
       end
     end

locale/gitlab.pot

@@ -5187,9 +5187,6 @@ msgstr ""
 msgid "Could not delete chat nickname %{chat_name}."
 msgstr ""
 
-msgid "Could not delete project ID"
-msgstr ""
-
 msgid "Could not fetch projects"
 msgstr ""
 

spec/lib/gitlab/database_importers/self_monitoring/project/delete_service_spec.rb

@@ -4,12 +4,8 @@ require 'spec_helper'
 
 describe Gitlab::DatabaseImporters::SelfMonitoring::Project::DeleteService do
   describe '#execute' do
+    let!(:application_setting) { create(:application_setting) }
     let(:result) { subject.execute }
-    let(:application_setting) { Gitlab::CurrentSettings.current_application_settings }
-
-    before do
-      allow(ApplicationSetting).to receive(:current_without_cache) { application_setting }
-    end
 
     context 'when project does not exist' do
       it 'returns error' do
@@ -21,24 +17,16 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::DeleteService do
       end
     end
 
-    context 'with project destroyed but ID still present in application settings' do
-      before do
-        application_setting.instance_administration_project_id = 1
-      end
-
-      it 'deletes project ID from application settings' do
-        subject.execute
-
-        expect(application_setting.instance_administration_project_id).to be_nil
-      end
-    end
-
     context 'when self monitoring project exists' do
       let(:group) { create(:group) }
       let(:project) { create(:project, namespace: group) }
 
-      before do
-        application_setting.instance_administration_project = project
+      let(:application_setting) do
+        create(
+          :application_setting,
+          instance_administration_project_id: project.id,
+          instance_administrators_group_id: group.id
+        )
       end
 
       it 'destroys project' do
@@ -50,7 +38,13 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::DeleteService do
       it 'deletes project ID from application settings' do
         subject.execute
 
-        expect(application_setting.instance_administration_project_id).to be_nil
+        expect(application_setting.reload.instance_administration_project_id).to be_nil
+      end
+
+      it 'does not delete group' do
+        subject.execute
+
+        expect(application_setting.instance_administrators_group).to eq(group)
       end
     end
   end