Merge branch '344013_remove_projects_pending_deletion_from_ui' into 'master'

Remove projects that are marked for deletion from UI

See merge request gitlab-org/gitlab!75742

app/controllers/dashboard/projects_controller.rb

@@ -35,7 +35,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
 
   # rubocop: disable CodeReuse/ActiveRecord
   def starred
-    @projects = load_projects(params.merge(starred: true))
+    @projects = load_projects(params.merge(starred: true, not_aimed_for_deletion: true))
       .includes(:forked_from_project, :topics)
 
     @groups = []
@@ -54,7 +54,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
   private
 
   def projects
-    @projects ||= load_projects(params.merge(non_public: true))
+    @projects ||= load_projects(params.merge(non_public: true, not_aimed_for_deletion: true))
   end
 
   def render_projects
@@ -65,8 +65,8 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
   end
 
   def load_projects(finder_params)
-    @total_user_projects_count = ProjectsFinder.new(params: { non_public: true, without_deleted: true }, current_user: current_user).execute
-    @total_starred_projects_count = ProjectsFinder.new(params: { starred: true, without_deleted: true }, current_user: current_user).execute
+    @total_user_projects_count = ProjectsFinder.new(params: { non_public: true, without_deleted: true, not_aimed_for_deletion: true }, current_user: current_user).execute
+    @total_starred_projects_count = ProjectsFinder.new(params: { starred: true, without_deleted: true, not_aimed_for_deletion: true }, current_user: current_user).execute
 
     finder_params[:use_cte] = true if use_cte_for_finder?
     finder_params[:without_deleted] = true

app/controllers/explore/projects_controller.rb

@@ -92,7 +92,12 @@ class Explore::ProjectsController < Explore::ApplicationController
   def load_projects
     load_project_counts
 
-    projects = ProjectsFinder.new(current_user: current_user, params: params.merge(minimum_search_length: MIN_SEARCH_LENGTH)).execute
+    finder_params = {
+      minimum_search_length: MIN_SEARCH_LENGTH,
+      not_aimed_for_deletion: true
+    }
+
+    projects = ProjectsFinder.new(current_user: current_user, params: params.merge(finder_params)).execute
 
     projects = preload_associations(projects)
     projects = projects.page(params[:page]).without_count

app/controllers/users_controller.rb

@@ -186,7 +186,7 @@ class UsersController < ApplicationController
   end
 
   def starred_projects
-    StarredProjectsFinder.new(user, current_user: current_user).execute
+    StarredProjectsFinder.new(user, params: finder_params, current_user: current_user).execute
   end
 
   def contributions_calendar
@@ -252,6 +252,15 @@ class UsersController < ApplicationController
       end
     end
   end
+
+  def finder_params
+    {
+      # don't display projects pending deletion
+      without_deleted: true,
+      # don't display projects marked for deletion
+      not_aimed_for_deletion: true
+    }
+  end
 end
 
 UsersController.prepend_mod_with('UsersController')

app/finders/group_descendants_finder.rb

@@ -25,7 +25,7 @@ class GroupDescendantsFinder
   def initialize(current_user: nil, parent_group:, params: {})
     @current_user = current_user
     @parent_group = parent_group
-    @params = params.reverse_merge(non_archived: params[:archived].blank?)
+    @params = params.reverse_merge(non_archived: params[:archived].blank?, not_aimed_for_deletion: true)
   end
 
   def execute

app/finders/projects_finder.rb

@@ -27,6 +27,7 @@
 #     last_activity_before: datetime
 #     repository_storage: string
 #     without_deleted: boolean
+#     not_aimed_for_deletion: boolean
 #
 class ProjectsFinder < UnionFinder
   include CustomAttributesFilter
@@ -84,6 +85,7 @@ class ProjectsFinder < UnionFinder
     collection = by_archived(collection)
     collection = by_custom_attributes(collection)
     collection = by_deleted_status(collection)
+    collection = by_not_aimed_for_deletion(collection)
     collection = by_last_activity_after(collection)
     collection = by_last_activity_before(collection)
     by_repository_storage(collection)
@@ -203,6 +205,10 @@ class ProjectsFinder < UnionFinder
     params[:without_deleted].present? ? items.without_deleted : items
   end
 
+  def by_not_aimed_for_deletion(items)
+    params[:not_aimed_for_deletion].present? ? items.not_aimed_for_deletion : items
+  end
+
   def by_last_activity_after(items)
     if params[:last_activity_after].present?
       items.where("last_activity_at > ?", params[:last_activity_after]) # rubocop: disable CodeReuse/ActiveRecord

