Rewrite BsRequest query methods as scopes

These methods where used to query requests that contain reviews that
match the query attributes.
All of this can be done withActiveRecord::QueryMethods and a few
scopes.

This also required a change of how we ordered requests in the staging
controller, since we couldn't use the Array#sort anymore. A nice
side-effect of this is that we now do the request sorting as part of
the SQL query.

Co-authored-by: Dany Marcoux <dmarcoux@suse.com>

src/api/app/controllers/webui/obs_factory/staging_projects_controller.rb

@@ -9,22 +9,20 @@ def index
       respond_to do |format|
         format.html do
           @staging_projects = ::ObsFactory::StagingProjectPresenter.sort(@distribution.staging_projects_all)
-          @backlog_requests = BsRequest.with_open_reviews_for(by_group: @distribution.staging_manager, target_project: @distribution.name)
-          @requests_state_new = BsRequest.in_state_new(by_group: @distribution.staging_manager, target_project: @distribution.name)
+          @backlog_requests = BsRequest.with_open_reviews_for(by_group: @distribution.staging_manager)
+                                       .with_target_project(@distribution.name)
+          @requests_state_new = @backlog_requests.rewhere(state: :new)
 
           staging_project = Project.find_by_name("#{@distribution.project}:Staging")
           @ignored_requests = staging_project.dashboard.try(:ignored_requests)
 
           if @ignored_requests
-            @backlog_requests_ignored = @backlog_requests.select { |req| @ignored_requests.key?(req.number) }
-            @backlog_requests = @backlog_requests.select { |req| !@ignored_requests.key?(req.number) }
-            @requests_state_new = @requests_state_new.select { |req| !@ignored_requests.key?(req.number) }
-            @backlog_requests_ignored.sort! { |x, y| x.first_target_package <=> y.first_target_package }
+            @backlog_requests_ignored = @backlog_requests.where(number: @ignored_requests.keys)
+            @backlog_requests = @backlog_requests.where.not(number: @ignored_requests.keys)
+            @requests_state_new = @requests_state_new.where.not(number: @ignored_requests.keys)
           else
-            @backlog_requests_ignored = []
+            @backlog_requests_ignored = BsRequest.none
           end
-          @backlog_requests.sort! { |x, y| x.first_target_package <=> y.first_target_package }
-          @requests_state_new.sort! { |x, y| x.first_target_package <=> y.first_target_package }
           # For the breadcrumbs
           @project = @distribution.project
         end

src/api/app/models/bs_request.rb

@@ -77,6 +77,13 @@ class SaveError < APIError
   scope :for_project, ->(params) { BsRequest::FindFor::Project.new(params).all }
   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)
+  }
+  scope :with_open_reviews_for, lambda { |review_attributes|
+    where(state: 'review', id: Review.where(review_attributes).where(state: 'new').pluck(:bs_request_id))
+      .includes(:reviews)
+  }
 
   before_save :assign_number
   has_many :bs_request_actions, -> { includes([:bs_request_action_accept_info]) }, dependent: :destroy
@@ -269,27 +276,6 @@ def self.sourcediff_has_shown_attribute?(sourcediff)
   end
   private_class_method :sourcediff_has_shown_attribute?
 
-  # Requests in 'review' state that have new reviews for the given project
-  #
-  # @param [Hash] props can contain :by_project, :by_group, :by_user, :by_package
-  #               or :target_project
-  # @return [Array]  Array of Request objects
-  def self.with_open_reviews_for(attributes)
-    with_reviews(attributes).where(state: 'new').rewhere('bs_requests.state': 'review').map(&:bs_request)
-  end
-
-  def self.in_state_new(attributes)
-    with_reviews(attributes).where('bs_requests.state': 'new').map(&:bs_request)
-  end
-
-  def self.with_reviews(attributes)
-    review_attributes = attributes.slice(:by_project, :by_group, :by_user, :by_package, :state)
-
-    reviews = Review.where(review_attributes).includes(bs_request: [:reviews, :bs_request_actions])
-    return reviews unless attributes[:target_project]
-    reviews.where('bs_request_actions.target_project': attributes[:target_project])
-  end
-
   # Currently only used by staging projects for the obs factories and
   # customized for that.
   def as_json(*)

src/api/app/models/obs_factory/distribution.rb

@@ -129,7 +129,7 @@ def openqa_jobs_for(version)
     # @param [String] group name of the group
     # @return [Array] list of Request objects
     def requests_with_reviews_for_group(group)
-      BsRequest.with_open_reviews_for(by_group: group, target_project: root_project_name)
+      BsRequest.with_open_reviews_for(by_group: group).with_target_project(root_project_name)
     end
 
     # Requests with some open review targeting the distribution, filtered by
@@ -138,7 +138,7 @@ def requests_with_reviews_for_group(group)
     # @param [String] user name of the user
     # @return [Array] list of Request objects
     def requests_with_reviews_for_user(user)
-      BsRequest.with_open_reviews_for(by_user: user, target_project: root_project_name)
+      BsRequest.with_open_reviews_for(by_user: user).with_target_project(root_project_name)
     end
 
     # Standard project

src/api/spec/models/bs_request_spec.rb

@@ -568,40 +568,6 @@
     end
   end
 
-  describe '::in_state_new' do
-    include_context 'a BsRequest with reviews'
-
-    context "when request state is not 'new'" do
-      subject { BsRequest.in_state_new(by_user: reviewer.login) }
-
-      it { is_expected.to be_empty }
-    end
-
-    context "when request state is 'new'" do
-      before do
-        bs_request.update(state: 'new')
-      end
-
-      it 'queries requests with reviews by user' do
-        expect(BsRequest.in_state_new(by_user: reviewer.login)).to contain_exactly(bs_request)
-      end
-
-      it 'queries requests with reviews by group' do
-        bs_request.reviews.create!(by_group: group.title)
-        expect(BsRequest.in_state_new(by_group: group.title)).to contain_exactly(bs_request)
-      end
-
-      it 'queries requests with reviews by package' do
-        bs_request.reviews.create!(by_package: target_package, by_project: target_project)
-        expect(BsRequest.in_state_new(by_package: target_package.name)).to contain_exactly(bs_request)
-      end
-
-      it 'queries requests with reviews by target project of bs request' do
-        expect(BsRequest.in_state_new(target_project: target_project.name)).to contain_exactly(bs_request)
-      end
-    end
-  end
-
   describe '::with_open_reviews_for' do
     include_context 'a BsRequest with reviews'
 
@@ -627,10 +593,6 @@
         bs_request.reviews.create!(by_package: target_package, by_project: target_project)
         expect(BsRequest.with_open_reviews_for(by_package: target_package.name)).to contain_exactly(bs_request)
       end
-
-      it 'queries requests with reviews by target project of bs request' do
-        expect(BsRequest.with_open_reviews_for(target_project: target_project.name)).to contain_exactly(bs_request)
-      end
     end
   end