Compare commits

...

8 commits

Author SHA1 Message Date
3ac9e7ce69 Migrate item search away from item translations
Lightning fast for simple name queries now, gotta say!!
2024-02-20 16:04:41 -08:00
04af1ee319 Migrate away from item translations in misc pages 2024-02-20 15:53:56 -08:00
b7bc0ecd70 Migrate away from item translations in contributions 2024-02-20 15:52:10 -08:00
5ee3b472ec Migrate away from item translations in modeling
This one is important, I didn't notice that this is a way of setting
attributes that won't be written to both tables! `name` will only be
written to the translation table (which crashes the save), and the
other fields would only be written to the main table. Fixed! (I don't
like the super-dynamic this code was written before, anyway.)
2024-02-20 15:52:03 -08:00
0e8f457aa1 Oops, fix bug on item page now that translations aren't available
Missed this at first - now that the `name` field is just a normal field
and is always English, it's now an error to provide the locale to it as
a parameter, like we used to for the translated version of the field!
2024-02-20 15:37:07 -08:00
c75d988497 Migrate away from item translations in the Your Items feature
Just replacing references to the `Item::Translation` model to the
fields on `Item` itself!
2024-02-20 15:36:20 -08:00
a1066d9c8a Add translated item fields directly to the Item model
Like with Species, Color, and Zone, we're moving the translation data
directly onto the model, and just using English. This will simplify
some of our queries a lot (way fewer joins!), and it's what Neopets
does now anyway, and I have a secret hope that removing the complexity
along the codepath for `item.name` might help speed up large item lists
if we're lucky?? 🤞

Anyway, this is the first step, performing the migration to copy the
data onto the `items` table, making sure to keep them in sync for the
2020 app for now!
2024-02-20 15:25:03 -08:00
c215ebee09 Remove old unhelpful comment in item search
I think this was to explain why `order` wasn't part of this query, and
we probably used to sort in the controller? But now the item search
module takes care of all that, this is just confusing to say now imo!
2024-02-20 14:58:01 -08:00
12 changed files with 95 additions and 79 deletions

View file

@ -235,7 +235,6 @@ class ClosetHangersController < ApplicationController
lists, lists,
hangers_scope: hangers_scope, hangers_scope: hangers_scope,
items_scope: items_scope, items_scope: items_scope,
item_translations_scope: item_translations_scope,
) )
lists.group_by(&:hangers_owned) lists.group_by(&:hangers_owned)
end end
@ -248,7 +247,6 @@ class ClosetHangersController < ApplicationController
ClosetHanger.preload_items( ClosetHanger.preload_items(
hangers, hangers,
items_scope: items_scope, items_scope: items_scope,
item_translations_scope: item_translations_scope,
) )
hangers.group_by(&:owned) hangers.group_by(&:owned)
else else
@ -261,12 +259,8 @@ class ClosetHangersController < ApplicationController
end end
def items_scope def items_scope
Item.select(:id, :thumbnail_url, :rarity_index, :is_manually_nc) Item.select(:id, :name, :description, :thumbnail_url, :rarity_index,
end :is_manually_nc)
def item_translations_scope
Item::Translation.select(:id, :item_id, :locale, :name, :description).
where(locale: I18n.locale)
end end
def owned def owned

View file

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

View file

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

View file

@ -51,8 +51,8 @@ class OutfitsController < ApplicationController
@species = Species.alphabetical @species = Species.alphabetical
newest_items = Item.newest. 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) .limit(18)
@newest_modeled_items, @newest_unmodeled_items = @newest_modeled_items, @newest_unmodeled_items =
newest_items.partition(&:predicted_fully_modeled?) newest_items.partition(&:predicted_fully_modeled?)

View file

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

View file

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

View file

