Skip to content

Commit

Permalink
Merge pull request #16120 from eduardoj/refactoring/bs_request_find_f…
Browse files Browse the repository at this point in the history
…or_i

Drop `find_for` BsRequest scope
  • Loading branch information
danidoni committed May 13, 2024
2 parents a4cb8ab + 59f2e0e commit 3159e67
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/api/app/controllers/request_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def render_request_collection
params[:review_states] = params[:reviewstates].split(',') if params[:reviewstates]
params[:ids] = params[:ids].split(',').map(&:to_i) if params[:ids]

rel = BsRequest.find_for(params)
rel = BsRequest::FindFor::Query.new(params).all
rel = BsRequest.where(id: rel.select(:id)).preload([{ bs_request_actions: :bs_request_action_accept_info, reviews: { history_elements: :user } }])
rel = rel.limit(params[:limit].to_i) if params[:limit].to_i.positive?
rel = rel.offset(params[:offset].to_i) if params[:offset].to_i.positive?
Expand Down
8 changes: 4 additions & 4 deletions src/api/app/controllers/webui/project_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,10 @@ def find_maintenance_infos
@is_incident_project = @project.is_maintenance_incident?
return unless @is_incident_project

@open_release_requests = BsRequest.find_for(project: @project.name,
states: %w[new review],
types: ['maintenance_release'],
roles: ['source']).pluck(:number)
@open_release_requests = BsRequest::FindFor::Query.new(project: @project.name,
states: %w[new review],
types: ['maintenance_release'],
roles: ['source']).all.pluck(:number) # rubocop:disable Rails/RedundantActiveRecordAllMethod
end

def valid_target_name?(name)
Expand Down
9 changes: 3 additions & 6 deletions src/api/app/models/bs_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ class BsRequest < ApplicationRecord
scope :by_package_reviews, ->(package_ids) { where(reviews: { package: package_ids }) }
scope :by_group_reviews, ->(group_ids) { where(reviews: { group: group_ids }) }

# FIXME: Get rid of this find_for scope since Rails 6.1, named scope chain does no longer leak scope to class-level querying methods
# For details, see https://guides.rubyonrails.org/6_1_release_notes.html#active-record-notable-changes
scope :find_for, ->(params) { BsRequest::FindFor::Query.new(params).all }
scope :obsolete, -> { where(state: OBSOLETE_STATES) }
scope :with_target_project, lambda { |target_project|
includes(:bs_request_actions).where('bs_request_actions.target_project': target_project)
Expand Down Expand Up @@ -119,10 +116,10 @@ def self.list(opts)

# it's wiser to split the queries
if opts[:project] && roles.empty? && (states.empty? || states.include?('review'))
(BsRequest.find_for(opts.merge(roles: ['reviewer'])) +
BsRequest.find_for(opts.merge(roles: %w[target source]))).uniq
(BsRequest::FindFor::Query.new(opts.merge(roles: ['reviewer'])).all +
BsRequest::FindFor::Query.new(opts.merge(roles: %w[target source])).all).uniq
else
BsRequest.find_for(opts).uniq
BsRequest::FindFor::Query.new(opts).all.uniq
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,11 @@ def count_requests
private

def request_query
BsRequest.find_for(
@params.merge(project: @project.name, package: @package.name)
)
BsRequest::FindFor::Query.new(@params.merge(project: @project.name, package: @package.name)).all
end

def request_query_without_search
BsRequest.find_for(
@params.except(:search).merge(project: @project.name, package: @package.name)
)
BsRequest::FindFor::Query.new(@params.except(:search).merge(project: @project.name, package: @package.name)).all
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,11 @@ def count_requests
private

def request_query
BsRequest.find_for(
@params.merge(project: @project.name)
)
BsRequest::FindFor::Query.new(@params.merge(project: @project.name)).all
end

def request_query_without_search
BsRequest.find_for(
@params.except(:search).merge(project: @project.name)
)
BsRequest::FindFor::Query.new(@params.except(:search).merge(project: @project.name)).all
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions src/api/app/models/group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,21 +161,21 @@ def display_name
end

def involved_reviews(search = nil)
BsRequest.find_for(
BsRequest::FindFor::Query.new(
group: title,
roles: [:reviewer],
review_states: [:new],
states: [:review],
search: search
)
).all
end

def incoming_requests(search = nil)
BsRequest.find_for(group: title, states: [:new], roles: [:maintainer], search: search)
BsRequest::FindFor::Query.new(group: title, states: [:new], roles: [:maintainer], search: search).all
end

def requests(search = nil)
BsRequest.find_for(group: title, search: search)
BsRequest::FindFor::Query.new(group: title, search: search).all
end

def all_requests_count
Expand Down

0 comments on commit 3159e67

Please sign in to comment.