Skip to content

Commit

Permalink
Improve performance of ratings in JSON
Browse files Browse the repository at this point in the history
  • Loading branch information
reid-rigo committed Feb 16, 2024
1 parent 50d2e6c commit 0dfd3b2
Show file tree
Hide file tree
Showing 11 changed files with 51 additions and 14 deletions.
2 changes: 1 addition & 1 deletion app/controllers/api/v1/bulk/queries_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def create
if Query.import queries_to_import

@case.reload
@queries = @case.queries.includes([ :ratings ])
@queries = @case.queries
@display_order = @queries.map(&:id)

respond_with @queries, @display_order
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/queries_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class QueriesController < Api::ApiController
before_action :check_query, only: [ :update, :destroy ]

def index
@queries = @case.queries.includes([ :ratings ])
@queries = @case.queries

@display_order = @queries.map(&:id)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,8 @@ def case_with_all_the_bells_whistles
if current_user
@case = current_user
.cases_involved_with
.where(id: params[:case_id])
.includes(:tries )
.preload([ queries: [ :ratings ], tries: [ :curator_variables, :search_endpoint ] ])
.order('tries.try_number DESC')
.first
.preload(:queries, tries: [ :curator_variables, :search_endpoint ])
.find_by(id: params[:case_id])
end
@case = Case.public_cases.find_by(id: params[:case_id]) if @case.nil?
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/application_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@

class ApplicationRecord < ActiveRecord::Base
primary_abstract_class

def connection
self.class.connection
end
end
9 changes: 9 additions & 0 deletions app/models/case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,15 @@ def public_id
Rails.application.message_verifier('magic').generate(id)
end

# Optimized method to retrieve all doc ratings a single time
# Returns [5424, [{"query_id"=>5424, "doc_id"=>"l_21426", "rating"=>1.0}], ...]
def doc_ratings_by_query
@doc_ratings_by_query ||= begin
result = connection.select_all(ratings.select(:query_id, :doc_id, :rating).to_sql)
result.group_by { |r| r['query_id'] }
end
end

private

def set_scorer
Expand Down
2 changes: 1 addition & 1 deletion app/models/rating.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#

class Rating < ApplicationRecord
belongs_to :query
belongs_to :query, touch: true
belongs_to :user, optional: true

# arguably we shouldn't need this, however today you can have a rating object that doesn't have a
Expand Down
2 changes: 1 addition & 1 deletion app/views/api/v1/export/cases/_query.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ json.notes query.notes
json.information_need query.information_need

json.ratings do
json.array! query.ratings, partial: 'rating', as: :rating
@case.doc_ratings_by_query[query.id]&.each { |rating| json.set! rating['doc_id'], rating['rating'] }
end
6 changes: 2 additions & 4 deletions app/views/api/v1/queries/_query.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ json.query_text query.query_text
json.options query.options
json.notes query.notes
json.information_need query.information_need

# pick the most recent update between a query and it's ratings to represent modified_at
json.modified_at [ query, query.ratings ].flatten.max_by(&:updated_at).updated_at
json.modified_at query.updated_at

json.ratings do
query.ratings.each { |rating| json.set! rating.doc_id, rating.rating }
@case.doc_ratings_by_query[query.id]&.each { |rating| json.set! rating['doc_id'], rating['rating'] }
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class UpdateQueryUpdatedAtWithRatings < ActiveRecord::Migration[7.1]
def change
execute <<-SQL
UPDATE queries SET updated_at = GREATEST(updated_at, (SELECT MAX(updated_at) FROM ratings WHERE query_id = queries.id))
SQL
end
end
2 changes: 1 addition & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions test/models/case_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -290,4 +290,26 @@ class CaseTest < ActiveSupport::TestCase
assert_equal 2, the_case.queries_count
end
end

describe 'doc_ratings_by_query' do
let(:the_case) { cases(:random_case) }

it 'returns an empty hash for a new case' do
assert_equal Case.new.doc_ratings_by_query, {}
end

it 'returns an empty hash when there are no queries' do
the_case.queries.destroy_all
assert_equal the_case.doc_ratings_by_query, {}
end

it 'returns a hash of doc ratings by query id' do
assert_equal the_case.doc_ratings_by_query, {
488_291_725 => [
{ 'query_id' => 488_291_725, 'doc_id' => '234', 'rating' => nil },
{ 'query_id' => 488_291_725, 'doc_id' => '123', 'rating' => 5.0 }
],
}
end
end
end

0 comments on commit 0dfd3b2

Please sign in to comment.