From 97ffffb67ae21de6092c86f07ef3245eb46ebee9 Mon Sep 17 00:00:00 2001 From: Emi Matchu <emi@matchu.dev> Date: Sat, 15 Feb 2025 21:52:47 -0800 Subject: [PATCH] Add configurable full name field to alt styles Sigh, the "Valentine Plushie" series is messing with me again! It doesn't follow the previously established pattern of the names being "<series> <color> <species>", because in this case the base color is considered "Valentine". Okay, well! In this change we add `full_name` as an explicit database field, and set the previous full name value as a fallback. (We also extract the generic fallback logic into `ApplicationRecord`, to help us express it more concisely.) We also tweak `adjective_name` to be able to shorten custom `full_name` values, too. That way, in the outfit editor, the Styles options show correct values like "Cherub Plushie" for the "Cherub Plushie Acara". I also make some changes in the outfit editor to better accommodate the longer series names, to try to better handle long words but also to just only use the first word of the series main name anyway. (Currently, all series main names are one word, except "Valentine Plushie" becomes "Valentine".) --- app/assets/stylesheets/alt_styles/index.sass | 6 +- app/controllers/alt_styles_controller.rb | 3 +- .../WardrobePage/OutfitControls.js | 2 +- .../wardrobe-2020/WardrobePage/PosePicker.js | 11 +- app/models/alt_style.rb | 33 +----- app/models/application_record.rb | 32 +++++ app/views/alt_styles/_alt_style.html.haml | 8 +- app/views/alt_styles/edit.html.haml | 4 + ...50216041650_add_full_name_to_alt_styles.rb | 5 + db/openneo_id_schema.rb | 2 +- db/schema.rb | 3 +- lib/tasks/neopets/import/styling_studio.rake | 10 +- spec/models/alt_style_spec.rb | 112 ++++++++++++++++++ 13 files changed, 188 insertions(+), 43 deletions(-) create mode 100644 db/migrate/20250216041650_add_full_name_to_alt_styles.rb create mode 100644 spec/models/alt_style_spec.rb diff --git a/app/assets/stylesheets/alt_styles/index.sass b/app/assets/stylesheets/alt_styles/index.sass index f2586027..66f146c1 100644 --- a/app/assets/stylesheets/alt_styles/index.sass +++ b/app/assets/stylesheets/alt_styles/index.sass @@ -1,9 +1,9 @@ @import "../partials/clean/constants" -// Prefer to break the name at certain points. +// Prefer to break the name at visually appealing points. .rainbow-pool-list - .name span - display: inline-block + .name + text-wrap: balance // De-emphasize Prismatic styles, in browsers that support it. .rainbow-pool-filters diff --git a/app/controllers/alt_styles_controller.rb b/app/controllers/alt_styles_controller.rb index 9da54fde..aad704c6 100644 --- a/app/controllers/alt_styles_controller.rb +++ b/app/controllers/alt_styles_controller.rb @@ -61,7 +61,8 @@ class AltStylesController < ApplicationController protected def alt_style_params - params.require(:alt_style).permit(:real_series_name, :thumbnail_url) + params.require(:alt_style). + permit(:real_series_name, :real_full_name, :thumbnail_url) end def find_color diff --git a/app/javascript/wardrobe-2020/WardrobePage/OutfitControls.js b/app/javascript/wardrobe-2020/WardrobePage/OutfitControls.js index bdd98f54..65465378 100644 --- a/app/javascript/wardrobe-2020/WardrobePage/OutfitControls.js +++ b/app/javascript/wardrobe-2020/WardrobePage/OutfitControls.js @@ -233,7 +233,7 @@ function OutfitControls({ /> </DarkMode> </Box> - <Flex flex="0 0 auto" align="center" pl="2"> + <Flex flex="0 1 auto" align="center" pl="2"> <PosePicker speciesId={outfitState.speciesId} colorId={outfitState.colorId} diff --git a/app/javascript/wardrobe-2020/WardrobePage/PosePicker.js b/app/javascript/wardrobe-2020/WardrobePage/PosePicker.js index d13db88d..f9dee373 100644 --- a/app/javascript/wardrobe-2020/WardrobePage/PosePicker.js +++ b/app/javascript/wardrobe-2020/WardrobePage/PosePicker.js @@ -283,7 +283,10 @@ const PosePickerButton = React.forwardRef( const theme = useTheme(); const icon = altStyle != null ? twemojiSunglasses : getIcon(pose); - const label = altStyle != null ? altStyle.seriesMainName : getLabel(pose); + const label = + altStyle != null + ? altStyle.seriesMainName.split(/\s+/)[0] + : getLabel(pose); return ( <ClassNames> @@ -336,9 +339,9 @@ const PosePickerButton = React.forwardRef( ref={ref} > <EmojiImage src={icon} alt="Style" /> - <Box width=".5em" /> - {label} - <Box width=".5em" /> + <Box overflow="hidden" textOverflow="ellipsis" marginX=".5em"> + {label} + </Box> <ChevronDownIcon /> </Button> )} diff --git a/app/models/alt_style.rb b/app/models/alt_style.rb index b343fdc1..751ca3e8 100644 --- a/app/models/alt_style.rb +++ b/app/models/alt_style.rb @@ -9,11 +9,15 @@ class AltStyle < ApplicationRecord has_many :contributions, as: :contributed, inverse_of: :contributed validates :body_id, presence: true + validates :full_name, presence: true, allow_nil: true validates :series_name, presence: true, allow_nil: true validates :thumbnail_url, presence: true before_validation :infer_thumbnail_url, unless: :thumbnail_url? + fallback_for(:full_name) { "#{series_name} #{pet_name}" } + fallback_for(:series_name) { AltStyle.placeholder_name } + scope :matching_name, ->(series_name, color_name, species_name) { color = Color.find_by_name!(color_name) species = Species.find_by_name!(species_name) @@ -54,28 +58,6 @@ class AltStyle < ApplicationRecord alias_method :name, :pet_name - # If the series_name hasn't yet been set manually by support staff, show the - # string "<New?>" instead. But it won't be searchable by that string—that is, - # `fits:<New?>-faerie-draik` intentionally will not work, and the canonical - # filter name will be `fits:alt-style-IDNUMBER`, instead. - def series_name - real_series_name || AltStyle.placeholder_name - end - - def real_series_name=(new_series_name) - self[:series_name] = new_series_name - end - - def real_series_name - self[:series_name] - end - - # You can use this to check whether `series_name` is returning the actual - # value or its placeholder value. - def real_series_name? - real_series_name.present? - end - def series_main_name series_name.split(': ').last end @@ -84,12 +66,9 @@ class AltStyle < ApplicationRecord series_name.split(': ').first end + # Returns the full name, with the species removed from the end (if present). def adjective_name - "#{series_name} #{color.human_name}" - end - - def full_name - "#{series_name} #{name}" + full_name.sub(/\s+#{Regexp.escape(species.name)}\Z/i, "") end EMPTY_IMAGE_URL = "data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 71a1a03c..a02a0e36 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -1,3 +1,35 @@ class ApplicationRecord < ActiveRecord::Base self.abstract_class = true + + # When the given attribute's value is nil, return the given block's value + # instead. This is useful for fields where there's a sensible derived value + # to use as the default for display purposes, but it's distinctly *not* the + # true value, and should be recognizable as such. + # + # This also creates methods `real_<attr>`, `real_<attr>=`, and `real_<attr>?`, + # to work with the actual attribute when necessary. + # + # It also creates `fallback_<attr>`, to find what the fallback value *would* + # be if the attribute's value were nil. + def self.fallback_for(attribute_name, &block) + define_method attribute_name do + read_attribute(attribute_name) || instance_eval(&block) + end + + define_method "real_#{attribute_name}" do + read_attribute(attribute_name) + end + + define_method "real_#{attribute_name}?" do + read_attribute(attribute_name).present? + end + + define_method "real_#{attribute_name}=" do |new_value| + write_attribute(attribute_name, new_value) + end + + define_method "fallback_#{attribute_name}" do + instance_eval(&block) + end + end end \ No newline at end of file diff --git a/app/views/alt_styles/_alt_style.html.haml b/app/views/alt_styles/_alt_style.html.haml index 9be4b858..9f20ab94 100644 --- a/app/views/alt_styles/_alt_style.html.haml +++ b/app/views/alt_styles/_alt_style.html.haml @@ -1,12 +1,12 @@ %li = link_to view_or_edit_alt_style_url(alt_style) do = image_tag alt_style.preview_image_url, class: "preview", loading: "lazy" - .name - %span= alt_style.series_name - %span= alt_style.pet_name + .name= alt_style.full_name .info %p Added = time_tag alt_style.created_at, title: alt_style.created_at.to_formatted_s(:long_nst) do - = time_with_only_month_if_old alt_style.created_at \ No newline at end of file + = time_with_only_month_if_old alt_style.created_at + - if support_staff? && !alt_style.real_series_name? + %p ⚠️ Needs series name diff --git a/app/views/alt_styles/edit.html.haml b/app/views/alt_styles/edit.html.haml index d6ee1835..6daaa8ba 100644 --- a/app/views/alt_styles/edit.html.haml +++ b/app/views/alt_styles/edit.html.haml @@ -22,6 +22,10 @@ = f.text_field :real_series_name, autofocus: !@alt_style.real_series_name?, placeholder: AltStyle.placeholder_name + = f.field do + = f.label :real_series_name, "Full name" + = f.text_field :real_full_name, placeholder: @alt_style.fallback_full_name + = f.field do = f.label :thumbnail_url, "Thumbnail" = f.thumbnail_input :thumbnail_url diff --git a/db/migrate/20250216041650_add_full_name_to_alt_styles.rb b/db/migrate/20250216041650_add_full_name_to_alt_styles.rb new file mode 100644 index 00000000..0d27e085 --- /dev/null +++ b/db/migrate/20250216041650_add_full_name_to_alt_styles.rb @@ -0,0 +1,5 @@ +class AddFullNameToAltStyles < ActiveRecord::Migration[8.0] + def change + add_column :alt_styles, :full_name, :string, null: true + end +end diff --git a/db/openneo_id_schema.rb b/db/openneo_id_schema.rb index e6d281a4..250d7a4d 100644 --- a/db/openneo_id_schema.rb +++ b/db/openneo_id_schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_04_08_120359) do +ActiveRecord::Schema[8.0].define(version: 2024_04_08_120359) do create_table "users", id: { type: :integer, unsigned: true }, charset: "utf8mb3", collation: "utf8mb3_general_ci", force: :cascade do |t| t.string "name", limit: 30, null: false t.string "encrypted_password", limit: 64 diff --git a/db/schema.rb b/db/schema.rb index 6c888b83..809c71ff 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_11_19_214543) do +ActiveRecord::Schema[8.0].define(version: 2025_02_16_041650) do create_table "alt_styles", charset: "utf8mb4", collation: "utf8mb4_unicode_520_ci", force: :cascade do |t| t.integer "species_id", null: false t.integer "color_id", null: false @@ -19,6 +19,7 @@ ActiveRecord::Schema[7.2].define(version: 2024_11_19_214543) do t.datetime "updated_at", precision: nil, null: false t.string "series_name" t.string "thumbnail_url", null: false + t.string "full_name" t.index ["color_id"], name: "index_alt_styles_on_color_id" t.index ["species_id"], name: "index_alt_styles_on_species_id" end diff --git a/lib/tasks/neopets/import/styling_studio.rake b/lib/tasks/neopets/import/styling_studio.rake index 8284c393..322eaa72 100644 --- a/lib/tasks/neopets/import/styling_studio.rake +++ b/lib/tasks/neopets/import/styling_studio.rake @@ -47,6 +47,14 @@ namespace "neopets:import" do next end + if !record.real_full_name? + record.full_name = style[:name] + puts "✅ [#{label}]: Full name is now #{style[:name].inspect}" + elsif record.full_name != style[:name] + puts "⚠️ [#{label}: Full name may have changed, handle manually? " + + "#{record.full_name.inspect} -> #{style[:name].inspect}" + end + if !record.real_thumbnail_url? record.thumbnail_url = style[:image] puts "✅ [#{label}]: Thumbnail URL is now #{style[:image].inspect}" @@ -72,7 +80,7 @@ namespace "neopets:import" do end else puts "⚠️ [#{label}]: Unable to detect series name, handle manually? " + - "#{record.full_name.inspect} -> #{style[:name].inspect}" + "#{record.pet_name.inspect} <-> #{style[:name].inspect}" end if record.changed? diff --git a/spec/models/alt_style_spec.rb b/spec/models/alt_style_spec.rb new file mode 100644 index 00000000..38546838 --- /dev/null +++ b/spec/models/alt_style_spec.rb @@ -0,0 +1,112 @@ +require_relative '../rails_helper' + +RSpec.describe AltStyle do + fixtures :colors, :species + + describe "series name" do + subject(:alt_style) { AltStyle.new } + + describe "for a new alt style" do + it("returns a placeholder value by default") do + expect(alt_style.series_name).to eq "<New?>" + end + + it("has no real_series_name by default") do + expect(alt_style.real_series_name?).to be false + expect(alt_style.real_series_name).to be nil + end + end + + describe "with a real series name" do + before { alt_style.real_series_name = "Nostalgic" } + + it("returns the real series name") do + expect(alt_style.series_name).to eq "Nostalgic" + end + + it("has a real_series_name field too") do + expect(alt_style.real_series_name?).to be true + expect(alt_style.real_series_name).to eq "Nostalgic" + end + end + end + + describe "full name" do + subject(:alt_style) do + AltStyle.new(color: colors(:blue), species: species(:acara)) + end + + describe "for a new alt style" do + it("returns a placeholder value by default") do + expect(alt_style.full_name).to eq "<New?> Blue Acara" + end + + it("has no real_full_name by default") do + expect(alt_style.real_full_name?).to be false + expect(alt_style.real_full_name).to be nil + end + end + + describe "with a real series name, but no full name" do + before { alt_style.real_series_name = "Nostalgic" } + + it("returns a placeholder value including the real series name") do + expect(alt_style.full_name).to eq "Nostalgic Blue Acara" + end + + it("still has no real_full_name") do + expect(alt_style.real_full_name?).to be false + expect(alt_style.real_full_name).to eq nil + end + end + + describe "with a real full name" do + before { alt_style.real_full_name = "Cute Lil Guy" } + + it("returns the real full name") do + expect(alt_style.full_name).to eq "Cute Lil Guy" + end + + it("has a real_full_name field too") do + expect(alt_style.real_full_name?).to be true + expect(alt_style.real_full_name).to eq "Cute Lil Guy" + end + end + end + + describe "adjective name" do + subject(:alt_style) do + AltStyle.new(color: colors(:blue), species: species(:acara)) + end + + describe "for a new alt style" do + it("returns the placeholder series name and color name") do + expect(alt_style.adjective_name).to eq "<New?> Blue" + end + end + + describe "with a real series name, but no full name" do + before { alt_style.series_name = "Nostalgic" } + + it("returns the series name and color name") do + expect(alt_style.adjective_name).to eq "Nostalgic Blue" + end + end + + describe "with a real full name that ends with the species name" do + before { alt_style.real_full_name = "Cute Lil Acara" } + + it("returns the real full name minus the species") do + expect(alt_style.adjective_name).to eq "Cute Lil" + end + end + + describe "with a real full name that does not end with the species name" do + before { alt_style.real_full_name = "Cute Lil Guy" } + + it("returns the real full name") do + expect(alt_style.adjective_name).to eq "Cute Lil Guy" + end + end + end +end