From 217d25edab7094ea764e4ec2b884c9b407a0e232 Mon Sep 17 00:00:00 2001 From: Emi Matchu Date: Fri, 15 Nov 2024 19:56:07 -0800 Subject: [PATCH] Handle new colors/species in the Rainbow Pool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Oh right, yeah, we like to do things gracefully around here when there's no corresponding color/species record yet! Paying more attention to this, I'm thinking like… it could be a cool idea to, in modeling, *create* the new color/species record, and just not have all the attributes filled in yet? Especially now that we're less dependent on attributes like `standard` to be set for correct functioning. But for now, we follow the same strategy we do elsewhere in the app: a pet type can have `color_id` and `species_id` that don't correspond to a real record, and we cover over that smoothly. --- app/controllers/pet_states_controller.rb | 2 +- app/controllers/pet_types_controller.rb | 4 +-- app/models/color.rb | 8 +++++ app/models/pet_type.rb | 15 ++++++--- app/models/species.rb | 8 +++++ app/views/pet_states/edit.html.haml | 8 ++--- app/views/pet_types/show.html.haml | 8 ++--- spec/fixtures/colors.yml | 3 ++ spec/fixtures/pet_types.yml | 19 +++++++++++ spec/models/color_spec.rb | 38 ++++++++++++++++++++++ spec/models/pet_type_spec.rb | 41 ++++++++++++++++++++++++ spec/models/species_spec.rb | 34 ++++++++++++++++++++ 12 files changed, 171 insertions(+), 17 deletions(-) create mode 100644 spec/fixtures/pet_types.yml create mode 100644 spec/models/color_spec.rb create mode 100644 spec/models/pet_type_spec.rb create mode 100644 spec/models/species_spec.rb diff --git a/app/controllers/pet_states_controller.rb b/app/controllers/pet_states_controller.rb index d6cc3193..090640ac 100644 --- a/app/controllers/pet_states_controller.rb +++ b/app/controllers/pet_states_controller.rb @@ -17,7 +17,7 @@ class PetStatesController < ApplicationController protected def find_pet_state - @pet_type = PetType.matching_name_param(params[:pet_type_name]).first! + @pet_type = PetType.find_by_param!(params[:pet_type_name]) @pet_state = @pet_type.pet_states.find(params[:id]) end diff --git a/app/controllers/pet_types_controller.rb b/app/controllers/pet_types_controller.rb index 392ba4fe..8fa2f524 100644 --- a/app/controllers/pet_types_controller.rb +++ b/app/controllers/pet_types_controller.rb @@ -70,9 +70,7 @@ class PetTypesController < ApplicationController color_id: params[:color_id], ) elsif params[:name] - color_name, _, species_name = params[:name].rpartition("-") - raise ActiveRecord::RecordNotFound if species_name.blank? - PetType.matching_name(color_name, species_name).first! + PetType.find_by_param!(params[:name]) else raise "expected params: species_id and color_id, or name" end diff --git a/app/models/color.rb b/app/models/color.rb index 9211aac9..dc647e99 100644 --- a/app/models/color.rb +++ b/app/models/color.rb @@ -21,6 +21,10 @@ class Color < ApplicationRecord end end + def to_param + name? ? human_name : id.to_s + end + def example_pet_type(preferred_species: nil) preferred_species ||= Species.first pet_types.order([Arel.sql("species_id = ? DESC"), preferred_species.id], @@ -36,4 +40,8 @@ class Color < ApplicationRecord nil end end + + def self.param_to_id(param) + param.match?(/\A\d+\Z/) ? param.to_i : find_by_name!(param).id + end end diff --git a/app/models/pet_type.rb b/app/models/pet_type.rb index a73f510d..1ef5e884 100644 --- a/app/models/pet_type.rb +++ b/app/models/pet_type.rb @@ -15,10 +15,6 @@ class PetType < ApplicationRecord species = Species.find_by_name!(species_name) where(color_id: color.id, species_id: species.id) } - scope :matching_name_param, ->(name_param) { - color_name, _, species_name = name_param.rpartition("-") - matching_name(color_name, species_name) - } scope :preferring_species, ->(species_id) { joins(:species).order([Arel.sql("species_id = ? DESC"), species_id]) } @@ -116,7 +112,7 @@ class PetType < ApplicationRecord end def to_param - "#{color.human_name}-#{species.human_name}" + "#{possibly_new_color.to_param}-#{possibly_new_species.to_param}" end def fully_labeled? @@ -136,6 +132,15 @@ class PetType < ApplicationRecord pet_states.count { |ps| ps.pose == "UNKNOWN" } end + def self.find_by_param!(param) + raise ActiveRecord::RecordNotFound unless param.include?("-") + color_param, _, species_param = param.rpartition("-") + where( + color_id: Color.param_to_id(color_param), + species_id: Species.param_to_id(species_param), + ).first! + end + def self.basic_body_ids PetType.basic.distinct.pluck(:body_id) end diff --git a/app/models/species.rb b/app/models/species.rb index 9c5b1efe..8f43f05f 100644 --- a/app/models/species.rb +++ b/app/models/species.rb @@ -16,6 +16,10 @@ class Species < ApplicationRecord end end + def to_param + name? ? human_name : id.to_s + end + # Given a list of body IDs, return a hash from body ID to Species. # (We assume that each body ID belongs to just one species; if not, which # species we return for that body ID is undefined.) @@ -26,4 +30,8 @@ class Species < ApplicationRecord to_h { |s| [s.id, s] } species_ids_by_body_id.transform_values { |id| species_by_id[id] } end + + def self.param_to_id(param) + param.match?(/\A\d+\Z/) ? param.to_i : find_by_name!(param).id + end end diff --git a/app/views/pet_states/edit.html.haml b/app/views/pet_states/edit.html.haml index 0f2d1b1a..4c1432c5 100644 --- a/app/views/pet_states/edit.html.haml +++ b/app/views/pet_states/edit.html.haml @@ -5,11 +5,11 @@ %li = link_to "Rainbow Pool", pet_types_path %li - = link_to @pet_type.color.human_name, - pet_types_path(color: @pet_type.color.human_name) + = link_to @pet_type.possibly_new_color.human_name, + pet_types_path(color: @pet_type.possibly_new_color.human_name) %li{"data-relation-to-prev": "sibling"} - = link_to @pet_type.species.human_name, - pet_types_path(species: @pet_type.species.human_name) + = link_to @pet_type.possibly_new_species.human_name, + pet_types_path(species: @pet_type.possibly_new_species.human_name) %li = link_to "Appearances", @pet_type %li diff --git a/app/views/pet_types/show.html.haml b/app/views/pet_types/show.html.haml index 3ba0600a..4215c16d 100644 --- a/app/views/pet_types/show.html.haml +++ b/app/views/pet_types/show.html.haml @@ -5,11 +5,11 @@ %li = link_to "Rainbow Pool", pet_types_path %li - = link_to @pet_type.color.human_name, - pet_types_path(color: @pet_type.color.human_name) + = link_to @pet_type.possibly_new_color.human_name, + pet_types_path(color: @pet_type.possibly_new_color.human_name) %li{"data-relation-to-prev": "sibling"} - = link_to @pet_type.species.human_name, - pet_types_path(species: @pet_type.species.human_name) + = link_to @pet_type.possibly_new_species.human_name, + pet_types_path(species: @pet_type.possibly_new_species.human_name) %li Appearances diff --git a/spec/fixtures/colors.yml b/spec/fixtures/colors.yml index 21580a06..2404832d 100644 --- a/spec/fixtures/colors.yml +++ b/spec/fixtures/colors.yml @@ -10,3 +10,6 @@ robot: striped: id: 77 name: striped +swamp_gas: + id: 93 + name: "swamp gas" diff --git a/spec/fixtures/pet_types.yml b/spec/fixtures/pet_types.yml new file mode 100644 index 00000000..4eafc4b8 --- /dev/null +++ b/spec/fixtures/pet_types.yml @@ -0,0 +1,19 @@ +blue_acara: + color_id: 8 + species_id: 1 + body_id: 123 + +newcolor_acara: + color_id: 123 + species_id: 1 + body_id: 123 + +blue_newspecies: + color_id: 8 + species_id: 456 + body_id: 123 + +newcolor_newspecies: + color_id: 123 + species_id: 456 + body_id: 123 diff --git a/spec/models/color_spec.rb b/spec/models/color_spec.rb new file mode 100644 index 00000000..371bd11a --- /dev/null +++ b/spec/models/color_spec.rb @@ -0,0 +1,38 @@ +require 'rails_helper' + +RSpec.describe Color do + fixtures :colors + + describe '#to_param' do + it("uses name when possible") do + expect(colors(:blue).to_param).to eq "Blue" + end + + it("uses spaces for multi-word names") do + expect(colors(:swamp_gas).to_param).to eq "Swamp Gas" + end + + it("uses IDs for new colors") do + expect(Color.new(id: 12345).to_param).to eq "12345" + end + end + + describe ".param_to_id" do + it("looks up by name") do + expect(Color.param_to_id("blue")).to eq colors(:blue).id + end + + it("is case-insensitive for name") do + expect(Color.param_to_id("bLUe")).to eq colors(:blue).id + end + + it("returns ID when the param is just a number, even if it doesn't exist") do + expect(Color.param_to_id("123456")).to eq 123456 + end + + it("raises RecordNotFound if no name matches") do + expect { Color.param_to_id("nonexistant") }. + to raise_error ActiveRecord::RecordNotFound + end + end +end diff --git a/spec/models/pet_type_spec.rb b/spec/models/pet_type_spec.rb new file mode 100644 index 00000000..dbd3b3a6 --- /dev/null +++ b/spec/models/pet_type_spec.rb @@ -0,0 +1,41 @@ +require 'rails_helper' + +RSpec.describe PetType do + fixtures :colors, :species, :pet_types + + describe '#to_param' do + it('uses color and species name when possible ("Blue-Acara")') do + expect(pet_types(:blue_acara).to_param).to eq "Blue-Acara" + end + + it('uses color ID for new colors (123-Acara)') do + expect(pet_types(:newcolor_acara).to_param).to eq "123-Acara" + end + + it('uses species ID for new colors (Blue-456)') do + expect(pet_types(:blue_newspecies).to_param).to eq "Blue-456" + end + + it('uses color ID and species ID when both are new (123-456)') do + expect(pet_types(:newcolor_newspecies).to_param).to eq "123-456" + end + end + + describe ".find_by_param!" do + it('looks up by species and color name ("Blue-Acara")') do + expect(PetType.find_by_param!("Blue-Acara")).to eq pet_types(:blue_acara) + end + + it('looks up by color ID for new colors ("123-Acara")') do + expect(PetType.find_by_param!("123-Acara")).to eq pet_types(:newcolor_acara) + end + + it('looks up by species ID for new species ("Blue-456")') do + expect(PetType.find_by_param!("Blue-456")).to eq pet_types(:blue_newspecies) + end + + it('looks up by color ID and species ID when both are new ("123-456")') do + expect(PetType.find_by_param!("123-456")).to eq pet_types(:newcolor_newspecies) + end + end +end diff --git a/spec/models/species_spec.rb b/spec/models/species_spec.rb new file mode 100644 index 00000000..5f014357 --- /dev/null +++ b/spec/models/species_spec.rb @@ -0,0 +1,34 @@ +require 'rails_helper' + +RSpec.describe Species do + fixtures :species + + describe '#to_param' do + it("uses name when possible") do + expect(species(:acara).to_param).to eq "Acara" + end + + it("uses IDs for new species") do + expect(Species.new(id: 12345).to_param).to eq "12345" + end + end + + describe ".param_to_id" do + it("looks up by name") do + expect(Species.param_to_id("acara")).to eq species(:acara).id + end + + it("is case-insensitive for name") do + expect(Species.param_to_id("aCaRa")).to eq species(:acara).id + end + + it("returns ID when the param is just a number, even if no record exists") do + expect(Species.param_to_id("123456")).to eq 123456 + end + + it("raises RecordNotFound if no name matches") do + expect { Species.param_to_id("nonexistant") }. + to raise_error ActiveRecord::RecordNotFound + end + end +end