Merge branch 'project-soft-delete-setting' into 'master'

Introduce Group level setting for Delayed Project removal

See merge request gitlab-org/gitlab!35689

app/views/groups/settings/_permissions.html.haml

@@ -30,6 +30,7 @@
           %span.d-block= s_('GroupSettings|Disable group mentions')
           %span.text-muted= s_('GroupSettings|This setting will prevent group members from being notified if the group is mentioned.')
 
+    = render_if_exists 'groups/settings/delayed_project_removal', f: f, group: @group
     = render_if_exists 'groups/settings/ip_restriction', f: f, group: @group
     = render_if_exists 'groups/settings/allowed_email_domain', f: f, group: @group
     = render 'groups/settings/lfs', f: f

changelogs/unreleased/project-soft-delete-setting.yml

+---
+title: Introduces Group Level Delayed Project Removal Setting
+merge_request: 35689
+author:
+type: added

db/migrate/20200701190523_add_delayed_project_removal_to_namespaces.rb

+# frozen_string_literal: true
+
+class AddDelayedProjectRemovalToNamespaces < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_column :namespaces, :delayed_project_removal, :boolean, default: false, null: false
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :namespaces, :delayed_project_removal
+    end
+  end
+end

db/structure.sql

@@ -13136,7 +13136,8 @@ CREATE TABLE public.namespaces (
     push_rule_id bigint,
     shared_runners_enabled boolean DEFAULT true NOT NULL,
     allow_descendants_override_disabled_shared_runners boolean DEFAULT false NOT NULL,
-    traversal_ids integer[] DEFAULT '{}'::integer[] NOT NULL
+    traversal_ids integer[] DEFAULT '{}'::integer[] NOT NULL,
+    delayed_project_removal boolean DEFAULT false NOT NULL
 );
 
 CREATE SEQUENCE public.namespaces_id_seq
@@ -23725,6 +23726,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200701070435
 20200701091253
 20200701093859
+20200701190523
 20200701205710
 20200702123805
 20200702201039

ee/app/controllers/ee/groups_controller.rb

@@ -74,6 +74,7 @@ module EE
         params_ee << :allowed_email_domains_list if current_group&.feature_available?(:group_allowed_email_domains)
         params_ee << :max_pages_size if can?(current_user, :update_max_pages_size)
         params_ee << :max_personal_access_token_lifetime if current_group&.personal_access_token_expiration_policy_available?
+        params_ee << :delayed_project_removal if current_group&.configure_project_deletion_mode_available?
       end
     end
 

ee/app/helpers/ee/groups_helper.rb

@@ -117,6 +117,11 @@ module EE
       end
     end
 
+    def show_delayed_project_removal_setting?(group)
+      group.feature_available?(:adjourned_deletion_for_projects_and_groups) &&
+        ::Feature.enabled?(:configure_project_deletion_mode, group)
+    end
+
     private
 
     def get_group_sidebar_links

ee/app/models/ee/group.rb

@@ -390,6 +390,11 @@ module EE
       owners.pluck(:email)
     end
 
+    def configure_project_deletion_mode_available?
+      feature_available?(:adjourned_deletion_for_projects_and_groups) &&
+        ::Feature.enabled?(:configure_project_deletion_mode, self)
+    end
+
     private
 
     def custom_project_templates_group_allowed

ee/app/models/ee/project.rb

@@ -616,7 +616,8 @@ module EE
 
     def adjourned_deletion?
       feature_available?(:adjourned_deletion_for_projects_and_groups) &&
-        ::Gitlab::CurrentSettings.deletion_adjourned_period > 0
+        ::Gitlab::CurrentSettings.deletion_adjourned_period.positive? &&
+        group_deletion_mode_configured?
     end
 
     def marked_for_deletion?
@@ -750,6 +751,13 @@ module EE
         end
       end
     end
+
+    # If the feature to configure project deletion mode is NOT enabled, we default to delayed deletion
+    def group_deletion_mode_configured?
+      return true unless ::Feature.enabled?(:configure_project_deletion_mode, self)
+
+      group && group.delayed_project_removal? # Return the group's setting for delayed deletion, false for user namespace projects
+    end
   end
 end
 

ee/app/services/ee/groups/create_service.rb

