Add Indexed#find_by! to reduce repetition (#590)

Author: danielpgross

dev/lib/product_taxonomy/commands/add_attributes_to_categories_command.rb

@@ -18,20 +18,8 @@ def execute
     private
 
     def add_attributes_to_categories!
-      @attributes = attribute_friendly_ids.map do |friendly_id|
-        attribute = Attribute.find_by(friendly_id:)
-        next attribute if attribute
-
-        raise "Attribute with friendly ID `#{friendly_id}` not found"
-      end
-
-      @categories = category_ids.map do |id|
-        category = Category.find_by(id:)
-        next category if category
-
-        raise "Category with ID `#{id}` not found"
-      end
-
+      @attributes = attribute_friendly_ids.map { |friendly_id| Attribute.find_by!(friendly_id:) }
+      @categories = category_ids.map { |id| Category.find_by!(id:) }
       @categories = @categories.flat_map(&:descendants_and_self) if @include_descendants
 
       @categories.each do |category|

dev/lib/product_taxonomy/commands/add_category_command.rb

@@ -18,9 +18,7 @@ def execute
     private
 
     def create_category!
-      parent = Category.find_by(id: @parent_id)
-      raise "Parent category `#{@parent_id}` not found" if parent.nil?
-
+      parent = Category.find_by!(id: @parent_id)
       @new_category = Category.new(id: @id || parent.next_child_id, name: @name, parent:)
       raise "Failed to create category: #{@new_category.errors.full_messages.to_sentence}" unless @new_category.valid?
 

dev/lib/product_taxonomy/commands/add_value_command.rb

@@ -17,8 +17,7 @@ def execute
     private
 
     def create_value!
-      @attribute = Attribute.find_by(friendly_id: @attribute_friendly_id)
-      raise "Attribute `#{@attribute_friendly_id}` not found" if @attribute.nil?
+      @attribute = Attribute.find_by!(friendly_id: @attribute_friendly_id)
       if @attribute.extended?
         raise "Attribute `#{@attribute.name}` is an extended attribute, please use a primary attribute instead"
       end

dev/lib/product_taxonomy/commands/dump_categories_command.rb

@@ -12,8 +12,7 @@ def execute
       logger.info("Dumping #{@verticals.size} verticals")
 
       @verticals.each do |vertical_id|
-        vertical_root = Category.find_by(id: vertical_id)
-        raise "Vertical not found: #{vertical_id}" unless vertical_root
+        vertical_root = Category.find_by!(id: vertical_id)
         raise "Category #{vertical_id} is not a vertical" unless vertical_root.root?
 
         logger.info("Updating `#{vertical_root.name}`...")

dev/lib/product_taxonomy/models/mixins/indexed.rb

@@ -3,6 +3,8 @@
 module ProductTaxonomy
   # Mixin providing indexing for a model, using hashes to support fast uniqueness checks and lookups by field value.
   module Indexed
+    NotFoundError = Class.new(StandardError)
+
     class << self
       # Keep track of which model class used "extend Indexed" so that the model can be subclassed while still storing
       # model objects in a shared index on the superclass.
@@ -67,6 +69,20 @@ def find_by(**conditions)
       hashed_models[field][value]
     end
 
+    # Find a model by field value. Returns the first matching record or raises an error if not found. Only works for
+    # fields marked unique.
+    #
+    # @param conditions [Hash] Hash of field-value pairs to search by
+    # @return [Object] The matching model
+    def find_by!(**conditions)
+      record = find_by(**conditions)
+      return record if record
+
+      field, value = conditions.first
+      field = field.to_s.humanize(capitalize: false, keep_id_suffix: true)
+      raise NotFoundError, "#{self.name.demodulize} with #{field} \"#{value}\" not found"
+    end
+
     # Get the hash of models indexed by a given field. Only works for fields marked unique.
     #
     # @param field [Symbol] The field to get the hash for.

dev/test/commands/add_attributes_to_categories_command_test.rb

@@ -137,7 +137,7 @@ class AddAttributesToCategoriesCommandTest < TestCase
     end
 
     test "execute raises error when attribute is not found" do
-      assert_raises(RuntimeError) do
+      assert_raises(Indexed::NotFoundError) do
         AddAttributesToCategoriesCommand.new(
           attribute_friendly_ids: "nonexistent",
           category_ids: "aa-1",
@@ -147,7 +147,7 @@ class AddAttributesToCategoriesCommandTest < TestCase
     end
 
     test "execute raises error when category is not found" do
-      assert_raises(RuntimeError) do
+      assert_raises(Indexed::NotFoundError) do
         AddAttributesToCategoriesCommand.new(
           attribute_friendly_ids: "color",
           category_ids: "nonexistent",

dev/test/commands/add_category_command_test.rb

@@ -44,7 +44,7 @@ class AddCategoryCommandTest < TestCase
     end
 
     test "execute raises error when parent category not found" do
-      assert_raises(RuntimeError) do
+      assert_raises(Indexed::NotFoundError) do
         AddCategoryCommand.new(name: "New Category", parent_id: "nonexistent").execute
       end
     end

dev/test/commands/add_value_command_test.rb

@@ -57,7 +57,7 @@ class AddValueCommandTest < TestCase
     end
 
     test "execute raises error when attribute not found" do
-      assert_raises(RuntimeError) do
+      assert_raises(Indexed::NotFoundError) do
         AddValueCommand.new(name: "Blue", attribute_friendly_id: "nonexistent").execute
       end
     end

dev/test/commands/dump_categories_command_test.rb

@@ -54,7 +54,7 @@ class DumpCategoriesCommandTest < TestCase
       Category.stubs(:find_by).with(id: "nonexistent").returns(nil)
 
       command = DumpCategoriesCommand.new(verticals: ["nonexistent"])
-      assert_raises(RuntimeError) do
+      assert_raises(Indexed::NotFoundError) do
         command.execute
       end
     end

dev/test/models/mixins/indexed_test.rb

@@ -94,6 +94,16 @@ class SubModel < Model
       assert_raises(ArgumentError) { Model.find_by(name: "test") }
     end
 
+    test "find_by! returns the model with the specified field value" do
+      assert_equal @model, Model.find_by!(id: 1)
+    end
+
+    test "find_by! raises NotFoundError if the model with the specified field value is not in the index" do
+      assert_raises(ProductTaxonomy::Indexed::NotFoundError) do
+        Model.find_by!(id: 2)
+      end
+    end
+
     test "all returns all models in the index" do
       assert_equal [@model], Model.all
     end