Commit 32bada22 authored by Małgorzata Ksionek's avatar Małgorzata Ksionek

Change query in finder

WIP on changes

Refactor finder to use more efficient sql query
parent 33f5b834
......@@ -76,6 +76,7 @@ class Event < ApplicationRecord
# Scopes
scope :recent, -> { reorder(id: :desc) }
scope :code_push, -> { where(action: PUSHED) }
scope :merged, -> { where(action: MERGED) }
scope :with_associations, -> do
# We're using preload for "push_event_payload" as otherwise the association
......
......@@ -5,12 +5,11 @@ class Groups::Security::ComplianceDashboardsController < Groups::ApplicationCont
before_action :authorize_compliance_dashboard!
def show
preload_for_collection = [:target_project, :metrics, :approved_by_users, source_project: :route, target_project: :namespace]
preload_for_collection = [:approved_by_users, :metrics, source_project: :route, target_project: :namespace]
@merge_requests = MergeRequestsComplianceFinder.new(current_user, { group_id: @group.id })
@merge_requests = MergeRequestsComplianceFinder.new(current_user, { group_id: @group.id, preloads: preload_for_collection })
.execute
.preload(preload_for_collection) # rubocop: disable CodeReuse/ActiveRecord
.page(params[:page])
@merge_requests = Kaminari.paginate_array(@merge_requests).page(params[:page])
end
private
......
......@@ -8,17 +8,22 @@
# current_user - which user use
# params:
# group_id: integer
# preloads: array of associations to preload
#
class MergeRequestsComplianceFinder < MergeRequestsFinder
def execute
sql = super
.select('DISTINCT ON (merge_requests.target_project_id) merge_requests.*, merge_request_metrics.merged_at')
# rubocop: disable CodeReuse/ActiveRecord
lateral = Event
.select(:created_at, :target_id)
.where('projects.id = project_id')
.merged
.recent
.limit(1)
.to_sql
# rubocop: disable CodeReuse/ActiveRecord
MergeRequest
.from([Arel.sql("(#{sql}) AS #{MergeRequest.table_name}")])
.order('merged_at DESC')
sql = find_group_projects.as('projects').to_sql
records = Project.select('projects.id, events.target_id as merge_request_id').from([Arel.sql("#{sql} JOIN LATERAL (#{lateral}) #{Event.table_name} ON true")]).order('events.created_at DESC')
select_sorted_mrs(records)
# rubocop: enable CodeReuse/ActiveRecord
end
......@@ -26,12 +31,20 @@ class MergeRequestsComplianceFinder < MergeRequestsFinder
def params
finder_options = {
scope: :all,
state: :merged,
sort: :by_merged_at,
include_subgroups: true,
attempt_group_search_optimizations: true
}
super.merge(finder_options)
end
def select_sorted_mrs(records)
hash = {}
records.each { |row| hash[row['merge_request_id']] = nil }
mrs = MergeRequest.where(id: hash.keys).preload(params[:preloads]) # rubocop: disable CodeReuse/ActiveRecord
mrs.each { |mr| hash[mr.id] = mr }
hash.compact!
hash.values # sorted MRs
end
end
......@@ -73,6 +73,7 @@ module EE
def with_api_entity_associations
super.preload(:blocking_merge_requests)
end
<<<<<<< HEAD
def sort_by_attribute(method, *args)
if method.to_s == 'review_time_desc'
......@@ -91,6 +92,8 @@ module EE
grouping_columns << ::MergeRequest::Metrics.arel_table[:first_comment_at] if sort.to_s == 'review_time_desc'
grouping_columns
end
=======
>>>>>>> Change query in finder
end
override :mergeable?
......
%li{ id: dom_id(merge_request), class: mr_css_classes(merge_request), data: { id: merge_request.id } }
%li{ id: dom_id(merge_request), data: { id: merge_request.id } }
.issuable-info-container
.issuable-main-info
.merge-request-title.title
......
......@@ -3,7 +3,7 @@
%ul.content-list.mr-list.issuable-list
= render partial: 'merge_request', collection: @merge_requests
= paginate_without_count @merge_requests
= paginate @merge_requests
- else
= render 'empty_state'
......@@ -41,7 +41,7 @@ describe Groups::Security::ComplianceDashboardsController do
let(:mr_2) { create(:merge_request, source_project: project, source_branch: 'A', state: :merged) }
before do
mr_1.metrics.update!(merged_at: 20.minutes.ago)
create(:event, :merged, project: project, target: mr_1, author: user)
end
it 'renders merge request' do
......
......@@ -17,10 +17,10 @@ describe MergeRequestsComplianceFinder do
let(:mr_4) { create(:merge_request, source_project: project_2, source_branch: 'A', state: :merged) }
before do
mr_1.metrics.update!(merged_at: 20.minutes.ago)
mr_2.metrics.update!(merged_at: 40.minutes.ago)
mr_3.metrics.update!(merged_at: 30.minutes.ago)
mr_4.metrics.update!(merged_at: 50.minutes.ago)
create(:event, :merged, project: project_2, target: mr_4, author: current_user, created_at: 50.minutes.ago, id: 1)
create(:event, :merged, project: project_2, target: mr_2, author: current_user, created_at: 40.minutes.ago, id: 2)
create(:event, :merged, project: project, target: mr_3, author: current_user, created_at: 30.minutes.ago, id: 3)
create(:event, :merged, project: project, target: mr_1, author: current_user, created_at: 20.minutes.ago, id: 4)
end
context 'when there are merge requests from projects in group' do
......@@ -40,8 +40,8 @@ describe MergeRequestsComplianceFinder do
let(:mr_6) { create(:merge_request, source_project: sub_project, state: :merged) }
before do
mr_5.metrics.update!(merged_at: 10.minutes.ago)
mr_6.metrics.update!(merged_at: 30.minutes.ago)
create(:event, :merged, project: sub_project, target: mr_6, author: current_user, created_at: 30.minutes.ago, id: 6)
create(:event, :merged, project: sub_project, target: mr_5, author: current_user, created_at: 10.minutes.ago, id: 7)
end
it 'shows only most recent Merge Request from each project' do
......
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