@ -13,9 +13,8 @@ class ClosetHanger < ApplicationRecord
validate :list_belongs_to_user validate :list_belongs_to_user
scope :alphabetical_by_item_name, -> { scope :alphabetical_by_item_name, -> {
it = Item::Translation.arel_table i = Item.arel_table
joins(:item => :translations).where(it[:locale].eq(I18n.locale)). joins(:item).order(i[:name].asc)
order(it[:name].asc)
} }
scope :trading, -> { scope :trading, -> {
ch = arel_table ch = arel_table
@ -86,28 +85,24 @@ class ClosetHanger < ApplicationRecord
base base
end 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( def self.preload_items(
hangers, hangers,
items_scope: Item.all, items_scope: Item.all
item_translations_scope: Item::Translation.all
) )
# Preload the records we need. (This is like `includes`, but `includes` # Preload the records we need. (This is like `includes`, but `includes`
# always selects all fields for all records, and we give the caller the # always selects all fields for all records, and we give the caller the
# opportunity to specify which fields it actually wants via scope!) # opportunity to specify which fields it actually wants via scope!)
items = items_scope.where(id: hangers.map(&:item_id)) 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. # 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] } items_by_id = items.to_h { |i| [i.id, i] }
# Assign the preloaded records to the records they belong to. (This is like # 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 # doing e.g. h.item = ..., but that's a database write - we actually just
# actually just want to set the `translations` field itself directly! # want to set the `item` field itself directly! Hacky, ripped from how
# Hacky, ripped from how `ActiveRecord::Associations::Preloader` does it!) # `ActiveRecord::Associations::Preloader` does it!)
items.each do |item|
item.association(:translations).target = translations_by_item_id[item.id]
end
hangers.each do |hanger| hangers.each do |hanger|
hanger.association(:item).target = items_by_id[hanger.item_id] hanger.association(:item).target = items_by_id[hanger.item_id]
end end

View file

@ -45,8 +45,7 @@ class ClosetList < ApplicationRecord
def self.preload_items( def self.preload_items(
lists, lists,
hangers_scope: ClosetHanger.all, hangers_scope: ClosetHanger.all,
items_scope: Item.all, items_scope: Item.all
item_translations_scope: Item::Translation.all
) )
# Preload the records we need. (This is like `includes`, but `includes` # Preload the records we need. (This is like `includes`, but `includes`
# always selects all fields for all records, and we give the caller the # 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) hangers_by_list_id = hangers.group_by(&:list_id)
# Assign the preloaded records to the records they belong to. (This is like # 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 # doing e.g. h.item = ..., but that's a database write - we actually just
# actually just want to set the `translations` field itself directly! # want to set the `item` field itself directly! Hacky, ripped from how
# Hacky, ripped from how `ActiveRecord::Associations::Preloader` does it!) # `ActiveRecord::Associations::Preloader` does it!)
lists.each do |list| lists.each do |list|
list.association(:hangers).target = hangers_by_list_id[list.id] list.association(:hangers).target = hangers_by_list_id[list.id]
end end
@ -68,7 +67,6 @@ class ClosetList < ApplicationRecord
ClosetHanger.preload_items( ClosetHanger.preload_items(
hangers, hangers,
items_scope: items_scope, items_scope: items_scope,
item_translations_scope: item_translations_scope,
) )
end end

View file

@ -6,7 +6,12 @@ class Item < ApplicationRecord
SwfAssetType = 'object' 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_many :closet_hangers
has_one :contribution, :as => :contributed, :inverse_of => :contributed has_one :contribution, :as => :contributed, :inverse_of => :contributed
@ -23,13 +28,6 @@ class Item < ApplicationRecord
cattr_reader :per_page cattr_reader :per_page
@@per_page = 30 @@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, -> { scope :newest, -> {
order(arel_table[:created_at].desc) if arel_table[:created_at] 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 :with_closet_hangers, -> { joins(:closet_hangers) }
scope :name_includes, ->(value, locale = I18n.locale) { scope :name_includes, ->(value, locale = I18n.locale) {
it = Item::Translation.arel_table Item.where("name LIKE ?", "%" + sanitize_sql_like(value) + "%")
Item.joins(:translations).where(it[:locale].eq(locale)).
where(it[:name].matches('%' + sanitize_sql_like(value) + '%'))
} }
scope :name_excludes, ->(value, locale = I18n.locale) { scope :name_excludes, ->(value, locale = I18n.locale) {
it = Item::Translation.arel_table Item.where("name NOT LIKE ?", "%" + sanitize_sql_like(value) + "%")
Item.joins(:translations).where(it[:locale].eq(locale)).
where(it[:name].matches('%' + sanitize_sql_like(value) + '%').not)
} }
scope :is_nc, -> { scope :is_nc, -> {
i = Item.arel_table i = Item.arel_table
@ -57,14 +51,10 @@ class Item < ApplicationRecord
where(i[:rarity_index].in(Item::NCRarities).or(i[:is_manually_nc].eq(true)).not) where(i[:rarity_index].in(Item::NCRarities).or(i[:is_manually_nc].eq(true)).not)
} }
scope :is_pb, -> { scope :is_pb, -> {
it = Item::Translation.arel_table
joins(:translations).where(it[:locale].eq('en')).
where('description LIKE ?', where('description LIKE ?',
'%' + sanitize_sql_like(PAINTBRUSH_SET_DESCRIPTION) + '%') '%' + sanitize_sql_like(PAINTBRUSH_SET_DESCRIPTION) + '%')
} }
scope :is_not_pb, -> { scope :is_not_pb, -> {
it = Item::Translation.arel_table
joins(:translations).where(it[:locale].eq('en')).
where('description NOT LIKE ?', where('description NOT LIKE ?',
'%' + sanitize_sql_like(PAINTBRUSH_SET_DESCRIPTION) + '%') '%' + sanitize_sql_like(PAINTBRUSH_SET_DESCRIPTION) + '%')
} }
@ -122,12 +112,30 @@ class Item < ApplicationRecord
distinct 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 def nc_trade_value
return nil unless nc? return nil unless nc?
begin begin
OwlsValueGuide.find_by_name(name(:en)) OwlsValueGuide.find_by_name(name)
rescue OwlsValueGuide::NotFound => error 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 return nil
rescue OwlsValueGuide::NetworkError => error rescue OwlsValueGuide::NetworkError => error
Rails.logger.error("Couldn't load nc_trade_value: #{error.full_message}") 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'] || [] species_support_strs = info['species_support'] || []
self.species_support_ids = species_support_strs.map(&:to_i) self.species_support_ids = species_support_strs.map(&:to_i)
self.name_translations = {locale => info['name']} self.name = info['name']
self.description = info['description']
attribute_names.each do |attribute| self.thumbnail_url = info['thumbnail_url']
next if attribute == 'name' self.category = info['category']
value = info[attribute.to_sym] self.type = info['type']
if value self.rarity = info['rarity']
value = value.to_i if value.is_a? Float self.rarity_index = info['rarity_index'].to_i
self[attribute] = value self.price = info['price'].to_i
end self.weight_lbs = info['weight_lbs'].to_i
end self.zones_restrict = info['zones_restrict']
end end
def pending_swf_assets def pending_swf_assets

View file

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

View file

@ -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

View file

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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| create_table "alt_styles", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t|
t.integer "species_id", null: false t.integer "species_id", null: false
t.integer "color_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.integer "manual_special_color_id"
t.column "modeling_status_hint", "enum('done','glitchy')" t.column "modeling_status_hint", "enum('done','glitchy')"
t.boolean "is_manually_nc", default: false, null: false 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", "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", "created_at"], name: "items_modeling_status_hint_and_created_at"
t.index ["modeling_status_hint", "id"], name: "items_modeling_status_hint_and_id" t.index ["modeling_status_hint", "id"], name: "items_modeling_status_hint_and_id"