app/models/project.rb

@@ -511,6 +511,7 @@ class Project < ApplicationRecord
   # Scopes
   scope :pending_delete, -> { where(pending_delete: true) }
   scope :without_deleted, -> { where(pending_delete: false) }
+  scope :not_aimed_for_deletion, -> { where(marked_for_deletion_at: nil).without_deleted }
 
   scope :with_storage_feature, ->(feature) do
     where(arel_table[:storage_version].gteq(HASHED_STORAGE_FEATURES[feature]))

app/models/user.rb

@@ -1593,7 +1593,7 @@ class User < ApplicationRecord
       .distinct
       .reorder(nil)
 
-    Project.where(id: events)
+    Project.where(id: events).not_aimed_for_deletion
   end
 
   def can_be_removed?

ee/app/models/ee/project.rb

@@ -158,7 +158,6 @@ module EE
       scope :with_slack_integration, -> { joins(:slack_integration) }
       scope :with_slack_slash_commands_integration, -> { joins(:slack_slash_commands_integration) }
       scope :aimed_for_deletion, -> (date) { where('marked_for_deletion_at <= ?', date).without_deleted }
-      scope :not_aimed_for_deletion, -> { where(marked_for_deletion_at: nil) }
       scope :with_repos_templates, -> { where(namespace_id: ::Gitlab::CurrentSettings.current_application_settings.custom_project_templates_group_id) }
       scope :with_groups_level_repos_templates, -> { joins("INNER JOIN namespaces ON projects.namespace_id = namespaces.custom_project_templates_group_id") }
       scope :with_designs, -> { where(id: ::DesignManagement::Design.select(:project_id).distinct) }

ee/spec/finders/ee/projects_finder_spec.rb

@@ -52,11 +52,19 @@ RSpec.describe ProjectsFinder do
       context 'filter by aimed for deletion' do
         let_it_be(:params) { { aimed_for_deletion: true } }
         let_it_be(:aimed_for_deletion_project) { create(:project, :public, marked_for_deletion_at: 2.days.ago, pending_delete: false) }
-        let_it_be(:deleted_project) { create(:project, :public, marked_for_deletion_at: 1.month.ago, pending_delete: true) }
+        let_it_be(:pending_deletion_project) { create(:project, :public, marked_for_deletion_at: 1.month.ago, pending_delete: true) }
 
         it { is_expected.to contain_exactly(aimed_for_deletion_project) }
       end
 
+      context 'filter by not aimed for deletion' do
+        let_it_be(:params) { { not_aimed_for_deletion: true } }
+        let_it_be(:aimed_for_deletion_project) { create(:project, :public, marked_for_deletion_at: 2.days.ago, pending_delete: false) }
+        let_it_be(:pending_deletion_project) { create(:project, :public, marked_for_deletion_at: 1.month.ago, pending_delete: true) }
+
+        it { is_expected.to contain_exactly(ultimate_project, ultimate_project2, premium_project, no_plan_project) }
+      end
+
       private
 
       def create_project(plan)

spec/controllers/dashboard/projects_controller_spec.rb

@@ -97,14 +97,18 @@ RSpec.describe Dashboard::ProjectsController, :aggregate_failures do
       subject { get :starred, format: :json }
 
       let(:projects) { create_list(:project, 2, creator: user) }
+      let(:aimed_for_deletion_project) { create_list(:project, 2, :archived, creator: user, marked_for_deletion_at: 3.days.ago) }
 
       before do
-        allow(Kaminari.config).to receive(:default_per_page).and_return(1)
-
         projects.each do |project|
           project.add_developer(user)
           create(:users_star_project, project_id: project.id, user_id: user.id)
         end
+
+        aimed_for_deletion_project.each do |project|
+          project.add_developer(user)
+          create(:users_star_project, project_id: project.id, user_id: user.id)
+        end
       end
 
       it 'returns success' do
@@ -113,12 +117,24 @@ RSpec.describe Dashboard::ProjectsController, :aggregate_failures do
         expect(response).to have_gitlab_http_status(:ok)
       end
 
+      context "pagination" do
+        before do
+          allow(Kaminari.config).to receive(:default_per_page).and_return(1)
+        end
+
         it 'paginates the records' do
           subject
 
           expect(assigns(:projects).count).to eq(1)
         end
       end
+
+      it 'does not include projects aimed for deletion' do
+        subject
+
+        expect(assigns(:projects).count).to eq(2)
+      end
+    end
   end
 
   context 'atom requests' do

spec/controllers/explore/projects_controller_spec.rb

