From 2b535e7cdb58bda742474526f38ebff6137472bd Mon Sep 17 00:00:00 2001 From: "allison.browne" Date: Thu, 20 Aug 2020 17:27:31 -0400 Subject: [PATCH 1/3] Hide pending_delete projects from project index Add tests to project finder and used the without_deleted param on the project index page --- .../dashboard/projects_controller.rb | 5 +-- app/finders/projects_finder.rb | 1 + ...ing-delete-projects-from-project-index.yml | 5 +++ .../dashboard/projects_controller_spec.rb | 30 ++++++++++++++++- spec/finders/projects_finder_spec.rb | 33 +++++++++++++++++-- 5 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/ab-hide-pending-delete-projects-from-project-index.yml diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb index 91704f030cddae..f559e63f1dc1d5 100644 --- a/app/controllers/dashboard/projects_controller.rb +++ b/app/controllers/dashboard/projects_controller.rb @@ -63,10 +63,11 @@ def render_projects end def load_projects(finder_params) - @total_user_projects_count = ProjectsFinder.new(params: { non_public: true }, current_user: current_user).execute - @total_starred_projects_count = ProjectsFinder.new(params: { starred: true }, current_user: current_user).execute + @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 finder_params[:use_cte] = true if use_cte_for_finder? + finder_params[:without_deleted] = true # TODO FF projects = ProjectsFinder.new(params: finder_params, current_user: current_user).execute diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 7c7cd87a7c1935..471029c1ef90ab 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -24,6 +24,7 @@ # last_activity_after: datetime # last_activity_before: datetime # repository_storage: string +# without_deleted: boolean # class ProjectsFinder < UnionFinder include CustomAttributesFilter diff --git a/changelogs/unreleased/ab-hide-pending-delete-projects-from-project-index.yml b/changelogs/unreleased/ab-hide-pending-delete-projects-from-project-index.yml new file mode 100644 index 00000000000000..a593312a564ca3 --- /dev/null +++ b/changelogs/unreleased/ab-hide-pending-delete-projects-from-project-index.yml @@ -0,0 +1,5 @@ +--- +title: Hide projects that are pending delete from the project index +merge_request: 40035 +author: +type: changed diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index 1e1d9519f78276..73ef947a9784ec 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -15,6 +15,7 @@ context 'user logged in' do let_it_be(:project) { create(:project) } let_it_be(:project2) { create(:project) } + let(:projects) { [project, project2] } before_all do project.add_developer(user) @@ -41,7 +42,7 @@ get :index - expect(assigns(:projects)).to eq([project, project2]) + expect(assigns(:projects)).to eq(projects) end context 'project sorting' do @@ -66,6 +67,18 @@ it_behaves_like 'search and sort parameters', sort end end + + context 'with deleted project' do + let_it_be(:pending_deleted_project) { create(:project, creator: user, pending_delete: true) } + + it 'does not display deleted project', :aggregate_failures do + get :index + projects_result = assigns(:projects) + + expect(projects_result).not_to include(pending_deleted_project) + expect(projects_result).to include(*projects) + end + end end end @@ -165,6 +178,21 @@ "closed issue #{issue.to_reference}" ) end + + context 'with deleted project' do + let_it_be(:pending_deleted_project) { create(:project, creator: user, pending_delete: true) } + + before do + projects[1].add_developer(user) + end + + it 'does not display deleted project', :aggregate_failures do + get :index, format: :atom + + expect(response.body).not_to include(pending_deleted_project.full_name) + expect(response.body).to include(*projects.map(&:full_name)) + end + end end end end diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 29b6dc6138685e..ec62adfbedafe2 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -234,10 +234,39 @@ end describe 'filter by without_deleted' do - let(:params) { { without_deleted: true } } + let(:params) { { without_deleted: without_deleted } } let!(:pending_delete_project) { create(:project, :public, pending_delete: true) } - it { is_expected.to match_array([public_project, internal_project]) } + shared_examples 'returns all projects' do + it { expect(subject).to include(public_project, internal_project, pending_delete_project) } + end + + context 'with without_deleted present and true' do + let(:without_deleted) { true } + + it 'returns projects that are not pending_delete' do + expect(subject).not_to include(pending_delete_project) + expect(subject).to include(public_project, internal_project) + end + end + + context 'when without_deleted present and false' do + let(:without_deleted) { false } + + it_behaves_like 'returns all projects' + end + + context 'when without_deleted present and false' do + let(:without_deleted) { nil } + + it_behaves_like 'returns all projects' + end + + context 'with without_deleted key is not present' do + let(:params) { {} } + + it_behaves_like 'returns all projects' + end end describe 'filter by last_activity_after' do -- GitLab From 24314b289ee5b0388842ed1acccda9ebd27beb0c Mon Sep 17 00:00:00 2001 From: "allison.browne" Date: Fri, 21 Aug 2020 11:20:36 -0400 Subject: [PATCH 2/3] Changes based on code review --- .../dashboard/projects_controller.rb | 4 ++-- .../dashboard/projects_controller_spec.rb | 12 ++++++----- spec/finders/projects_finder_spec.rb | 20 +++++++++++-------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb index f559e63f1dc1d5..2bd6fd85381af2 100644 --- a/app/controllers/dashboard/projects_controller.rb +++ b/app/controllers/dashboard/projects_controller.rb @@ -67,7 +67,7 @@ def load_projects(finder_params) @total_starred_projects_count = ProjectsFinder.new(params: { starred: true, without_deleted: true }, current_user: current_user).execute finder_params[:use_cte] = true if use_cte_for_finder? - finder_params[:without_deleted] = true # TODO FF + finder_params[:without_deleted] = true projects = ProjectsFinder.new(params: finder_params, current_user: current_user).execute @@ -90,7 +90,7 @@ def use_cte_for_finder? def load_events projects = ProjectsFinder - .new(params: params.merge(non_public: true), current_user: current_user) + .new(params: params.merge(non_public: true, without_deleted: true), current_user: current_user) .execute @events = EventCollection diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index 73ef947a9784ec..707f08b4ee2c86 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -69,14 +69,16 @@ end context 'with deleted project' do - let_it_be(:pending_deleted_project) { create(:project, creator: user, pending_delete: true) } + let!(:pending_delete_project) do + project.tap { |p| p.update!(pending_delete: true) } + end it 'does not display deleted project', :aggregate_failures do get :index projects_result = assigns(:projects) - expect(projects_result).not_to include(pending_deleted_project) - expect(projects_result).to include(*projects) + expect(projects_result).not_to include(pending_delete_project) + expect(projects_result).to include(project2) end end end @@ -183,14 +185,14 @@ let_it_be(:pending_deleted_project) { create(:project, creator: user, pending_delete: true) } before do - projects[1].add_developer(user) + pending_deleted_project.add_developer(user) end it 'does not display deleted project', :aggregate_failures do get :index, format: :atom expect(response.body).not_to include(pending_deleted_project.full_name) - expect(response.body).to include(*projects.map(&:full_name)) + expect(response.body).to include(project.full_name) end end end diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index ec62adfbedafe2..fcc86c69524db3 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -235,34 +235,38 @@ describe 'filter by without_deleted' do let(:params) { { without_deleted: without_deleted } } - let!(:pending_delete_project) { create(:project, :public, pending_delete: true) } shared_examples 'returns all projects' do - it { expect(subject).to include(public_project, internal_project, pending_delete_project) } + it { expect(subject).to include(public_project, internal_project) } end - context 'with without_deleted present and true' do + before do + internal_project.update!(pending_delete: true) + internal_project.reload + end + + context 'with without_deleted is true' do let(:without_deleted) { true } it 'returns projects that are not pending_delete' do - expect(subject).not_to include(pending_delete_project) - expect(subject).to include(public_project, internal_project) + expect(subject).not_to include(internal_project) + expect(subject).to include(public_project) end end - context 'when without_deleted present and false' do + context 'when without_deleted is false' do let(:without_deleted) { false } it_behaves_like 'returns all projects' end - context 'when without_deleted present and false' do + context 'when without_deleted is nil' do let(:without_deleted) { nil } it_behaves_like 'returns all projects' end - context 'with without_deleted key is not present' do + context 'with without_deleted is not present' do let(:params) { {} } it_behaves_like 'returns all projects' -- GitLab From fb7317123b2f6d5be6379d4249bf2ac8c05fb9f4 Mon Sep 17 00:00:00 2001 From: "allison.browne" Date: Wed, 26 Aug 2020 15:51:58 -0400 Subject: [PATCH 3/3] Changes based on code review feedback --- .../dashboard/projects_controller_spec.rb | 10 +++++----- spec/finders/projects_finder_spec.rb | 17 +++++++---------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index 707f08b4ee2c86..2719b7c8a240ef 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Dashboard::ProjectsController do +RSpec.describe Dashboard::ProjectsController, :aggregate_failures do include ExternalAuthorizationServiceHelpers let_it_be(:user) { create(:user) } @@ -73,7 +73,7 @@ project.tap { |p| p.update!(pending_delete: true) } end - it 'does not display deleted project', :aggregate_failures do + it 'does not display deleted project' do get :index projects_result = assigns(:projects) @@ -168,7 +168,7 @@ project.add_developer(user) end - it 'renders all kinds of event without error', :aggregate_failures do + it 'renders all kinds of event without error' do get :index, format: :atom expect(assigns(:events)).to include(design_event, wiki_page_event, issue_event) @@ -182,13 +182,13 @@ end context 'with deleted project' do - let_it_be(:pending_deleted_project) { create(:project, creator: user, pending_delete: true) } + let(:pending_deleted_project) { projects.last.tap { |p| p.update!(pending_delete: true) } } before do pending_deleted_project.add_developer(user) end - it 'does not display deleted project', :aggregate_failures do + it 'does not display deleted project' do get :index, format: :atom expect(response.body).not_to include(pending_deleted_project.full_name) diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index fcc86c69524db3..8ae19757c25507 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -234,23 +234,20 @@ end describe 'filter by without_deleted' do + let_it_be(:pending_delete_project) { create(:project, :public, pending_delete: true) } + let(:params) { { without_deleted: without_deleted } } shared_examples 'returns all projects' do - it { expect(subject).to include(public_project, internal_project) } - end - - before do - internal_project.update!(pending_delete: true) - internal_project.reload + it { expect(subject).to include(public_project, internal_project, pending_delete_project) } end - context 'with without_deleted is true' do + context 'when without_deleted is true' do let(:without_deleted) { true } it 'returns projects that are not pending_delete' do - expect(subject).not_to include(internal_project) - expect(subject).to include(public_project) + expect(subject).not_to include(pending_delete_project) + expect(subject).to include(public_project, internal_project) end end @@ -266,7 +263,7 @@ it_behaves_like 'returns all projects' end - context 'with without_deleted is not present' do + context 'when without_deleted is not present' do let(:params) { {} } it_behaves_like 'returns all projects' -- GitLab