@@ -29,6 +29,7 @@ module EE
         unless current_user&.admin?
           params.delete(:shared_runners_minutes_limit)
           params.delete(:extra_shared_runners_minutes_limit)
+          params.delete(:delayed_project_removal)
         end
 
         super

ee/app/views/groups/settings/_delayed_project_removal.html.haml

+- return unless show_delayed_project_removal_setting?(group)
+
+.form-group.gl-mb-3
+  .form-check
+    = f.check_box :delayed_project_removal, checked: group.delayed_project_removal?, class: 'form-check-input'
+    = f.label :delayed_project_removal, class: 'form-check-label' do
+      %span.gl-display-block= s_('GroupSettings|Enable delayed project removal')
+      %span.gl-text-gray-600= s_('GroupSettings|Projects will be permanently deleted after a %{waiting_period}-day waiting period. This period can be %{customization_link} in instance settings').html_safe % { waiting_period: ::Gitlab::CurrentSettings.deletion_adjourned_period, customization_link: link_to('customized by an admin', general_admin_application_settings_path) }

ee/spec/controllers/ee/groups_controller_spec.rb

@@ -500,5 +500,43 @@ RSpec.describe GroupsController do
         end
       end
     end
+
+    context 'when delayed_project_removal param is specified' do
+      let_it_be(:params) { { delayed_project_removal: true } }
+      let_it_be(:user) { create(:user) }
+
+      subject do
+        put :update, params: { id: group.to_param, group: params }
+      end
+
+      before do
+        group.add_owner(user)
+        sign_in(user)
+        stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
+        stub_feature_flags(configure_project_deletion_mode: available)
+      end
+
+      context 'when feature is available' do
+        let(:available) { true }
+
+        it 'allows storing of setting' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:found)
+          expect(group.reload.delayed_project_removal).to eq(params[:delayed_project_removal])
+        end
+      end
+
+      context 'when feature is not available' do
+        let(:available) { false }
+
+        it 'does not allow storing of setting' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:found)
+          expect(group.reload.delayed_project_removal).not_to eq(params[:delayed_project_removal])
+        end
+      end
+    end
   end
 end

ee/spec/controllers/projects_controller_spec.rb

@@ -503,48 +503,92 @@ RSpec.describe ProjectsController do
 
   describe 'DELETE #destroy' do
     let(:owner) { create(:user) }
-    let(:project) { create(:project, namespace: owner.namespace)}
+    let(:group) { create(:group) }
+    let(:project) { create(:project, group: group)}
 
     before do
+      group.add_user(owner, Gitlab::Access::OWNER)
       controller.instance_variable_set(:@project, project)
       sign_in(owner)
     end
 
-    context 'feature is available' do
-      before do
-        stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
+    shared_examples 'deletes project right away' do
+      it do
+        delete :destroy, params: { namespace_id: project.namespace, id: project }
+
+        expect(project.marked_for_deletion?).to be_falsey
+        expect(response).to have_gitlab_http_status(:found)
+        expect(response).to redirect_to(dashboard_projects_path)
       end
+    end
 
-      it 'marks project for deletion' do
+    shared_examples 'marks project for deletion' do
+      it do
         delete :destroy, params: { namespace_id: project.namespace, id: project }
 
         expect(project.reload.marked_for_deletion?).to be_truthy
         expect(response).to have_gitlab_http_status(:found)
         expect(response).to redirect_to(project_path(project))
       end
+    end
 
-      it 'does not mark project for deletion because of error' do
-        message = 'Error'
+    context 'feature is available' do
+      before do
+        stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
+      end
 
-        expect(::Projects::MarkForDeletionService).to receive_message_chain(:new, :execute).and_return({ status: :error, message: message })
+      context 'when feature is enabled for group' do
+        before do
+          allow(group).to receive(:delayed_project_removal?).and_return(true)
+        end
 
-        delete :destroy, params: { namespace_id: project.namespace, id: project }
+        it_behaves_like 'marks project for deletion'
 
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(response).to render_template(:edit)
-        expect(flash[:alert]).to include(message)
-      end
+        it 'does not mark project for deletion because of error' do
+          message = 'Error'
 
