From c75d9884976b4c2fc9d74e00ebfeaed7544ec6d8 Mon Sep 17 00:00:00 2001 From: Emi Matchu Date: Tue, 20 Feb 2024 15:36:20 -0800 Subject: [PATCH] Migrate away from item translations in the Your Items feature Just replacing references to the `Item::Translation` model to the fields on `Item` itself! --- app/controllers/closet_hangers_controller.rb | 10 ++-------- app/controllers/outfits_controller.rb | 2 +- app/models/closet_hanger.rb | 21 ++++++++------------ app/models/closet_list.rb | 10 ++++------ 4 files changed, 15 insertions(+), 28 deletions(-) diff --git a/app/controllers/closet_hangers_controller.rb b/app/controllers/closet_hangers_controller.rb index fe3d33c4..9e9fbe64 100644 --- a/app/controllers/closet_hangers_controller.rb +++ b/app/controllers/closet_hangers_controller.rb @@ -235,7 +235,6 @@ class ClosetHangersController < ApplicationController lists, hangers_scope: hangers_scope, items_scope: items_scope, - item_translations_scope: item_translations_scope, ) lists.group_by(&:hangers_owned) end @@ -248,7 +247,6 @@ class ClosetHangersController < ApplicationController ClosetHanger.preload_items( hangers, items_scope: items_scope, - item_translations_scope: item_translations_scope, ) hangers.group_by(&:owned) else @@ -261,12 +259,8 @@ class ClosetHangersController < ApplicationController end def items_scope - Item.select(:id, :thumbnail_url, :rarity_index, :is_manually_nc) - end - - def item_translations_scope - Item::Translation.select(:id, :item_id, :locale, :name, :description). - where(locale: I18n.locale) + Item.select(:id, :name, :description, :thumbnail_url, :rarity_index, + :is_manually_nc) end def owned diff --git a/app/controllers/outfits_controller.rb b/app/controllers/outfits_controller.rb index ca371574..372ede4f 100644 --- a/app/controllers/outfits_controller.rb +++ b/app/controllers/outfits_controller.rb @@ -51,7 +51,7 @@ class OutfitsController < ApplicationController @species = Species.alphabetical newest_items = Item.newest. - select(:id, :updated_at, :thumbnail_url, :rarity_index, :is_manually_nc). + select(:id, :name, :updated_at, :thumbnail_url, :rarity_index, :is_manually_nc). includes(:translations).limit(18) @newest_modeled_items, @newest_unmodeled_items = newest_items.partition(&:predicted_fully_modeled?) diff --git a/app/models/closet_hanger.rb b/app/models/closet_hanger.rb index 7a67f1b3..000a12eb 100644 --- a/app/models/closet_hanger.rb +++ b/app/models/closet_hanger.rb @@ -13,9 +13,8 @@ class ClosetHanger < ApplicationRecord validate :list_belongs_to_user scope :alphabetical_by_item_name, -> { - it = Item::Translation.arel_table - joins(:item => :translations).where(it[:locale].eq(I18n.locale)). - order(it[:name].asc) + i = Item.arel_table + joins(:item).order(i[:name].asc) } scope :trading, -> { ch = arel_table @@ -86,28 +85,24 @@ class ClosetHanger < ApplicationRecord base end + # TODO: Is the performance improvement on this actually much better than just + # `includes`, now that `Item::Translation` records aren't part of it anymore? def self.preload_items( hangers, - items_scope: Item.all, - item_translations_scope: Item::Translation.all + items_scope: Item.all ) # Preload the records we need. (This is like `includes`, but `includes` # always selects all fields for all records, and we give the caller the # opportunity to specify which fields it actually wants via scope!) items = items_scope.where(id: hangers.map(&:item_id)) - translations = item_translations_scope.where(item_id: items.map(&:id)) # Group the records by relevant IDs. - translations_by_item_id = translations.group_by(&:item_id) items_by_id = items.to_h { |i| [i.id, i] } # Assign the preloaded records to the records they belong to. (This is like - # doing e.g. i.translations = ..., but that's a database write - we - # actually just want to set the `translations` field itself directly! - # Hacky, ripped from how `ActiveRecord::Associations::Preloader` does it!) - items.each do |item| - item.association(:translations).target = translations_by_item_id[item.id] - end + # doing e.g. h.item = ..., but that's a database write - we actually just + # want to set the `item` field itself directly! Hacky, ripped from how + # `ActiveRecord::Associations::Preloader` does it!) hangers.each do |hanger| hanger.association(:item).target = items_by_id[hanger.item_id] end diff --git a/app/models/closet_list.rb b/app/models/closet_list.rb index ea303db9..691e5243 100644 --- a/app/models/closet_list.rb +++ b/app/models/closet_list.rb @@ -45,8 +45,7 @@ class ClosetList < ApplicationRecord def self.preload_items( lists, hangers_scope: ClosetHanger.all, - items_scope: Item.all, - item_translations_scope: Item::Translation.all + items_scope: Item.all ) # Preload the records we need. (This is like `includes`, but `includes` # always selects all fields for all records, and we give the caller the @@ -57,9 +56,9 @@ class ClosetList < ApplicationRecord hangers_by_list_id = hangers.group_by(&:list_id) # Assign the preloaded records to the records they belong to. (This is like - # doing e.g. i.translations = ..., but that's a database write - we - # actually just want to set the `translations` field itself directly! - # Hacky, ripped from how `ActiveRecord::Associations::Preloader` does it!) + # doing e.g. h.item = ..., but that's a database write - we actually just + # want to set the `item` field itself directly! Hacky, ripped from how + # `ActiveRecord::Associations::Preloader` does it!) lists.each do |list| list.association(:hangers).target = hangers_by_list_id[list.id] end @@ -68,7 +67,6 @@ class ClosetList < ApplicationRecord ClosetHanger.preload_items( hangers, items_scope: items_scope, - item_translations_scope: item_translations_scope, ) end