@@ -73,6 +73,24 @@ RSpec.describe Explore::ProjectsController do
           expect(assigns(:projects)).to eq [project1, project2]
         end
       end
+
+      context 'projects aimed for deletion' do
+        let(:project1) { create(:project, :public, updated_at: 3.days.ago) }
+        let(:project2) { create(:project, :public, updated_at: 1.day.ago) }
+        let(:aimed_for_deletion_project) { create(:project, :public, :archived, updated_at: 2.days.ago, marked_for_deletion_at: 2.days.ago) }
+
+        before do
+          create(:trending_project, project: project1)
+          create(:trending_project, project: project2)
+          create(:trending_project, project: aimed_for_deletion_project)
+        end
+
+        it 'does not list projects aimed for deletion' do
+          get :trending
+
+          expect(assigns(:projects)).to eq [project2, project1]
+        end
+      end
     end
 
     describe 'GET #topic' do

spec/finders/group_descendants_finder_spec.rb

@@ -68,6 +68,12 @@ RSpec.describe GroupDescendantsFinder do
       expect(finder.execute).to be_empty
     end
 
+    it 'does not include projects aimed for deletion' do
+      _project_aimed_for_deletion = create(:project, :archived, marked_for_deletion_at: 2.days.ago, pending_delete: false)
+
+      expect(finder.execute).to be_empty
+    end
+
     context 'with a filter' do
       let(:params) { { filter: 'test' } }
 

spec/models/user_spec.rb

@@ -3602,13 +3602,16 @@ RSpec.describe User do
     let!(:project1) { create(:project) }
     let!(:project2) { fork_project(project3) }
     let!(:project3) { create(:project) }
+    let!(:project_aimed_for_deletion) { create(:project, marked_for_deletion_at: 2.days.ago, pending_delete: false) }
     let!(:merge_request) { create(:merge_request, source_project: project2, target_project: project3, author: subject) }
     let!(:push_event) { create(:push_event, project: project1, author: subject) }
     let!(:merge_event) { create(:event, :created, project: project3, target: merge_request, author: subject) }
+    let!(:merge_event_2) { create(:event, :created, project: project_aimed_for_deletion, target: merge_request, author: subject) }
 
     before do
       project1.add_maintainer(subject)
       project2.add_maintainer(subject)
+      project_aimed_for_deletion.add_maintainer(subject)
     end
 
     it 'includes IDs for projects the user has pushed to' do
@@ -3622,6 +3625,10 @@ RSpec.describe User do
     it "doesn't include IDs for unrelated projects" do
       expect(subject.contributed_projects).not_to include(project2)
     end
+
+    it "doesn't include projects aimed for deletion" do
+      expect(subject.contributed_projects).not_to include(project_aimed_for_deletion)
+    end
   end
 
   describe '#fork_of' do

spec/requests/users_controller_spec.rb

@@ -506,6 +506,7 @@ RSpec.describe UsersController do
 
   describe 'GET #contributed' do
     let(:project) { create(:project, :public) }
+    let(:aimed_for_deletion_project) { create(:project, :public, :archived, marked_for_deletion_at: 3.days.ago) }
 
     subject do
       get user_contributed_projects_url author.username, format: format
@@ -516,7 +517,10 @@ RSpec.describe UsersController do
 
       project.add_developer(public_user)
       project.add_developer(private_user)
+      aimed_for_deletion_project.add_developer(public_user)
+      aimed_for_deletion_project.add_developer(private_user)
       create(:push_event, project: project, author: author)
+      create(:push_event, project: aimed_for_deletion_project, author: author)
 
       subject
     end
@@ -526,6 +530,11 @@ RSpec.describe UsersController do
         expect(response).to have_gitlab_http_status(:ok)
         expect(response.body).not_to be_empty
       end
+
+      it 'does not list projects aimed for deletion' do
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(assigns(:contributed_projects)).to eq([project])
+      end
     end
 
     %i(html json).each do |format|
@@ -557,6 +566,7 @@ RSpec.describe UsersController do
 
   describe 'GET #starred' do
     let(:project) { create(:project, :public) }
+    let(:aimed_for_deletion_project) { create(:project, :public, :archived, marked_for_deletion_at: 3.days.ago) }
 
     subject do
       get user_starred_projects_url author.username, format: format
@@ -574,6 +584,11 @@ RSpec.describe UsersController do
         expect(response).to have_gitlab_http_status(:ok)
         expect(response.body).not_to be_empty
       end
+
+      it 'does not list projects aimed for deletion' do
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(assigns(:starred_projects)).to eq([project])
+      end
     end
 
     %i(html json).each do |format|