-      context 'when instance setting is set to 0 days' do
-        it 'deletes project right away' do
-          allow(Gitlab::CurrentSettings).to receive(:deletion_adjourned_period).and_return(0)
+          expect(::Projects::MarkForDeletionService).to receive_message_chain(:new, :execute).and_return({ status: :error, message: message })
 
           delete :destroy, params: { namespace_id: project.namespace, id: project }
 
-          expect(project.marked_for_deletion?).to be_falsey
-          expect(response).to have_gitlab_http_status(:found)
-          expect(response).to redirect_to(dashboard_projects_path)
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to render_template(:edit)
+          expect(flash[:alert]).to include(message)
         end
+
+        context 'when instance setting is set to 0 days' do
+          it 'deletes project right away' do
+            stub_application_setting(deletion_adjourned_period: 0)
+
+            delete :destroy, params: { namespace_id: project.namespace, id: project }
+
+            expect(project.marked_for_deletion?).to be_falsey
+            expect(response).to have_gitlab_http_status(:found)
+            expect(response).to redirect_to(dashboard_projects_path)
+          end
+        end
+      end
+
+      context 'when feature is disabled for group' do
+        before do
+          allow(group).to receive(:delayed_project_removal).and_return(false)
+        end
+
+        it_behaves_like 'deletes project right away'
+      end
+
+      context 'for projects in user namespace' do
+        let(:project) { create(:project, namespace: owner.namespace)}
+
+        it_behaves_like 'deletes project right away'
+      end
+
+      context 'when configure_project_deletion_mode feature is disabled' do
+        before do
+          stub_feature_flags(configure_project_deletion_mode: false)
+        end
+
+        it_behaves_like 'marks project for deletion'
       end
     end
 
@@ -553,13 +597,7 @@ RSpec.describe ProjectsController do
         stub_licensed_features(adjourned_deletion_for_projects_and_groups: false)
       end
 
-      it 'deletes project right away' do
-        delete :destroy, params: { namespace_id: project.namespace, id: project }
-
-        expect(project.marked_for_deletion?).to be_falsey
-        expect(response).to have_gitlab_http_status(:found)
-        expect(response).to redirect_to(dashboard_projects_path)
-      end
+      it_behaves_like 'deletes project right away'
     end
   end
 

ee/spec/helpers/ee/groups_helper_spec.rb

@@ -309,4 +309,22 @@ RSpec.describe GroupsHelper do
       end
     end
   end
+
+  describe '#show_delayed_project_removal_setting?' do
+    before do
+      stub_licensed_features(adjourned_deletion_for_projects_and_groups: licensed?)
+      stub_feature_flags(configure_project_deletion_mode: flag_enabled?)
+    end
+
+    where(:licensed?, :flag_enabled?, :result) do
+      true  | true  | true
+      true  | false | false
+      false | true  | false
+      false | false | false
+    end
+
+    with_them do
+      it { expect(helper.show_delayed_project_removal_setting?(group)).to be result }
+    end
+  end
 end

ee/spec/models/group_spec.rb

@@ -1029,4 +1029,24 @@ RSpec.describe Group do
 
     it { is_expected.to match([user.email]) }
   end
+
+  describe '#configure_project_deletion_mode_available?' do
+    using RSpec::Parameterized::TableSyntax
+
+    before do
+      stub_licensed_features(adjourned_deletion_for_projects_and_groups: licensed?)
+      stub_feature_flags(configure_project_deletion_mode: flag_enabled?)
+    end
+
+    where(:licensed?, :flag_enabled?, :result) do
+      true  | true  | true
+      true  | false | false
+      false | true  | false
+      false | false | false
+    end
+
+    with_them do
+      it { expect(group.configure_project_deletion_mode_available?).to be result }
+    end
+  end
 end

ee/spec/models/project_spec.rb

@@ -2433,50 +2433,69 @@ RSpec.describe Project do
   end
 
   describe '#adjourned_deletion?' do
