Migrate item search away from item translations

Lightning fast for simple name queries now, gotta say!!

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

app/controllers/contributions_controller.rb

@@ -10,7 +10,6 @@ class ContributionsController < ApplicationController
     Contribution.preload_contributeds_and_parents(
       @contributions,
       :scopes => {
-        'Item' => Item.includes(:translations),
         'PetType' => PetType.includes(:species, :color),
         'AltStyle' => AltStyle.includes(:species, :color),
       }

app/controllers/items_controller.rb

@@ -11,10 +11,8 @@ class ItemsController < ApplicationController
         else
           per_page = 30
         end
-        # Note that we sort by name by hand, since we might have to use
-        # fallbacks after the fact
-        @items = @query.results.includes(:translations).
-          paginate(page: params[:page], per_page: per_page)
+        @items = @query.results.paginate(
+          page: params[:page], per_page: per_page)
         assign_closeted!
         respond_to do |format|
           format.html {
@@ -46,7 +44,7 @@ class ItemsController < ApplicationController
       respond_to do |format|
         format.html {
           @campaign = Fundraising::Campaign.current rescue nil
-          @newest_items = Item.newest.includes(:translations).limit(18)
+          @newest_items = Item.newest.limit(18)
         }
         format.js { render json: {error: '$q required'}}
       end
@@ -88,8 +86,7 @@ class ItemsController < ApplicationController
       raise ActiveRecord::RecordNotFound, 'Pet type not found'
     end
     
-    @items = @pet_type.needed_items.includes(:translations).
-      alphabetize_by_translations
+    @items = @pet_type.needed_items.order(:name)
     assign_closeted!
     
     respond_to do |format|

app/controllers/outfits_controller.rb

@@ -51,8 +51,8 @@ class OutfitsController < ApplicationController
     @species = Species.alphabetical
     
     newest_items = Item.newest.
-      select(:id, :updated_at, :thumbnail_url, :rarity_index, :is_manually_nc).
-      includes(:translations).limit(18)
+      select(:id, :name, :updated_at, :thumbnail_url, :rarity_index, :is_manually_nc)
+      .limit(18)
     @newest_modeled_items, @newest_unmodeled_items =
       newest_items.partition(&:predicted_fully_modeled?)
 

app/controllers/pets_controller.rb

@@ -9,10 +9,7 @@ class PetsController < ApplicationController
     # return modeling_disabled unless user_signed_in? && current_user.admin?
 
     raise Pet::PetNotFound unless params[:name]
-    @pet = Pet.load(
-      params[:name],
-      :item_scope => Item.includes(:translations),
-    )
+    @pet = Pet.load(params[:name])
     points = contribute(current_user, @pet)
     
     respond_to do |format|

app/controllers/sitemap_controller.rb

@@ -3,7 +3,7 @@ class SitemapController < ApplicationController
 
   def index
     respond_to do |format|
-      format.xml { @items = Item.includes(:translations).sitemap }
+      format.xml { @items = Item.sitemap }
     end
   end
 end

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

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
 

app/models/item.rb

@@ -6,7 +6,12 @@ class Item < ApplicationRecord
 
   SwfAssetType = 'object'
   
-  translates :name, :description, :rarity
+  # Keep the reference to the deprecated `Item::Translation` record, but don't
+  # bind it directly to any attributes anymore. We have some temporary writers
+  # that hack around the API to keep the attributes synced, while no longer
+  # reading *from* them by default.
+  # TODO: Remove once we're all done with translations, both here and in 2020!
+  translates
 
   has_many :closet_hangers
   has_one :contribution, :as => :contributed, :inverse_of => :contributed
@@ -23,13 +28,6 @@ class Item < ApplicationRecord
   cattr_reader :per_page
   @@per_page = 30
 
-  scope :alphabetize_by_translations, ->(locale) {
-    locale = locale or I18n.locale
-    it = Item::Translation.arel_table
-    joins(:translations).where(it[:locale].eq('en')).
-      order(it[:name].asc)
-  }
-
   scope :newest, -> {
     order(arel_table[:created_at].desc) if arel_table[:created_at]
   }
@@ -39,14 +37,10 @@ class Item < ApplicationRecord
   scope :with_closet_hangers, -> { joins(:closet_hangers) }
 
   scope :name_includes, ->(value, locale = I18n.locale) {
-    it = Item::Translation.arel_table
-    Item.joins(:translations).where(it[:locale].eq(locale)).
-      where(it[:name].matches('%' + sanitize_sql_like(value) + '%'))
+    Item.where("name LIKE ?", "%" + sanitize_sql_like(value) + "%")
   }
   scope :name_excludes, ->(value, locale = I18n.locale) {
-    it = Item::Translation.arel_table
-    Item.joins(:translations).where(it[:locale].eq(locale)).
-      where(it[:name].matches('%' + sanitize_sql_like(value) + '%').not)
+    Item.where("name NOT LIKE ?", "%" + sanitize_sql_like(value) + "%")
   }
   scope :is_nc, -> {
     i = Item.arel_table
@@ -57,16 +51,12 @@ class Item < ApplicationRecord
     where(i[:rarity_index].in(Item::NCRarities).or(i[:is_manually_nc].eq(true)).not)
   }
   scope :is_pb, -> {
-    it = Item::Translation.arel_table
-    joins(:translations).where(it[:locale].eq('en')).
-      where('description LIKE ?',
-        '%' + sanitize_sql_like(PAINTBRUSH_SET_DESCRIPTION) + '%')
+    where('description LIKE ?',
+      '%' + sanitize_sql_like(PAINTBRUSH_SET_DESCRIPTION) + '%')
   }
   scope :is_not_pb, -> {
-    it = Item::Translation.arel_table
-    joins(:translations).where(it[:locale].eq('en')).
-      where('description NOT LIKE ?',
-        '%' + sanitize_sql_like(PAINTBRUSH_SET_DESCRIPTION) + '%')
+    where('description NOT LIKE ?',
+      '%' + sanitize_sql_like(PAINTBRUSH_SET_DESCRIPTION) + '%')
   }
   scope :occupies, ->(zone_label) {
     zone_ids = Zone.matching_label(zone_label).map(&:id)
@@ -122,12 +112,30 @@ class Item < ApplicationRecord
       distinct
   }
 
+  # Temporary writers to keep the English translation record updated, while
+  # primarily using the attributes on the model itself.
+  #
+  # Once this app and DTI 2020 are both comfortably off the translation system,
+  # we can remove this!
+  def name=(new_name)
+    globalize.write(:en, :name, new_name)
+    write_attribute(:name, new_name)
+  end
+  def description=(new_description)
+    globalize.write(:en, :description, new_description)
+    write_attribute(:description, new_description)
+  end
+  def rarity=(new_rarity)
+    globalize.write(:en, :rarity, new_rarity)
+    write_attribute(:rarity, new_rarity)
+  end
+
   def nc_trade_value
     return nil unless nc?
     begin
-      OwlsValueGuide.find_by_name(name(:en))
+      OwlsValueGuide.find_by_name(name)
     rescue OwlsValueGuide::NotFound => error
-      Rails.logger.debug("No NC trade value listed for #{name(:en)} (#{id})")
+      Rails.logger.debug("No NC trade value listed for #{name} (#{id})")
       return nil
     rescue OwlsValueGuide::NetworkError => error
       Rails.logger.error("Couldn't load nc_trade_value: #{error.full_message}")
@@ -440,16 +448,16 @@ class Item < ApplicationRecord
     species_support_strs = info['species_support'] || []
     self.species_support_ids = species_support_strs.map(&:to_i)
 
-    self.name_translations = {locale => info['name']}
-
-    attribute_names.each do |attribute|
-      next if attribute == 'name'
-      value = info[attribute.to_sym]
-      if value
-        value = value.to_i if value.is_a? Float
-        self[attribute] = value
-      end
-    end
+    self.name = info['name']
+    self.description = info['description']
+    self.thumbnail_url = info['thumbnail_url']
+    self.category = info['category']
+    self.type = info['type']
+    self.rarity = info['rarity']
+    self.rarity_index = info['rarity_index'].to_i
+    self.price = info['price'].to_i
+    self.weight_lbs = info['weight_lbs'].to_i
+    self.zones_restrict = info['zones_restrict']
   end
 
   def pending_swf_assets

app/models/item/search/query.rb

@@ -11,8 +11,7 @@ class Item
       end
       
       def results
-        @filters.map(&:to_query).inject(Item.all, &:merge).
-          alphabetize_by_translations(Query.locale)
+        @filters.map(&:to_query).inject(Item.all, &:merge).order(:name)
       end
 
       def to_s

db/migrate/20240220230420_add_translated_fields_directly_to_items.rb

@@ -0,0 +1,26 @@
+class AddTranslatedFieldsDirectlyToItems < ActiveRecord::Migration[7.1]
+  def change
+    add_column :items, :name, :string, null: false
+    add_column :items, :description, :text, null: false, default: ""
+    add_column :items, :rarity, :string, null: false, default: ""
+
+    reversible do |direction|
+      direction.up do
+        total_count = Item.count
+        saved_count = 0
+        Item.includes(:translations).find_in_batches do |items|
+          Item.transaction do
+            items.each do |item|
+              item.name = item.translation_for(:en).name
+              item.description = item.translation_for(:en).description || ""
+              item.rarity = item.translation_for(:en).rarity || ""
+              item.save!
+            end
+            saved_count += items.size
+            puts "Saved #{saved_count} of #{total_count} items"
+          end
+        end
+      end
+    end
+  end
+end

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_03_161355) do
+ActiveRecord::Schema[7.1].define(version: 2024_02_20_230420) do
   create_table "alt_styles", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t|
     t.integer "species_id", null: false
     t.integer "color_id", null: false
@@ -144,6 +144,9 @@ ActiveRecord::Schema[7.1].define(version: 2024_02_03_161355) do
     t.integer "manual_special_color_id"
     t.column "modeling_status_hint", "enum('done','glitchy')"
     t.boolean "is_manually_nc", default: false, null: false
+    t.string "name", null: false
+    t.text "description", default: "", null: false
+    t.string "rarity", default: "", null: false
     t.index ["modeling_status_hint", "created_at", "id"], name: "items_modeling_status_hint_and_created_at_and_id"
     t.index ["modeling_status_hint", "created_at"], name: "items_modeling_status_hint_and_created_at"
     t.index ["modeling_status_hint", "id"], name: "items_modeling_status_hint_and_id"