Add series_name database field to alt styles

Previously we did this hackily by comparing the ID to a hardcoded list
of IDs, but I think putting this in the database is clearer and more
robust, and it should also help with our upcoming item search stuff
that will filter by it!

app/controllers/items_controller.rb

@@ -4,55 +4,53 @@ class ItemsController < ApplicationController
 
   def index
     if @query
-      begin
-        if params[:per_page]
-          per_page = params[:per_page].to_i
-          per_page = 50 if per_page && per_page > 50
-        else
-          per_page = 30
-        end
+      if params[:per_page]
+        per_page = params[:per_page].to_i
+        per_page = 50 if per_page && per_page > 50
+      else
+        per_page = 30
+      end
 
-        @items = @query.results.paginate(
-          page: params[:page], per_page: per_page)
-        assign_closeted!
+      @items = @query.results.paginate(
+        page: params[:page], per_page: per_page)
+      assign_closeted!
 
-        respond_to do |format|
-          format.html {
-            @campaign = Fundraising::Campaign.current rescue nil
-            if @items.count == 1
-              redirect_to @items.first
-            else
-              render
-            end
-          }
-          format.json {
-            render json: {
-              items: @items.as_json(
-                methods: [:nc?, :pb?, :owned?, :wanted?],
-              ),
-              appearances: load_appearances.as_json(
-                include: {
-                  swf_assets: {
-                    only: [:id, :remote_id, :body_id],
-                    include: {
-                      zone: {
-                        only: [:id, :depth, :label],
-                        methods: [:is_commonly_used_by_items],
-                      },
-                      restricted_zones: {
-                        only: [:id, :depth, :label],
-                        methods: [:is_commonly_used_by_items],
-                      },
+      respond_to do |format|
+        format.html {
+          @campaign = Fundraising::Campaign.current rescue nil
+          if @items.count == 1
+            redirect_to @items.first
+          else
+            render
+          end
+        }
+        format.json {
+          render json: {
+            items: @items.as_json(
+              methods: [:nc?, :pb?, :owned?, :wanted?],
+            ),
+            appearances: load_appearances.as_json(
+              include: {
+                swf_assets: {
+                  only: [:id, :remote_id, :body_id],
+                  include: {
+                    zone: {
+                      only: [:id, :depth, :label],
+                      methods: [:is_commonly_used_by_items],
+                    },
+                    restricted_zones: {
+                      only: [:id, :depth, :label],
+                      methods: [:is_commonly_used_by_items],
                     },
-                    methods: [:urls, :known_glitches],
                   },
-                }
-              ),
-              total_pages: @items.total_pages,
-              query: @query.to_s,
-            }
+                  methods: [:urls, :known_glitches],
+                },
+              }
+            ),
+            total_pages: @items.total_pages,
+            query: @query.to_s,
           }
-        end
+        }
       end
     elsif params.has_key?(:ids) && params[:ids].is_a?(Array)
       @items = Item.find(params[:ids])
@@ -123,6 +121,7 @@ class ItemsController < ApplicationController
   def load_appearances
     appearance_params = params[:with_appearances_for]
     return {} if appearance_params.blank?
+    raise NotImplementedError if appearance_params[:alt_style_id].present?
 
     pet_type = Item::Search::Query.load_pet_type_by_color_and_species(
       appearance_params[:color_id], appearance_params[:species_id])
@@ -131,6 +130,7 @@ class ItemsController < ApplicationController
   
   def search_error(e)
     @items = []
+    @query = params[:q]
     respond_to do |format|
       format.html { flash.now[:alert] = e.message; render }
       format.json { render :json => {error: e.message} }
@@ -140,14 +140,7 @@ class ItemsController < ApplicationController
   def set_query
     q = params[:q]
     if q.is_a?(String)
-      begin
-        @query = Item::Search::Query.from_text(q, current_user)
-      rescue
-        # Set the query string for error handling messages, but let the error
-        # bubble up.
-        @query = params[:q]
-        raise
-      end
+      @query = Item::Search::Query.from_text(q, current_user)
     elsif q.is_a?(ActionController::Parameters)
       @query = Item::Search::Query.from_params(q, current_user)
     end

app/models/alt_style.rb

@@ -6,23 +6,11 @@ class AltStyle < ApplicationRecord
   has_many :swf_assets, through: :parent_swf_asset_relationships
   has_many :contributions, as: :contributed, inverse_of: :contributed
 
-  SERIES_ID_RANGES = {
-    nostalgic: (87249..87503)
-  }
-
   def name
     I18n.translate('pet_types.human_name', color_human_name: color.human_name,
                    species_human_name: species.human_name)
   end
 
-  def series_name
-    if SERIES_ID_RANGES[:nostalgic].include?(id)
-      "Nostalgic"
-    else
-      "???"
-    end
-  end
-
   def adjective_name
     "#{series_name} #{color.human_name}"
   end

app/models/item.rb

@@ -103,7 +103,11 @@ class Item < ApplicationRecord
     # bother looking into this, but one thing I notice is items with no assets
     # somehow would not match either scope in this impl (but LEFT JOIN would!)
     joins(:swf_assets).group(i[:id]).
-      having('FIND_IN_SET(?, GROUP_CONCAT(body_id)) = 0', body_id).
+      having(
+        "FIND_IN_SET(?, GROUP_CONCAT(body_id)) = 0 AND " +
+        "FIND_IN_SET(0, GROUP_CONCAT(body_id)) = 0",
+        body_id
+      ).
       distinct
   }
 

app/models/item/search/query.rb

@@ -27,75 +27,8 @@ class Item
           value = quoted_value || unquoted_value
           is_positive = (sign != '-')
           
-          case key
-          when 'name'
-            filters << (is_positive ?
-              Filter.name_includes(value) :
-              Filter.name_excludes(value))
-          when 'occupies'
-            filters << (is_positive ?
-              Filter.occupies(value) :
-              Filter.not_occupies(value))
-          when 'restricts'
-            filters << (is_positive ?
-              Filter.restricts(value) :
-              Filter.not_restricts(value))
-          when 'fits'
-            color_name, species_name = value.split("-")
-            pet_type = load_pet_type_by_name(color_name, species_name)
-            filters << (is_positive ?
-              Filter.fits(pet_type.body_id, color_name, species_name) :
-              Filter.not_fits(pet_type.body_id, color_name, species_name))
-          when 'species'
-            begin
-              species = Species.find_by_name!(value)
-              color = Color.find_by_name!('blue')
-              pet_type = PetType.where(color_id: color.id, species_id: species.id).first!
-            rescue ActiveRecord::RecordNotFound
-              message = I18n.translate('items.search.errors.not_found.species',
-                species_name: value.capitalize)
-              raise Item::Search::Error, message
-            end
-            filters << (is_positive ?
-              Filter.fits_species(pet_type.body_id, value) :
-              Filter.not_fits_species(pet_type.body_id, value))
-          when 'user'
-            if user.nil?
-              message = I18n.translate('items.search.errors.not_logged_in')
-              raise Item::Search::Error, message
-            end
-            case value
-            when 'owns'
-              filters << (is_positive ?
-                Filter.owned_by(user) :
-                Filter.not_owned_by(user))
-            when 'wants'
-              filters << (is_positive ?
-                Filter.wanted_by(user) :
-                Filter.not_wanted_by(user))
-            else
-              message = I18n.translate('items.search.errors.not_found.ownership',
-                keyword: value)
-              raise Item::Search::Error, message
-            end
-          when 'is'
-            case value
-            when 'nc'
-              filters << (is_positive ? Filter.is_nc : Filter.is_not_nc)
-            when 'np'
-              filters << (is_positive ? Filter.is_np : Filter.is_not_np)
-            when 'pb'
-              filters << (is_positive ? Filter.is_pb : Filter.is_not_pb)
-            else
-              message = I18n.translate('items.search.errors.not_found.label',
-                :label => "is:#{value}")
-              raise Item::Search::Error, message
-            end
-          else
-            message = I18n.translate('items.search.errors.not_found.label',
-              :label => key)
-            raise Item::Search::Error, message
-          end
+          filter = parse_text_filter(key, value, is_positive, user)
+          filters << filter if filter.present?
         end
         
         self.new(filters, user, text)
@@ -109,59 +42,126 @@ class Item
           value = filter_params[:value]
           is_positive = filter_params[:is_positive] != 'false'
 
-          case filter_params[:key]
-          when 'name'
-            filters << (is_positive ?
-              Filter.name_includes(value) :
-              Filter.name_excludes(value))
-          when 'is_nc'
-            filters << (is_positive ? Filter.is_nc : Filter.is_not_nc)
-          when 'is_pb'
-            filters << (is_positive ? Filter.is_pb : Filter.is_not_pb)
-          when 'is_np'
-            filters << (is_positive ? Filter.is_np : Filter.is_not_np)
-          when 'occupied_zone_set_name'
-            filters << (is_positive ? Filter.occupies(value) :
-              Filter.not_occupies(value))
-          when 'restricted_zone_set_name'
-            filters << (is_positive ?
-              Filter.restricts(value) :
-              Filter.not_restricts(value))
-          when 'fits'
-            raise NotImplementedError if value[:alt_style_id].present?
-            pet_type = load_pet_type_by_color_and_species(
-              value[:color_id], value[:species_id])
-            color = Color.find value[:color_id]
-            species = Species.find value[:species_id]
-            filters << (is_positive ?
-              Filter.fits(pet_type.body_id, color.name, species.name) :
-              Filter.not_fits(pet_type.body_id, color.name, species.name))
-          when 'user_closet_hanger_ownership'
-            case value
-            when 'true'
-              filters << (is_positive ?
-                Filter.owned_by(user) :
-                Filter.not_owned_by(user))
-            when 'false'
-              filters << (is_positive ?
-                Filter.wanted_by(user) :
-                Filter.not_wanted_by(user))
-            end
-          else
-            Rails.logger.warn "Ignoring unexpected search filter key: #{key}"
-          end
+          filter = parse_params_filter(key, value, is_positive, user)
+          filters << filter if filter.present?
         end
 
         self.new(filters, user)
       end
 
+      private
+
+      def self.parse_text_filter(key, value, is_positive, user)
+        case key
+        when 'name'
+          is_positive ?
+            Filter.name_includes(value) :
+            Filter.name_excludes(value)
+        when 'occupies'
+          is_positive ? Filter.occupies(value) : Filter.not_occupies(value)
+        when 'restricts'
+          is_positive ? Filter.restricts(value) : Filter.not_restricts(value)
+        when 'fits'
+          match = value.match(/^([^-]+)-([^-]+)$/)
+          if match.present?
+            color_name, species_name = match.captures
+            pet_type = load_pet_type_by_name(color_name, species_name)
+            return is_positive ?
+              Filter.fits_pet_type(pet_type, color_name:, species_name:) :
+              Filter.not_fits_pet_type(pet_type, color_name:, species_name:)
+          end
+          raise_search_error "not_found.fits_target", value: value
+        when 'species'
+          begin
+            species = Species.find_by_name!(value)
+            color = Color.find_by_name!('blue')
+            pet_type = PetType.where(color_id: color.id, species_id: species.id).first!
+          rescue ActiveRecord::RecordNotFound
+            raise_search_error "not_found.species",
+              species_name: value.capitalize
+          end
+          is_positive ?
+            Filter.fits_species(pet_type.body_id, value) :
+            Filter.not_fits_species(pet_type.body_id, value)
+        when 'user'
+          if user.nil?
+            raise_search_error "not_logged_in"
+          end
+          case value
+          when 'owns'
+            is_positive ? Filter.owned_by(user) : Filter.not_owned_by(user)
+          when 'wants'
+            is_positive ? Filter.wanted_by(user) : Filter.not_wanted_by(user)
+          else
+            raise_search_error "not_found.ownership", keyword: value
+          end
+        when 'is'
+          case value
+          when 'nc'
+            is_positive ? Filter.is_nc : Filter.is_not_nc
+          when 'np'
+            is_positive ? Filter.is_np : Filter.is_not_np
+          when 'pb'
+            is_positive ? Filter.is_pb : Filter.is_not_pb
+          else
+            raise_search_error "not_found.label", label: "is:#{value}"
+          end
+        else
+          raise_search_error "not_found.label", label: key
+        end
+      end
+
+      def self.parse_params_filter(key, value, is_positive, user)
+        case key
+        when 'name'
+          is_positive ?
+            Filter.name_includes(value) :
+            Filter.name_excludes(value)
+        when 'is_nc'
+          is_positive ? Filter.is_nc : Filter.is_not_nc
+        when 'is_pb'
+          is_positive ? Filter.is_pb : Filter.is_not_pb
+        when 'is_np'
+          is_positive ? Filter.is_np : Filter.is_not_np
+        when 'occupied_zone_set_name'
+          is_positive ? Filter.occupies(value) : Filter.not_occupies(value)
+        when 'restricted_zone_set_name'
+          is_positive ? Filter.restricts(value) : Filter.not_restricts(value)
+        when 'fits'
+          if value[:alt_style_id].present?
+            alt_style = load_alt_style_by_id(value[:alt_style_id])
+            is_positive ?
+              Filter.fits_alt_style(alt_style) :
+              Filter.fits_alt_style(alt_style)
+          else
+            pet_type = load_pet_type_by_color_and_species(
+              value[:color_id], value[:species_id])
+            is_positive ?
+              Filter.fits_pet_type(pet_type) :
+              Filter.not_fits_pet_type(pet_type)
+          end
+        when 'user_closet_hanger_ownership'
+          case value
+          when 'true'
+            is_positive ?
+              Filter.owned_by(user) :
+              Filter.not_owned_by(user)
+          when 'false'
+            is_positive ?
+              Filter.wanted_by(user) :
+              Filter.not_wanted_by(user)
+          end
+        else
+          Rails.logger.warn "Ignoring unexpected search filter key: #{key}"
+        end
+      end
+
       def self.load_pet_type_by_name(color_name, species_name)
         begin
           PetType.matching_name(color_name, species_name).first!
         rescue ActiveRecord::RecordNotFound
-          message = I18n.translate('items.search.errors.not_found.pet_type',
-            name1: color_name.capitalize, name2: species_name.capitalize)
-          raise Item::Search::Error, message
+          raise_search_error "not_found.pet_type",
+            name1: color_name.capitalize, name2: species_name.capitalize
         end
       end
 
@@ -171,11 +171,24 @@ class Item
         rescue ActiveRecord::RecordNotFound
           color_name = Color.find(color_id).name rescue "Color #{color_id}"
           species_name = Species.find(species_id).name rescue "Species #{species_id}"
-          message = I18n.translate('items.search.errors.not_found.pet_type',
-            name1: color_name.capitalize, name2: species_name.capitalize)
-          raise Item::Search::Error, message
+          raise_search_error "not_found.pet_type",
+            name1: color_name.capitalize, name2: species_name.capitalize
         end
       end
+
+      def self.load_alt_style_by_id(alt_style_id)
+        begin
+          AltStyle.find(alt_style_id)
+        rescue
+          raise_search_error "not_found.alt_style",
+            filter_text: "alt-style-#{alt_style_id}"
+        end
+      end
+
+      def self.raise_search_error(kind, ...)
+        raise Item::Search::Error,
+          I18n.translate("items.search.errors.#{kind}", ...)
+      end
     end
 
     class Error < Exception
@@ -227,16 +240,24 @@ class Item
         self.new Item.not_restricts(value), "-restricts:#{q value}"
       end
 
-      def self.fits(body_id, color_name, species_name)
-        # NOTE: Some color syntaxes are weird, like `fits:"polka dot-aisha"`!
-        value = "#{color_name}-#{species_name}".downcase
-        self.new Item.fits(body_id), "fits:#{q value}"
+      def self.fits_pet_type(pet_type, color_name: nil, species_name: nil)
+        value = pet_type_to_filter_text(pet_type, color_name:, species_name:)
+        self.new Item.fits(pet_type.body_id), "fits:#{q value}"
       end
 
-      def self.not_fits(body_id, color_name, species_name)
-        # NOTE: Some color syntaxes are weird, like `fits:"polka dot-aisha"`!
-        value = "#{color_name}-#{species_name}".downcase
-        self.new Item.not_fits(body_id), "-fits:#{q value}"
+      def self.not_fits_pet_type(pet_type, color_name: nil, species_name: nil)
+        value = pet_type_to_filter_text(pet_type, color_name:, species_name:)
+        self.new Item.not_fits(pet_type.body_id), "-fits:#{q value}"
+      end
+
+      def self.fits_alt_style(alt_style)
+        value = alt_style_to_filter_text(alt_style)
+        self.new Item.fits(alt_style.body_id), "fits:#{q value}"
+      end
+
+      def self.not_fits_alt_style(alt_style)
+        value = alt_style_to_filter_text(alt_style)
+        self.new Item.not_fits(alt_style.body_id), "-fits:#{q value}"
       end
 
       def self.fits_species(body_id, species_name)
@@ -293,6 +314,20 @@ class Item
       def self.q(value)
         /\s/.match(value) ? '"' + value + '"' : value
       end
+
+      def self.pet_type_to_filter_text(pet_type, color_name: nil, species_name: nil)
+        # Load the color & species name if needed, or use them from the params
+        # if already known (e.g. from parsing a "fits:blue-acara" text query).
+        color_name ||= pet_type.color.name
+        species_name ||= pet_type.species.name
+
+        # NOTE: Some color syntaxes are weird, like `fits:"polka dot-aisha"`!
+        "#{color_name}-#{species_name}".downcase
+      end
+
+      def self.alt_style_to_filter_text(alt_style)
+        "alt-style-#{alt_style.id}"
+      end
     end
   end
 end

config/locales/en.yml

@@ -358,7 +358,11 @@ en:
             user:wants?
           pet_type: We have no record of the %{name1} %{name2}.
             It is spelled correctly?
+          alt_style: We have no record of the "%{filter_text}" alt style. Is it
+            spelled correctly?
           pet_type_id: We have no record of pet type %{id}. Weird.
+          fits_target: I'm not sure what "fits:%{value}" means. You can
+            use "fits:blue-acara", "fits:nostalgic-faerie-draik", or similar!
         not_logged_in: The "user" filters are only available if you're logged in.
       flag_keywords:
         is: is

db/migrate/20240227231815_add_series_name_to_alt_styles.rb

@@ -0,0 +1,18 @@
+class AddSeriesNameToAltStyles < ActiveRecord::Migration[7.1]
+  def change
+    add_column :alt_styles, :series_name, :string, null: false,
+      default: "<New?>"
+
+    reversible do |direction|
+      direction.up do
+        # These IDs are determined by Neopets.com, so referencing them like
+        # this should be relatively stable! Backfill the series name
+        # "Nostalgic" for all alt styles in the Nostalgic ID range. (At time
+        # of writing, *all* alt styles are Nostalgic, but I'm writing it like
+        # this for clarity and future-proofing!)
+        AltStyle.where("id >= 87249 AND id <= 87503").update_all(
+          series_name: "Nostalgic")
+      end
+    end
+  end
+end

db/schema.rb

@@ -10,13 +10,14 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_02_25_231346) do
+ActiveRecord::Schema[7.1].define(version: 2024_02_27_231815) 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
     t.integer "body_id", null: false
     t.datetime "created_at", precision: nil, null: false
     t.datetime "updated_at", precision: nil, null: false
+    t.string "series_name", default: "<New?>", null: false
     t.index ["color_id"], name: "index_alt_styles_on_color_id"
     t.index ["species_id"], name: "index_alt_styles_on_species_id"
   end
@@ -115,6 +116,20 @@ ActiveRecord::Schema[7.1].define(version: 2024_02_25_231346) do
     t.index ["outfit_id", "is_worn"], name: "index_item_outfit_relationships_on_outfit_id_and_is_worn"
   end
 
+  create_table "item_translations", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
+    t.integer "item_id"
+    t.string "locale"
+    t.string "name"
+    t.text "description"
+    t.string "rarity"
+    t.datetime "created_at", precision: nil
+    t.datetime "updated_at", precision: nil
+    t.index ["item_id", "locale"], name: "index_item_translations_on_item_id_and_locale"
+    t.index ["item_id"], name: "index_item_translations_on_item_id"
+    t.index ["locale"], name: "index_item_translations_on_locale"
+    t.index ["name"], name: "index_item_translations_name"
+  end
+
   create_table "items", id: :integer, charset: "utf8mb3", collation: "utf8mb3_unicode_ci", force: :cascade do |t|
     t.text "zones_restrict", null: false
     t.text "thumbnail_url", size: :medium, null: false