-    context 'when marking for deletion feature is available' do
-      let(:project) { create(:project) }
+    using RSpec::Parameterized::TableSyntax
+
+    subject { project.adjourned_deletion? }
+
+    where(:licensed?, :feature_enabled_on_group?, :adjourned_period, :result) do
+      true    | true  | 0 | false
+      true    | true  | 1 | true
+      true    | false | 0 | false
+      true    | false | 1 | false
+      false   | true  | 0 | false
+      false   | true  | 1 | false
+      false   | false | 0 | false
+      false   | false | 1 | false
+    end
+
+    with_them do
+      let_it_be(:group) { create(:group) }
+      let_it_be(:project) { create(:project, group: group) }
 
       before do
-        stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
+        stub_licensed_features(adjourned_deletion_for_projects_and_groups: licensed?)
+        stub_application_setting(deletion_adjourned_period: adjourned_period)
+        allow(group).to receive(:delayed_project_removal?).and_return(feature_enabled_on_group?)
       end
 
-      context 'when number of days is set to more than 0' do
-        it 'returns true' do
-          stub_application_setting(deletion_adjourned_period: 1)
+      it { is_expected.to be result }
+    end
 
-          expect(project.adjourned_deletion?).to eq(true)
-        end
+    context 'when configure_project_deletion_mode feature is disabled' do
+      before do
+        stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
+        stub_application_setting(deletion_adjourned_period: 7)
+        stub_feature_flags(configure_project_deletion_mode: false)
       end
 
-      context 'when number of days is set to 0' do
-        it 'returns false' do
-          stub_application_setting(deletion_adjourned_period: 0)
-
-          expect(project.adjourned_deletion?).to eq(false)
-        end
+      it 'adjourns deletion' do
+        is_expected.to be true
       end
     end
 
-    context 'when marking for deletion feature is not available' do
-      let(:project) { create(:project) }
+    context 'when project belongs to user namespace' do
+      let_it_be(:user) { create(:user) }
+      let_it_be(:user_project) { create(:project, namespace: user.namespace) }
 
       before do
-        stub_licensed_features(adjourned_deletion_for_projects_and_groups: false)
+        stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
+        stub_application_setting(deletion_adjourned_period: 7)
+        stub_feature_flags(configure_project_deletion_mode: feature_enabled?)
       end
 
-      context 'when number of days is set to more than 0' do
-        it 'returns false' do
-          stub_application_setting(deletion_adjourned_period: 1)
+      context 'configure_project_deletion_mode is enabled' do
+        let(:feature_enabled?) { true }
 
-          expect(project.adjourned_deletion?).to eq(false)
+        it 'deletes immediately' do
+          expect(user_project.adjourned_deletion?).to be nil
         end
       end
 
-      context 'when number of days is set to 0' do
-        it 'returns false' do
-          stub_application_setting(deletion_adjourned_period: 0)
+      context 'configure_project_deletion_mode is disabled' do
+        let(:feature_enabled?) { false }
 
-          expect(project.adjourned_deletion?).to eq(false)
+        it 'adjourns deletion' do
+          expect(user_project.adjourned_deletion?).to be true
         end
       end
     end

ee/spec/requests/api/projects_spec.rb

@@ -920,37 +920,79 @@ RSpec.describe API::Projects do
   end
 
   describe 'DELETE /projects/:id' do
-    context 'when feature is available' do
-      before do
-        stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
+    let(:group) { create(:group) }
+    let(:project) { create(:project, group: group)}
+
+    before do
+      group.add_user(user, Gitlab::Access::OWNER)
+    end
+
+    shared_examples 'deletes project immediately' do
+      it do
+        delete api("/projects/#{project.id}", user)
+
+        expect(response).to have_gitlab_http_status(:accepted)
+        expect(project.reload.pending_delete).to eq(true)
       end
+    end
 
-      it 'marks project for deletion' do
+    shared_examples 'marks project for deletion' do
+      it do
         delete api("/projects/#{project.id}", user)
 
         expect(response).to have_gitlab_http_status(:accepted)
         expect(project.reload.marked_for_deletion?).to be_truthy
       end
+    end
 
-      it 'returns error if project cannot be marked for deletion' do
-        message = 'Error'
-        expect(::Projects::MarkForDeletionService).to receive_message_chain(:new, :execute).and_return({ status: :error, message: message })
+    context 'when feature is available' do
+      before do
+        stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
+      end
 
