From 34f57b462bc14ad194cf890a4585d403bdbde55c Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Thu, 15 Jun 2017 13:06:49 +0100 Subject: Fix current feature related specs --- app/finders/todos_finder.rb | 5 ++++- app/models/notification_setting.rb | 2 +- app/models/project.rb | 2 +- app/services/ci/register_job_service.rb | 8 ++++---- app/services/groups/destroy_service.rb | 2 +- app/services/users/destroy_service.rb | 2 +- app/workers/stuck_ci_jobs_worker.rb | 2 +- .../30708-stop-using-deleted-at-to-filter-namespaces.yml | 4 ++++ lib/api/helpers/runner.rb | 3 ++- lib/ci/api/helpers.rb | 3 ++- spec/models/project_spec.rb | 9 --------- 11 files changed, 21 insertions(+), 21 deletions(-) create mode 100644 changelogs/unreleased/30708-stop-using-deleted-at-to-filter-namespaces.yml diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index c358f23f541..5a4c5767be5 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -78,11 +78,14 @@ class TodosFinder end def project + return nil if @project&.pending_delete? return @project if defined?(@project) if project? @project = Project.find(params[:project_id]) + @project = nil if @project.pending_delete? + unless Ability.allowed?(current_user, :read_project, @project) @project = nil end @@ -95,7 +98,7 @@ class TodosFinder def projects(items) item_project_ids = items.reorder(nil).select(:project_id) - ProjectsFinder.new(current_user: current_user, project_ids_relation: item_project_ids).execute + ProjectsFinder.new(current_user: current_user, project_ids_relation: item_project_ids).execute.without_deleted end def type? diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index b0df7aeb323..81844b1e2ca 100644 --- a/app/models/notification_setting.rb +++ b/app/models/notification_setting.rb @@ -19,7 +19,7 @@ class NotificationSetting < ActiveRecord::Base # pending delete). # scope :for_projects, -> do - includes(:project).references(:projects).where(source_type: 'Project').where.not(projects: { id: nil }) + includes(:project).references(:projects).where(source_type: 'Project').where.not(projects: { id: nil, pending_delete: true }) end EMAIL_EVENTS = [ diff --git a/app/models/project.rb b/app/models/project.rb index 2ba7f683d12..40a8e7f07c6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1456,7 +1456,7 @@ class Project < ActiveRecord::Base def pending_delete_twin return false unless path - Project.unscoped.where(pending_delete: true).find_by_full_path(path_with_namespace) + Project.with_deleted.find_by_full_path(path_with_namespace) end ## diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index af84d4c7427..27bdb6b9a2f 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -54,9 +54,9 @@ module Ci def builds_for_shared_runner new_builds. # don't run projects which have not enabled shared runners and builds - joins(:project).where(projects: { shared_runners_enabled: true }) - .joins('LEFT JOIN project_features ON ci_builds.project_id = project_features.project_id') - .where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0'). + joins(:project).where(projects: { shared_runners_enabled: true, pending_delete: false }). + joins('LEFT JOIN project_features ON ci_builds.project_id = project_features.project_id'). + where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0'). # Implement fair scheduling # this returns builds that are ordered by number of running builds @@ -66,7 +66,7 @@ module Ci end def builds_for_specific_runner - new_builds.where(project: runner.projects.with_builds_enabled).order('created_at ASC') + new_builds.where(project: runner.projects.without_deleted.with_builds_enabled).order('created_at ASC') end def running_builds_for_shared_runners diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index 497fdb09cdc..d40d280140a 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -10,7 +10,7 @@ module Groups def execute group.prepare_for_destroy - group.projects.with_deleted.each do |project| + group.projects.each do |project| # Execute the destruction of the models immediately to ensure atomic cleanup. # Skip repository removal because we remove directory with namespace # that contain all these repositories diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index 673afb8b5b9..9d7237c2fbb 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -35,7 +35,7 @@ module Users Groups::DestroyService.new(group, current_user).execute end - user.personal_projects.with_deleted.each do |project| + user.personal_projects.each do |project| # Skip repository removal because we remove directory with namespace # that contain all this repositories ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb index ae8c980c9e4..8b0cfcc8af8 100644 --- a/app/workers/stuck_ci_jobs_worker.rb +++ b/app/workers/stuck_ci_jobs_worker.rb @@ -45,7 +45,7 @@ class StuckCiJobsWorker def search(status, timeout) builds = Ci::Build.where(status: status).where('ci_builds.updated_at < ?', timeout.ago) - builds.joins(:project).includes(:tags, :runner, project: :namespace).find_each(batch_size: 50).each do |build| + builds.joins(:project).merge(Project.without_deleted).includes(:tags, :runner, project: :namespace).find_each(batch_size: 50).each do |build| yield(build) end end diff --git a/changelogs/unreleased/30708-stop-using-deleted-at-to-filter-namespaces.yml b/changelogs/unreleased/30708-stop-using-deleted-at-to-filter-namespaces.yml new file mode 100644 index 00000000000..83ce3fb4d0a --- /dev/null +++ b/changelogs/unreleased/30708-stop-using-deleted-at-to-filter-namespaces.yml @@ -0,0 +1,4 @@ +--- +title: Removes deleted_at and pending_delete occurrences in Project related queries +merge_request: 12091 +author: diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb index 1369b021ea4..f8645e364ce 100644 --- a/lib/api/helpers/runner.rb +++ b/lib/api/helpers/runner.rb @@ -46,7 +46,8 @@ module API yield if block_given? - forbidden!('Project has been deleted!') unless job.project + project = job.project + forbidden!('Project has been deleted!') if project.nil? || project.pending_delete? forbidden!('Job has been erased!') if job.erased? end diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb index 5109dc9670f..a40b6ab6c9f 100644 --- a/lib/ci/api/helpers.rb +++ b/lib/ci/api/helpers.rb @@ -28,7 +28,8 @@ module Ci yield if block_given? - forbidden!('Project has been deleted!') unless build.project + project = build.project + forbidden!('Project has been deleted!') if project.nil? || project.pending_delete? forbidden!('Build has been erased!') if build.erased? end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d7fcadb895e..1dbe901b004 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -284,15 +284,6 @@ describe Project, models: true do end end - describe 'default_scope' do - it 'excludes projects pending deletion from the results' do - project = create(:empty_project) - create(:empty_project, pending_delete: true) - - expect(Project.all).to eq [project] - end - end - describe 'project token' do it 'sets an random token if none provided' do project = FactoryGirl.create :empty_project, runners_token: '' -- cgit v1.2.3