diff --git a/app/controllers/facility_reviews_controller.rb b/app/controllers/facility_reviews_controller.rb index 3d7da449..676329d3 100644 --- a/app/controllers/facility_reviews_controller.rb +++ b/app/controllers/facility_reviews_controller.rb @@ -29,9 +29,6 @@ def create return create_failed unless FacilityReview.create!(relevant_reviews) - # NB: Under the current model, this needs to happen before we add the descriptions with update_space_facilities, - # as aggregating DESTROYS all space_facilities, including the descriptions (wtf) - @space.aggregate_facility_reviews(facilities: @affected_facilities) update_space_facilities(reviews_and_descriptions[:space_facilities_attributes].values) create_succeeded diff --git a/app/models/facility_review.rb b/app/models/facility_review.rb index e71fab49..4dd6c7b6 100644 --- a/app/models/facility_review.rb +++ b/app/models/facility_review.rb @@ -13,6 +13,10 @@ class FacilityReview < ApplicationRecord validates :space, uniqueness: { scope: [:user, :facility] } + after_commit do + space.aggregate_facility_reviews(facilities: [facility]) + end + def experience_icon ICON_FOR_EXPERIENCE[experience] end @@ -46,6 +50,7 @@ def experience_icon # # index_facility_reviews_on_facility_id (facility_id) # index_facility_reviews_on_space_id (space_id) +# index_facility_reviews_on_space_id_and_facility_id (space_id,facility_id) # index_facility_reviews_on_space_id_and_user_id_and_facility_id (space_id,user_id,facility_id) UNIQUE # index_facility_reviews_on_user_id (user_id) # diff --git a/app/models/space.rb b/app/models/space.rb index 2a598c94..7d5243e6 100644 --- a/app/models/space.rb +++ b/app/models/space.rb @@ -34,10 +34,14 @@ class Space < ApplicationRecord # rubocop:disable Metrics/ClassLength south_east_lat:, south_east_lng:) } + + # This scope cuts the db calls when aggregating space_facilities + has_many :facility_reviews_ordered_by_newest_first, -> { order(created_at: :desc) }, + class_name: "FacilityReview", dependent: :destroy, inverse_of: :space scope :with_aggregation_data, lambda { preload( :space_facilities, - :facility_reviews, + :facility_reviews_ordered_by_newest_first, space_types: [ :facilities ] diff --git a/app/models/space_types_facility.rb b/app/models/space_types_facility.rb index aab7af29..0361635f 100644 --- a/app/models/space_types_facility.rb +++ b/app/models/space_types_facility.rb @@ -4,13 +4,7 @@ class SpaceTypesFacility < ApplicationRecord belongs_to :space_type belongs_to :facility - after_create do - space_type.reload.spaces.each do |space| - space.reload.aggregate_facility_reviews(facilities: [facility]) - end - end - - after_destroy do + after_commit do space_type.reload.spaces.each do |space| space.reload.aggregate_facility_reviews(facilities: [facility]) end @@ -29,8 +23,9 @@ class SpaceTypesFacility < ApplicationRecord # # Indexes # -# index_space_types_facilities_on_facility_id (facility_id) -# index_space_types_facilities_on_space_type_id (space_type_id) +# index_space_types_facilities_on_facility_id (facility_id) +# index_space_types_facilities_on_space_type_id (space_type_id) +# index_space_types_facilities_on_space_type_id_and_facility_id (space_type_id,facility_id) UNIQUE # # Foreign Keys # diff --git a/app/models/space_types_relation.rb b/app/models/space_types_relation.rb index 3f1d109f..6ec1aa38 100644 --- a/app/models/space_types_relation.rb +++ b/app/models/space_types_relation.rb @@ -4,11 +4,7 @@ class SpaceTypesRelation < ApplicationRecord belongs_to :space_type belongs_to :space - after_save do - space.reload.aggregate_facility_reviews(facilities: space_type.facilities) - end - - after_destroy do + after_commit do space.reload.aggregate_facility_reviews(facilities: space_type.facilities) end end diff --git a/app/services/spaces/aggregate_facility_reviews_service.rb b/app/services/spaces/aggregate_facility_reviews_service.rb index 828250c1..7e8aa80e 100644 --- a/app/services/spaces/aggregate_facility_reviews_service.rb +++ b/app/services/spaces/aggregate_facility_reviews_service.rb @@ -11,6 +11,7 @@ def initialize(space:, facilities: []) @facilities = facilities.any? ? facilities : Facility.order(:created_at) @space_facilities = @space.space_facilities @space_types = @space.space_types + @facility_reviews = @space.facility_reviews_ordered_by_newest_first group_recent_facility_reviews_by_facility(count: 5) super() diff --git a/app/services/spaces/concerns/countable_reviews.rb b/app/services/spaces/concerns/countable_reviews.rb index 9d70671a..5711717d 100644 --- a/app/services/spaces/concerns/countable_reviews.rb +++ b/app/services/spaces/concerns/countable_reviews.rb @@ -8,10 +8,9 @@ def most_recent_facility_reviews_for(facility) end def group_recent_facility_reviews_by_facility(count: 5) - @grouped_facility_reviews = @space.facility_reviews - .order(created_at: :desc) - .group_by(&:facility_id) - .transform_values { |reviews| reviews.first(count) } + @grouped_facility_reviews = @facility_reviews + .group_by(&:facility_id) + .transform_values { |reviews| reviews.first(count) } end def facility_reviews_for(facility) diff --git a/db/migrate/20240302202011_add_indexes_for_aggregations.rb b/db/migrate/20240302202011_add_indexes_for_aggregations.rb new file mode 100644 index 00000000..cdfa58eb --- /dev/null +++ b/db/migrate/20240302202011_add_indexes_for_aggregations.rb @@ -0,0 +1,9 @@ +class AddIndexesForAggregations < ActiveRecord::Migration[7.1] + def change + # Add index for facility_reviews on space_id and facility_id + add_index :facility_reviews, %i[space_id facility_id] + + # Add index for space types facilities on space_type_id and facility_id + add_index :space_types_facilities, [:space_type_id, :facility_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index c39a2e14..e15f7022 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_02_19_111137) do +ActiveRecord::Schema[7.1].define(version: 2024_03_02_202011) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -84,6 +84,7 @@ t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["facility_id"], name: "index_facility_reviews_on_facility_id" + t.index ["space_id", "facility_id"], name: "index_facility_reviews_on_space_id_and_facility_id" t.index ["space_id", "user_id", "facility_id"], name: "index_facility_reviews_on_space_id_and_user_id_and_facility_id", unique: true t.index ["space_id"], name: "index_facility_reviews_on_space_id" t.index ["user_id"], name: "index_facility_reviews_on_user_id" @@ -163,6 +164,7 @@ t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["facility_id"], name: "index_space_types_facilities_on_facility_id" + t.index ["space_type_id", "facility_id"], name: "index_space_types_facilities_on_space_type_id_and_facility_id", unique: true t.index ["space_type_id"], name: "index_space_types_facilities_on_space_type_id" end diff --git a/spec/fabricators/facility_review_fabricator.rb b/spec/fabricators/facility_review_fabricator.rb index f3d38c11..f3e510ec 100644 --- a/spec/fabricators/facility_review_fabricator.rb +++ b/spec/fabricators/facility_review_fabricator.rb @@ -23,6 +23,7 @@ # # index_facility_reviews_on_facility_id (facility_id) # index_facility_reviews_on_space_id (space_id) +# index_facility_reviews_on_space_id_and_facility_id (space_id,facility_id) # index_facility_reviews_on_space_id_and_user_id_and_facility_id (space_id,user_id,facility_id) UNIQUE # index_facility_reviews_on_user_id (user_id) # diff --git a/spec/fabricators/space_types_facility_fabricator.rb b/spec/fabricators/space_types_facility_fabricator.rb index 78af5ef6..553f53b1 100644 --- a/spec/fabricators/space_types_facility_fabricator.rb +++ b/spec/fabricators/space_types_facility_fabricator.rb @@ -17,8 +17,9 @@ # # Indexes # -# index_space_types_facilities_on_facility_id (facility_id) -# index_space_types_facilities_on_space_type_id (space_type_id) +# index_space_types_facilities_on_facility_id (facility_id) +# index_space_types_facilities_on_space_type_id (space_type_id) +# index_space_types_facilities_on_space_type_id_and_facility_id (space_type_id,facility_id) UNIQUE # # Foreign Keys # diff --git a/spec/services/spaces/aggregate_facility_reviews_service_spec.rb b/spec/services/spaces/aggregate_facility_reviews_service_spec.rb index 72a7faec..6c8c525c 100644 --- a/spec/services/spaces/aggregate_facility_reviews_service_spec.rb +++ b/spec/services/spaces/aggregate_facility_reviews_service_spec.rb @@ -198,12 +198,12 @@ def experience(experience, other_facility = nil) end it "is performant for a changing a single facility, even if there are a large amount of other facilities" do - Array.new(10) { Fabricate(:facility, space_types: [space_type]) } + Array.new(20) { Fabricate(:facility, space_types: [space_type]) } expect { space.aggregate_facility_reviews(facilities: [facility]) }.to perform_under(20).ms end it "is performant for a large amount of facilities" do - Array.new(10) { Fabricate(:facility, space_types: [space_type]) } + Array.new(20) { Fabricate(:facility, space_types: [space_type]) } expect { space.aggregate_facility_reviews }.to perform_under(30).ms end end