1
0
Fork 0
forked from OpenNeo/impress

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!
This commit is contained in:
Emi Matchu 2024-01-23 05:10:43 -08:00
parent b28459c1cf
commit 0dca538b0a
6 changed files with 47 additions and 35 deletions

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

@ -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', 'en')
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

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

View file

@ -1,5 +1,5 @@
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, -> {
@ -7,21 +7,19 @@ class Species < ApplicationRecord
with_translations(I18n.locale).order(st[:name].asc) 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,16 @@
class AddNameToSpeciesAndColor < ActiveRecord::Migration[7.1]
def change
add_column :species, :name, :string, null: false
add_column :colors, :name, :string, null: false
Species.find_each do |species|
species.name = species.translation_for(:en).name
species.save!
end
Color.find_each do |color|
color.name = color.translation_for(:en).name
color.save!
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|