-        delete api("/projects/#{project.id}", user)
+      context 'delayed project removal is enabled for group' do
+        let(:group) { create(:group, delayed_project_removal: true) }
 
-        expect(response).to have_gitlab_http_status(:bad_request)
-        expect(json_response["message"]).to eq(message)
-      end
+        it_behaves_like 'marks project for deletion'
+
+        it 'returns error if project cannot be marked for deletion' do
+          message = 'Error'
+          expect(::Projects::MarkForDeletionService).to receive_message_chain(:new, :execute).and_return({ status: :error, message: message })
 
-      context 'when instance setting is set to 0 days' do
-        it 'deletes project right away' do
-          allow(Gitlab::CurrentSettings).to receive(:deletion_adjourned_period).and_return(0)
           delete api("/projects/#{project.id}", user)
 
-          expect(response).to have_gitlab_http_status(:accepted)
-          expect(project.reload.pending_delete).to eq(true)
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response["message"]).to eq(message)
+        end
+
+        context 'when instance setting is set to 0 days' do
+          it 'deletes project right away' do
+            allow(Gitlab::CurrentSettings).to receive(:deletion_adjourned_period).and_return(0)
+            delete api("/projects/#{project.id}", user)
+
+            expect(response).to have_gitlab_http_status(:accepted)
+            expect(project.reload.pending_delete).to eq(true)
+          end
         end
       end
+
+      context 'delayed project removal is disabled for group' do
+        it_behaves_like 'deletes project immediately'
+      end
+
+      context 'for projects in user namespace' do
+        let(:project) { create(:project, namespace: user.namespace)}
+
+        it_behaves_like 'deletes project immediately'
+      end
+
+      context 'when configure_project_deletion_mode feature is disabled' do
+        before do
+          stub_feature_flags(configure_project_deletion_mode: false)
+        end
+
+        it_behaves_like 'marks project for deletion'
+      end
     end
 
     context 'when feature is not available' do
@@ -958,12 +1000,7 @@ RSpec.describe API::Projects do
         stub_licensed_features(adjourned_deletion_for_projects_and_groups: false)
       end
 
-      it 'deletes project' do
-        delete api("/projects/#{project.id}", user)
-
-        expect(response).to have_gitlab_http_status(:accepted)
-        expect(project.reload.pending_delete).to eq(true)
-      end
+      it_behaves_like 'deletes project immediately'
     end
   end
 

ee/spec/services/groups/create_service_spec.rb

@@ -61,7 +61,7 @@ RSpec.describe Groups::CreateService, '#execute' do
 
   context 'updating protected params' do
     let(:attrs) do
-      group_params.merge(shared_runners_minutes_limit: 1000, extra_shared_runners_minutes_limit: 100)
+      group_params.merge(shared_runners_minutes_limit: 1000, extra_shared_runners_minutes_limit: 100, delayed_project_removal: true)
     end
 
     context 'as an admin' do
@@ -72,6 +72,7 @@ RSpec.describe Groups::CreateService, '#execute' do
 
         expect(group.shared_runners_minutes_limit).to eq(1000)
         expect(group.extra_shared_runners_minutes_limit).to eq(100)
+        expect(group.delayed_project_removal).to be true
       end
     end
 
@@ -81,6 +82,7 @@ RSpec.describe Groups::CreateService, '#execute' do
 
         expect(group.shared_runners_minutes_limit).to be_nil
         expect(group.extra_shared_runners_minutes_limit).to be_nil
+        expect(group.delayed_project_removal).to be false
       end
     end
   end

locale/gitlab.pot

@@ -11739,6 +11739,9 @@ msgstr ""
 msgid "GroupSettings|Disable group mentions"
 msgstr ""
 
+msgid "GroupSettings|Enable delayed project removal"
+msgstr ""
+
 msgid "GroupSettings|Export group"
 msgstr ""
 
@@ -11766,6 +11769,9 @@ msgstr ""
 msgid "GroupSettings|Prevent sharing a project within %{group} with other groups"
 msgstr ""
 
+msgid "GroupSettings|Projects will be permanently deleted after a %{waiting_period}-day waiting period. This period can be %{customization_link} in instance settings"
+msgstr ""
+
 msgid "GroupSettings|Select a sub-group as the custom project template source for this group."
 msgstr ""