Commit 7e957445 authored by lauraMon's avatar lauraMon

Cleans up ci_jobs_finder_refactor ff code

* Removes conditional ff specs from jobs finder spec
* Removes conditional ff specs from request specs for pipelines
* Removes feature flag code from JobsFinder
* Removes feature flag code from pipelines lib
* Removes ff yaml file
parent 284ad184
......@@ -25,11 +25,7 @@ module Ci
attr_reader :current_user, :pipeline, :project, :params, :type
def init_collection
if Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: true)
pipeline_jobs || project_jobs || all_jobs
else
project ? project_builds : all_jobs
end
pipeline_jobs || project_jobs || all_jobs
end
def all_jobs
......@@ -38,12 +34,6 @@ module Ci
type.all
end
def project_builds
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :read_build, project)
project.builds.relevant
end
def project_jobs
return unless project
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :read_build, project)
......@@ -59,9 +49,7 @@ module Ci
end
def filter_by_scope(builds)
if Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: true)
return filter_by_statuses!(params[:scope], builds) if params[:scope].is_a?(Array)
end
return filter_by_statuses!(params[:scope], builds) if params[:scope].is_a?(Array)
case params[:scope]
when 'pending'
......
---
name: ci_jobs_finder_refactor
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36622
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/245183
group: group::continuous integration
type: development
default_enabled: true
......@@ -128,15 +128,9 @@ module API
pipeline = user_project.all_pipelines.find(params[:pipeline_id])
if Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: true)
builds = ::Ci::JobsFinder
.new(current_user: current_user, pipeline: pipeline, params: params)
.execute
else
authorize!(:read_build, pipeline)
builds = pipeline.builds
builds = filter_builds(builds, params[:scope])
end
builds = ::Ci::JobsFinder
.new(current_user: current_user, pipeline: pipeline, params: params)
.execute
builds = builds.with_preloads
......@@ -157,16 +151,9 @@ module API
pipeline = user_project.all_pipelines.find(params[:pipeline_id])
if Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: true)
bridges = ::Ci::JobsFinder
.new(current_user: current_user, pipeline: pipeline, params: params, type: ::Ci::Bridge)
.execute
else
authorize!(:read_pipeline, pipeline)
bridges = pipeline.bridges
bridges = filter_builds(bridges, params[:scope])
end
bridges = ::Ci::JobsFinder
.new(current_user: current_user, pipeline: pipeline, params: params, type: ::Ci::Bridge)
.execute
bridges = bridges.with_preloads
present paginate(bridges), with: Entities::Ci::Bridge
......@@ -246,21 +233,6 @@ module API
end
helpers do
# NOTE: This method should be removed once the ci_jobs_finder_refactor FF is
# removed. https://gitlab.com/gitlab-org/gitlab/-/issues/245183
# rubocop: disable CodeReuse/ActiveRecord
def filter_builds(builds, scope)
return builds if scope.nil? || scope.empty?
available_statuses = ::CommitStatus::AVAILABLE_STATUSES
unknown = scope - available_statuses
render_api_error!('Scope contains invalid value(s)', 400) unless unknown.empty?
builds.where(status: scope)
end
# rubocop: enable CodeReuse/ActiveRecord
def pipeline
strong_memoize(:pipeline) do
user_project.all_pipelines.find(params[:pipeline_id])
......
......@@ -36,135 +36,62 @@ RSpec.describe Ci::JobsFinder, '#execute' do
end
end
context 'with ci_jobs_finder_refactor ff enabled' do
before do
stub_feature_flags(ci_jobs_finder_refactor: true)
end
context 'scope is present' do
let(:jobs) { [job_1, job_2, job_3] }
where(:scope, :index) do
[
['pending', 0],
['running', 1],
['finished', 2]
]
end
with_them do
let(:params) { { scope: scope } }
it { expect(subject).to match_array([jobs[index]]) }
end
context 'scope is present' do
let(:jobs) { [job_1, job_2, job_3] }
where(:scope, :index) do
[
['pending', 0],
['running', 1],
['finished', 2]
]
end
context 'scope is an array' do
let(:jobs) { [job_1, job_2, job_3] }
let(:params) {{ scope: ['running'] }}
with_them do
let(:params) { { scope: scope } }
it 'filters by the job statuses in the scope' do
expect(subject).to match_array([job_2])
end
it { expect(subject).to match_array([jobs[index]]) }
end
end
context 'with ci_jobs_finder_refactor ff disabled' do
before do
stub_feature_flags(ci_jobs_finder_refactor: false)
end
context 'scope is present' do
let(:jobs) { [job_1, job_2, job_3] }
where(:scope, :index) do
[
['pending', 0],
['running', 1],
['finished', 2]
]
end
context 'scope is an array' do
let(:jobs) { [job_1, job_2, job_3] }
let(:params) {{ scope: ['running'] }}
with_them do
let(:params) { { scope: scope } }
it { expect(subject).to match_array([jobs[index]]) }
end
it 'filters by the job statuses in the scope' do
expect(subject).to match_array([job_2])
end
end
end
context 'with ci_jobs_finder_refactor ff enabled' do
before do
stub_feature_flags(ci_jobs_finder_refactor: true)
end
context 'a project is present' do
subject { described_class.new(current_user: user, project: project, params: params).execute }
context 'user has access to the project' do
before do
project.add_maintainer(user)
end
it 'returns jobs for the specified project' do
expect(subject).to match_array([job_3])
end
end
context 'a project is present' do
subject { described_class.new(current_user: user, project: project, params: params).execute }
context 'user has no access to project builds' do
before do
project.add_guest(user)
end
it 'returns no jobs' do
expect(subject).to be_empty
end
context 'user has access to the project' do
before do
project.add_maintainer(user)
end
context 'without user' do
let(:user) { nil }
it 'returns no jobs' do
expect(subject).to be_empty
end
it 'returns jobs for the specified project' do
expect(subject).to match_array([job_3])
end
end
end
context 'with ci_jobs_finder_refactor ff disabled' do
before do
stub_feature_flags(ci_jobs_finder_refactor: false)
end
context 'a project is present' do
subject { described_class.new(current_user: user, project: project, params: params).execute }
context 'user has access to the project' do
before do
project.add_maintainer(user)
end
it 'returns jobs for the specified project' do
expect(subject).to match_array([job_3])
end
context 'user has no access to project builds' do
before do
project.add_guest(user)
end
context 'user has no access to project builds' do
before do
project.add_guest(user)
end
it 'returns no jobs' do
expect(subject).to be_empty
end
it 'returns no jobs' do
expect(subject).to be_empty
end
end
context 'without user' do
let(:user) { nil }
context 'without user' do
let(:user) { nil }
it 'returns no jobs' do
expect(subject).to be_empty
end
it 'returns no jobs' do
expect(subject).to be_empty
end
end
end
......
This diff is collapsed.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment