Compare commits

...

4 commits

Author SHA1 Message Date
48f613d9bc Oops, fix recent bug in species:acara item search filter
Ah oops, I just typo'd this and didn't test it properly, womp womp!
2024-01-23 05:31:09 -08:00
413d2eed91 Improve species/color name migration performance & correctness
Oh right, this should only run in the up direction! And also we can get
a lot faster by preloading the translations.
2024-01-23 05:30:42 -08:00
ce64f12cc3 Remove references to species/color translations at call sites
We wisely did preloading in some places. Now, we don't need to!
Hopefully this'll even be a free speed boost on some pages!
2024-01-23 05:23:57 -08:00
0dca538b0a Start migrating off globalize gem for species/color names
Non-English languages haven't been supported on Neopets for a while, so
I'd like to remove this extra cross-cutting complexity, especially
since it's now inconsistent with the real site anyway!

The main motivation is that I'd like to do this for items too, because
I have a hunch that all the complexity of `globalize` to read
`item.name` is part of what's making large user lists so slow to
render: lots of little objects getting created down the stack, and
needing to be garbage-collected later.

I'm not sure that's why! But I figure removing this complexity is a
simplicity win anyway, so let's do it!

Note that this doesn't *finish* the migration, it just starts it! The
`Species::Translation` and `Color::Translation` models still exist, and
still have their data, and not all references to them are scrubbed yet.
I especially don't want to delete the backing tables until both DTI and
DTI 2020 are ready for it!

So this change will someday be paired with another change to actually
drop the tables - after backing up the data for future records just in
case, of course!
2024-01-23 05:10:43 -08:00
11 changed files with 62 additions and 56 deletions

View file

@ -11,8 +11,7 @@ class ContributionsController < ApplicationController
@contributions, @contributions,
:scopes => { :scopes => {
'Item' => Item.includes(:translations), 'Item' => Item.includes(:translations),
'PetType' => PetType.includes({:species => :translations, 'PetType' => PetType.includes(:species, :color)
:color => :translations})
} }
) )
end end

View file

@ -59,12 +59,9 @@ class OutfitsController < ApplicationController
@newest_unmodeled_items_predicted_missing_species_by_color = {} @newest_unmodeled_items_predicted_missing_species_by_color = {}
@newest_unmodeled_items_predicted_modeled_ratio = {} @newest_unmodeled_items_predicted_modeled_ratio = {}
@newest_unmodeled_items.each do |item| @newest_unmodeled_items.each do |item|
h = item.predicted_missing_nonstandard_body_ids_by_species_by_color( h = item.predicted_missing_nonstandard_body_ids_by_species_by_color
Color.includes(:translations),
Species.includes(:translations))
standard_body_ids_by_species = item. standard_body_ids_by_species = item.
predicted_missing_standard_body_ids_by_species( predicted_missing_standard_body_ids_by_species
Species.includes(:translations))
if standard_body_ids_by_species.present? if standard_body_ids_by_species.present?
h[:standard] = standard_body_ids_by_species h[:standard] = standard_body_ids_by_species
end end

View file

@ -2,7 +2,7 @@ class PetTypesController < ApplicationController
def index def index
color = Color.find params[:color_id] color = Color.find params[:color_id]
pet_types = color.pet_types.includes(pet_states: [:swf_assets]). pet_types = color.pet_types.includes(pet_states: [:swf_assets]).
includes(species: [:translations]) includes(:species)
# This is a relatively big request, for relatively static data. Let's # This is a relatively big request, for relatively static data. Let's
# consider it fresh for 10min (so new pet releases show up quickly), but # consider it fresh for 10min (so new pet releases show up quickly), but

View file

@ -106,7 +106,7 @@ module ItemsHelper
species_ids = species.map(&:id) species_ids = species.map(&:id)
pet_types = special_color ? pet_types = special_color ?
PetType.where(:color_id => special_color.id, :species_id => species_ids). PetType.where(:color_id => special_color.id, :species_id => species_ids).
order(:species_id).includes_child_translations : order(:species_id) :
PetType.random_basic_per_species(species.map(&:id)) PetType.random_basic_per_species(species.map(&:id))
pet_types.map(&block).join.html_safe pet_types.map(&block).join.html_safe
end end

View file

@ -1,27 +1,23 @@
class Color < ApplicationRecord class Color < ApplicationRecord
translates :name translates # TODO: Remove once we're all done with translations!
has_many :pet_types has_many :pet_types
scope :alphabetical, -> { scope :alphabetical, -> { order(:name) }
ct = Color::Translation.arel_table scope :basic, -> { where(basic: true) }
with_translations(I18n.locale).order(ct[:name].asc) scope :standard, -> { where(standard: true) }
} scope :nonstandard, -> { where(standard: false) }
scope :basic, -> { where(:basic => true) }
scope :standard, -> { where(:standard => true) }
scope :nonstandard, -> { where(:standard => false) }
scope :funny, -> { order(:prank) unless pranks_funny? } scope :funny, -> { order(:prank) unless pranks_funny? }
scope :matching_name, ->(name, locale = I18n.locale) {
ct = Color::Translation.arel_table
joins(:translations).where(ct[:locale].eq(locale)).
where(ct[:name].matches(sanitize_sql_like(name)))
}
validates :name, presence: true validates :name, presence: true
# TODO: Should we consider replacing this at call sites? This used to be # Temporary writer to keep the English translation record updated, while
# built into the globalize gem but isn't anymore! # primarily using the attribute on the model itself.
def self.find_by_name(name) #
matching_name(name).first # 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 end
def as_json(options={}) def as_json(options={})

View file

@ -352,8 +352,8 @@ class Item < ApplicationRecord
inject({}) { |h, pt| h[pt.species_id] = pt.body_id; h } inject({}) { |h, pt| h[pt.species_id] = pt.body_id; h }
end end
def predicted_missing_standard_body_ids_by_species(species_scope=Species.all) def predicted_missing_standard_body_ids_by_species
species = species_scope.where(id: predicted_missing_standard_body_ids_by_species_id.keys) species = Species.where(id: predicted_missing_standard_body_ids_by_species_id.keys)
species_by_id = species.inject({}) { |h, s| h[s.id] = s; h } species_by_id = species.inject({}) { |h, s| h[s.id] = s; h }
predicted_missing_standard_body_ids_by_species_id.inject({}) { |h, (sid, bid)| predicted_missing_standard_body_ids_by_species_id.inject({}) { |h, (sid, bid)|
h[species_by_id[sid]] = bid; h } h[species_by_id[sid]] = bid; h }
@ -365,16 +365,16 @@ class Item < ApplicationRecord
colors: {standard: false}) colors: {standard: false})
end end
def predicted_missing_nonstandard_body_ids_by_species_by_color(colors_scope=Color.all, species_scope=Species.all) def predicted_missing_nonstandard_body_ids_by_species_by_color
pet_types = predicted_missing_nonstandard_body_pet_types pet_types = predicted_missing_nonstandard_body_pet_types
species_by_id = {} species_by_id = {}
species_scope.find(pet_types.map(&:species_id)).each do |species| Species.find(pet_types.map(&:species_id)).each do |species|
species_by_id[species.id] = species species_by_id[species.id] = species
end end
colors_by_id = {} colors_by_id = {}
colors_scope.find(pet_types.map(&:color_id)).each do |color| Color.find(pet_types.map(&:color_id)).each do |color|
colors_by_id[color.id] = color colors_by_id[color.id] = color
end end

View file

@ -49,7 +49,7 @@ class Item
when 'fits' when 'fits'
color_name, species_name = value.split('-') color_name, species_name = value.split('-')
begin begin
pet_type = PetType.matching_name(color_name, species_name, locale).first! pet_type = PetType.matching_name(color_name, species_name).first!
rescue ActiveRecord::RecordNotFound rescue ActiveRecord::RecordNotFound
message = I18n.translate('items.search.errors.not_found.pet_type', message = I18n.translate('items.search.errors.not_found.pet_type',
name1: color_name.capitalize, name2: species_name.capitalize) name1: color_name.capitalize, name2: species_name.capitalize)
@ -60,8 +60,8 @@ class Item
Filter.not_fits(pet_type.body_id, color_name, species_name)) Filter.not_fits(pet_type.body_id, color_name, species_name))
when 'species' when 'species'
begin begin
species = Species.matching_name(value, locale).first! species = Species.find_by_name!(value)
color = Color.matching_name('blue', 'en').first! color = Color.find_by_name!('blue')
pet_type = PetType.where(color_id: color.id, species_id: species.id).first! pet_type = PetType.where(color_id: color.id, species_id: species.id).first!
rescue ActiveRecord::RecordNotFound rescue ActiveRecord::RecordNotFound
message = I18n.translate('items.search.errors.not_found.species', message = I18n.translate('items.search.errors.not_found.species',

View file

@ -19,12 +19,9 @@ class PetType < ApplicationRecord
scope :nonstandard_colors, -> { where(:color_id => Color.nonstandard) } scope :nonstandard_colors, -> { where(:color_id => Color.nonstandard) }
scope :includes_child_translations, scope :matching_name, ->(color_name, species_name) {
-> { includes({:color => :translations, :species => :translations}) } color = Color.find_by_name!(color_name)
species = Species.find_by_name!(species_name)
scope :matching_name, ->(color_name, species_name, locale = I18n.locale) {
color = Color.matching_name(color_name, locale).first!
species = Species.matching_name(species_name, locale).first!
where(color_id: color.id, species_id: species.id) where(color_id: color.id, species_id: species.id)
} }

View file

@ -1,27 +1,22 @@
class Species < ApplicationRecord class Species < ApplicationRecord
translates :name translates # TODO: Remove once we're all done with translations!
has_many :pet_types has_many :pet_types
scope :alphabetical, -> { scope :alphabetical, -> { order(:name) }
st = Species::Translation.arel_table
with_translations(I18n.locale).order(st[:name].asc)
}
scope :matching_name, ->(name, locale = I18n.locale) {
st = Species::Translation.arel_table
joins(:translations).where(st[:locale].eq(locale)).
where(st[:name].matches(sanitize_sql_like(name)))
}
scope :with_body_id, -> body_id { scope :with_body_id, -> body_id {
pt = PetType.arel_table pt = PetType.arel_table
joins(:pet_types).where(pt[:body_id].eq(body_id)).limit(1) joins(:pet_types).where(pt[:body_id].eq(body_id)).limit(1)
} }
# TODO: Should we consider replacing this at call sites? This used to be # Temporary writer to keep the English translation record updated, while
# built into the globalize gem but isn't anymore! # primarily using the attribute on the model itself.
def self.find_by_name(name) #
matching_name(name).first # 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 end
def as_json(options={}) def as_json(options={})

View file

@ -0,0 +1,20 @@
class AddNameToSpeciesAndColor < ActiveRecord::Migration[7.1]
def change
add_column :species, :name, :string, null: false
add_column :colors, :name, :string, null: false
reversible do |direction|
direction.up do
Species.includes(:translations).find_each do |species|
species.name = species.translation_for(:en).name
species.save!
end
Color.includes(:translations).find_each do |color|
color.name = color.translation_for(:en).name
color.save!
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_01_19_061745) do ActiveRecord::Schema[7.1].define(version: 2024_01_23_125509) do
create_table "auth_servers", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t| create_table "auth_servers", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.string "short_name", limit: 10, null: false t.string "short_name", limit: 10, null: false
t.string "name", limit: 40, null: false t.string "name", limit: 40, null: false
@ -74,6 +74,7 @@ ActiveRecord::Schema[7.1].define(version: 2024_01_19_061745) do
t.boolean "basic" t.boolean "basic"
t.boolean "standard" t.boolean "standard"
t.boolean "prank", default: false, null: false t.boolean "prank", default: false, null: false
t.string "name", null: false
end end
create_table "contributions", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t| create_table "contributions", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
@ -233,6 +234,7 @@ ActiveRecord::Schema[7.1].define(version: 2024_01_19_061745) do
end end
create_table "species", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t| create_table "species", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.string "name", null: false
end end
create_table "species_translations", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t| create_table "species_translations", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|