Commit ee1f9b7f authored by lauraMon's avatar lauraMon

Adds feature flag and more

* Adds ci_jobs_finder_refactor ff
* Updates specs to work with and without the ff
* Puts back old code for when FF is disabled
parent f19280fd
......@@ -25,7 +25,11 @@ module Ci
attr_reader :current_user, :pipeline, :project, :params, :type
def init_collection
pipeline_jobs || project_jobs || all_jobs
if Feature.enabled?(:ci_jobs_finder_refactor)
pipeline_jobs || project_jobs || all_jobs
else
project ? project_builds : all_jobs
end
end
def all_jobs
......@@ -34,6 +38,12 @@ 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)
......@@ -49,7 +59,9 @@ module Ci
end
def filter_by_scope(builds)
return filter_by_statuses!(params[:scope], builds) if params[:scope].is_a?(Array)
if Feature.enabled?(:ci_jobs_finder_refactor)
return filter_by_statuses!(params[:scope], builds) if params[:scope].is_a?(Array)
end
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: false
......@@ -127,9 +127,15 @@ module API
authorize!(:read_pipeline, user_project)
pipeline = user_project.all_pipelines.find(params[:pipeline_id])
builds = ::Ci::JobsFinder
.new(current_user: current_user, pipeline: pipeline, params: params)
.execute
if Feature.enabled?(:ci_jobs_finder_refactor)
builds = ::Ci::JobsFinder
.new(current_user: current_user, pipeline: pipeline, params: params)
.execute
else
builds = pipeline.builds
builds = filter_builds(builds, params[:scope])
end
builds = builds.with_preloads
......@@ -150,9 +156,14 @@ module API
pipeline = user_project.all_pipelines.find(params[:pipeline_id])
bridges = ::Ci::JobsFinder
.new(current_user: current_user, pipeline: pipeline, params: params, type: ::Ci::Bridge)
.execute
if Feature.enabled?(:ci_jobs_finder_refactor)
bridges = ::Ci::JobsFinder
.new(current_user: current_user, pipeline: pipeline, params: params, type: ::Ci::Bridge)
.execute
else
bridges = pipeline.bridges
bridges = filter_builds(bridges, params[:scope])
end
bridges = bridges.with_preloads
......@@ -233,6 +244,21 @@ module API
end
helpers do
if Feature.disabled?(:ci_jobs_finder_refactor)
# 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
end
def pipeline
strong_memoize(:pipeline) do
user_project.all_pipelines.find(params[:pipeline_id])
......
......@@ -78,6 +78,10 @@ module Gitlab
def self.child_of_child_pipeline_enabled?(project)
::Feature.enabled?(:ci_child_of_child_pipeline, project, default_enabled: false)
end
def self.ci_jobs_finder_refactor?
::Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: false)
end
end
end
end
......
......@@ -36,62 +36,135 @@ RSpec.describe Ci::JobsFinder, '#execute' do
end
end
context 'scope is present' do
let(:jobs) { [job_1, job_2, job_3] }
where(:scope, :index) do
[
['pending', 0],
['running', 1],
['finished', 2]
]
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
end
with_them do
let(:params) { { scope: scope } }
context 'scope is an array' do
let(:jobs) { [job_1, job_2, job_3] }
let(:params) {{ scope: ['running'] }}
it { expect(subject).to match_array([jobs[index]]) }
it 'filters by the job statuses in the scope' do
expect(subject).to match_array([job_2])
end
end
end
context 'scope is an array' do
let(:jobs) { [job_1, job_2, job_3] }
let(:params) {{ scope: ['running'] }}
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
it 'filters by the job statuses in the scope' do
expect(subject).to match_array([job_2])
with_them do
let(:params) { { scope: scope } }
it { expect(subject).to match_array([jobs[index]]) }
end
end
end
end
context 'a project is present' do
subject { described_class.new(current_user: user, project: project, params: params).execute }
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)
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
it 'returns jobs for the specified project' do
expect(subject).to match_array([job_3])
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
end
context 'without user' do
let(:user) { nil }
it 'returns no jobs' do
expect(subject).to be_empty
end
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 no access to project builds' do
before do
project.add_guest(user)
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
it 'returns no jobs' do
expect(subject).to be_empty
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
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
it 'returns no jobs' do
expect(subject).to be_empty
end
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