Dramatically improve RfC query

While the code gets more complex, the new query will first filter all relevant RfCs and only use the `with_last_activity` scope for the current page.

Using `distinct` in the policy is not required (no duplicated RfCs), but causes issues for the ordering otherwise. Therefore, it is removed.

Fixes CODEOCEAN-J2
This commit is contained in:
Sebastian Serth
2023-02-21 00:42:29 +01:00
parent 896f1dcbee
commit d0b713ddcd
2 changed files with 32 additions and 11 deletions

View File

@ -17,15 +17,24 @@ class RequestForCommentsController < ApplicationController
def index def index
@search = policy_scope(RequestForComment) @search = policy_scope(RequestForComment)
.last_per_user(2) .last_per_user(2)
.with_last_activity
.ransack(params[:q])
@request_for_comments = @search.result
.joins(:exercise) .joins(:exercise)
.where(exercises: {unpublished: false}) .where(exercises: {unpublished: false})
.order(created_at: :desc) # Order for the LIMIT part of the query
.ransack(params[:q])
# This total is used later to calculate the total number of entries
request_for_comments = @search.result
# All conditions are included in the query, so get the number of requested records
.paginate(page: params[:page], per_page: per_page_param)
@request_for_comments = RequestForComment.where(id: request_for_comments)
.with_last_activity # expensive query, so we only do it for the current page
.includes(submission: %i[study_group exercise]) .includes(submission: %i[study_group exercise])
.includes(:file, :comments, :user) .includes(:file, :comments, :user)
.order(created_at: :desc) .order(created_at: :desc) # Order for the view
.paginate(page: params[:page], per_page: per_page_param) # We need to "paginate" again to enable the pagination links.
# This `paginate` call is not expensive, because the total number of entries is already known.
.paginate(page: params[:page], per_page: per_page_param, total_entries: @search.result.length)
authorize! authorize!
end end
@ -33,20 +42,32 @@ class RequestForCommentsController < ApplicationController
# GET /my_request_for_comments # GET /my_request_for_comments
def my_comment_requests def my_comment_requests
@search = policy_scope(RequestForComment) @search = policy_scope(RequestForComment)
.with_last_activity
.where(user: current_user) .where(user: current_user)
.order(created_at: :desc) # Order for the LIMIT part of the query
.ransack(params[:q]) .ransack(params[:q])
@request_for_comments = @search.result
# This total is used later to calculate the total number of entries
request_for_comments = @search.result
# All conditions are included in the query, so get the number of requested records
.paginate(page: params[:page], per_page: per_page_param)
@request_for_comments = RequestForComment.where(id: request_for_comments)
.with_last_activity
.includes(submission: %i[study_group exercise]) .includes(submission: %i[study_group exercise])
.includes(:file, :comments, :user) .includes(:file, :comments, :user)
.order(created_at: :desc) .order(created_at: :desc) # Order for the view
.paginate(page: params[:page], per_page: per_page_param) # We need to "paginate" again to enable the pagination links.
# This `paginate` call is not expensive, because the total number of entries is already known.
.paginate(page: params[:page], per_page: per_page_param, total_entries: @search.result.length)
authorize! authorize!
render 'index' render 'index'
end end
# GET /my_rfc_activity # GET /my_rfc_activity
def rfcs_with_my_comments def rfcs_with_my_comments
# As we order by `last_comment`, we need to include `with_last_activity` in the original query.
# Therefore, the optimization chosen above doesn't work here.
@search = policy_scope(RequestForComment) @search = policy_scope(RequestForComment)
.with_last_activity .with_last_activity
.joins(:comments) # we don't need to outer join here, because we know the user has commented on these .joins(:comments) # we don't need to outer join here, because we know the user has commented on these

View File

@ -63,14 +63,14 @@ class RequestForCommentPolicy < ApplicationPolicy
when 'all' when 'all'
@scope.all @scope.all
when 'consumer' when 'consumer'
rfcs_with_users = @scope.distinct rfcs_with_users = @scope
.joins('LEFT OUTER JOIN external_users ON request_for_comments.user_type = \'ExternalUser\' AND request_for_comments.user_id = external_users.id') .joins('LEFT OUTER JOIN external_users ON request_for_comments.user_type = \'ExternalUser\' AND request_for_comments.user_id = external_users.id')
.joins('LEFT OUTER JOIN internal_users ON request_for_comments.user_type = \'InternalUser\' AND request_for_comments.user_id = internal_users.id') .joins('LEFT OUTER JOIN internal_users ON request_for_comments.user_type = \'InternalUser\' AND request_for_comments.user_id = internal_users.id')
rfcs_with_users.where(external_users: {consumer_id: @user.consumer.id}) rfcs_with_users.where(external_users: {consumer_id: @user.consumer.id})
.or(rfcs_with_users.where(internal_users: {consumer_id: @user.consumer.id})) .or(rfcs_with_users.where(internal_users: {consumer_id: @user.consumer.id}))
when 'study_group' when 'study_group'
@scope.distinct @scope
.joins(:submission) .joins(:submission)
.where(submission: {study_group: @user.current_study_group_id}) .where(submission: {study_group: @user.current_study_group_id})
else else