Merge pull request #114 from Shopify/revert-113-add-minitest-hooks

Revert "Add minitest hooks and make seeding fast"

Gemfile

@@ -12,5 +12,4 @@ gem "rubocop-shopify", require: false
 
 group :test do
   gem "minitest", "~> 5.21"
-  gem "minitest-hooks", "~> 1.5"
 end

Gemfile.lock

@@ -70,8 +70,6 @@ GEM
       rb-inotify (~> 0.9, >= 0.9.10)
     mercenary (0.4.0)
     minitest (5.21.2)
-    minitest-hooks (1.5.1)
-      minitest (> 5.3)
     mutex_m (0.2.0)
     parallel (1.24.0)
     parser (3.3.0.5)
@@ -130,7 +128,6 @@ DEPENDENCIES
   byebug
   jekyll (~> 4.3)
   minitest (~> 5.21)
-  minitest-hooks (~> 1.5)
   rake (~> 13.1)
   rubocop-shopify
   sqlite3 (~> 1.7)

app/models/category.rb

@@ -7,14 +7,7 @@ class Category < ApplicationRecord
 
   has_many :children, class_name: "Category", inverse_of: :parent
   belongs_to :parent, class_name: "Category", optional: true
-
-  has_and_belongs_to_many :properties,
-    join_table: :categories_properties,
-    foreign_key: :category_id,
-    association_foreign_key: :property_friendly_id
-  def property_friendly_ids=(ids)
-    self.properties = Property.where(friendly_id: ids)
-  end
+  has_and_belongs_to_many :properties
 
   validates :id,
     presence: { strict: true },

app/models/property.rb

@@ -3,13 +3,8 @@
 class Property < ApplicationRecord
   default_scope { order(:name) }
 
-  has_and_belongs_to_many :categories,
-    join_table: :categories_properties,
-    foreign_key: :property_friendly_id,
-    association_foreign_key: :category_id
-
-  has_and_belongs_to_many :property_values,
-    join_table: :properties_property_values
+  has_and_belongs_to_many :categories
+  has_and_belongs_to_many :property_values
 
   validates :name, presence: true
 

app/models/property_value.rb

@@ -3,8 +3,7 @@
 class PropertyValue < ApplicationRecord
   default_scope { order(Arel.sql("CASE WHEN name = 'Other' THEN 1 ELSE 0 END, name")) }
 
-  has_and_belongs_to_many :properties,
-    join_table: :properties_property_values
+  has_and_belongs_to_many :properties
 
   validates :name, presence: true
 

app/serializers/data/category_serializer.rb

@@ -3,10 +3,6 @@
 module Serializers
   module Data
     class CategorySerializer < ObjectSerializer
-      class << self
-        delegate(:deserialize_for_insert_all, :deserialize_for_join_insert_all, to: :instance)
-      end
-
       def serialize(category)
         {
           "id" => category.id,
@@ -17,34 +13,15 @@ def serialize(category)
       end
 
       def deserialize(hash)
-        Category.new(**attributes_from(hash)).tap do |category|
-          category.child_ids = hash["children"] if hash["children"]
-          category.property_friendly_ids = hash["attributes"] if hash["attributes"]
-        end
-      end
-
-      def deserialize_for_insert_all(array)
-        array.map { attributes_from(_1) }
-      end
+        id = hash["id"].downcase
+        parent_id = id.split("-")[0...-1].join("-").presence
+        name = hash["name"]
 
-      def deserialize_for_join_insert_all(array)
-        array.flat_map do |hash|
-          category_id = hash["id"].downcase
-          hash["attributes"].map do |property_friendly_id|
-            { category_id:, property_friendly_id: }
-          end
+        Category.new(id:, parent_id:, name:).tap do |category|
+          category.child_ids = hash["children"] if hash["children"]
+          category.property_ids = Property.where(friendly_id: hash["attributes"]).pluck(:id) if hash["attributes"]
         end
       end
-
-      private
-
-      def attributes_from(hash)
-        {
-          id: hash["id"].downcase,
-          parent_id: hash["id"].split("-")[0...-1].join("-").presence,
-          name: hash["name"],
-        }
-      end
     end
   end
 end

app/serializers/data/property_serializer.rb

@@ -3,10 +3,6 @@
 module Serializers
   module Data
     class PropertySerializer < ObjectSerializer
-      class << self
-        delegate(:deserialize_for_insert_all, :deserialize_for_join_insert_all, to: :instance)
-      end
-
       def serialize(property)
         {
           "id" => property.id,
@@ -24,27 +20,6 @@ def deserialize(hash)
           property_value_ids: hash["values"].map { _1["id"] },
         )
       end
-
-      def deserialize_for_insert_all(array)
-        array.map do |hash|
-          {
-            id: hash["id"],
-            friendly_id: hash["friendly_id"],
-            name: hash["name"],
-          }
-        end
-      end
-
-      def deserialize_for_join_insert_all(array)
-        array.flat_map do |hash|
-          hash["values"].map do |value_hash|
-            {
-              property_id: hash["id"],
-              property_value_id: value_hash["id"],
-            }
-          end
-        end
-      end
     end
   end
 end

app/serializers/data/property_value_serializer.rb

@@ -3,10 +3,6 @@
 module Serializers
   module Data
     class PropertyValueSerializer < ObjectSerializer
-      class << self
-        delegate(:deserialize_for_insert_all, to: :instance)
-      end
-
       def serialize(property_value)
         {
           "id" => property_value.id,
@@ -15,20 +11,10 @@ def serialize(property_value)
       end
 
       def deserialize(hash)
-        PropertyValue.new(**attributes_from(hash))
-      end
-
-      def deserialize_for_insert_all(array)
-        array.map { attributes_from(_1) }
-      end
-
-      private
-
-      def attributes_from(hash)
-        {
+        PropertyValue.new(
           id: hash["id"],
           name: hash["name"],
-        }
+        )
       end
     end
   end

app/serializers/object_serializer.rb

@@ -5,7 +5,7 @@ class ObjectSerializer
     include Singleton
 
     class << self
-      delegate(:serialize, :deserialize, to: :instance)
+      delegate :serialize, :deserialize, to: :instance
     end
 
     def serialize(object)

application.rb

@@ -12,8 +12,6 @@
 require_relative "app/models/category"
 require_relative "app/models/property"
 require_relative "app/models/property_value"
-require_relative "app/models/categories_property"
-require_relative "app/models/properties_property_value"
 
 require_relative "app/serializers/object_serializer"
 require_relative "app/serializers/data/category_serializer"

db/schema.rb

@@ -19,10 +19,10 @@
 
   create_table :categories_properties, id: false, force: :cascade do |t|
     t.string(:category_id, null: false)
-    t.string(:property_friendly_id, null: false)
+    t.integer(:property_id, null: false)
 
-    t.index([:category_id, :property_friendly_id], unique: true)
-    t.index(:property_friendly_id)
+    t.index([:category_id, :property_id], unique: true)
+    t.index(:property_id)
   end
   create_table :properties_property_values, id: false, force: :cascade do |t|
     t.integer(:property_id, null: false)

db/seed.rb

@@ -7,37 +7,39 @@ class Seed
     class << self
       def attributes_from(data)
         puts "Importing values"
-        raw_values = data.flat_map { _1.fetch("values") }
-        values = Serializers::Data::PropertyValueSerializer.deserialize_for_insert_all(raw_values)
-        PropertyValue.insert_all(values)
+        data.each do |property_json|
+          property_json["values"].each do |property_json|
+            property_value = Serializers::Data::PropertyValueSerializer.deserialize(property_json)
+            by_id = PropertyValue.find_by(id: property_value.id)
+
+            if by_id.nil?
+              property_value.save!
+            elsif by_id.name != property_value.name
+              puts "  ⨯ Failed to import value: #{property_value.name} <#{property_value.id}> already exists as " \
+                "#{by_id.name} <#{by_id.id}>"
+            end
+          end
+        end
         puts "✓ Imported #{PropertyValue.count} values"
 
         puts "Importing properties"
-        properties = Serializers::Data::PropertySerializer.deserialize_for_insert_all(data)
-        Property.insert_all(properties)
+        data.each do |json|
+          Serializers::Data::PropertySerializer.deserialize(json).save!
+        end
         puts "✓ Imported #{Property.count} properties"
-
-        puts "Importing property ↔ value relationships"
-        joins = Serializers::Data::PropertySerializer.deserialize_for_join_insert_all(data)
-        PropertiesPropertyValue.insert_all(joins)
-        puts "✓ Imported #{PropertiesPropertyValue.count} property ↔ value relationships"
       end
 
       def categories_from(data)
         puts "Importing #{data.count} category verticals"
         data.each do |vertical_json|
           puts "  → #{vertical_json.first.fetch("name")}"
-          categories = Serializers::Data::CategorySerializer.deserialize_for_insert_all(vertical_json)
-          Category.insert_all(categories)
+          vertical_json.each do |category_json|
+            unless Serializers::Data::CategorySerializer.deserialize(category_json.except("children")).save
+              puts "  ⨯ Failed to import category: #{category_json["name"]} <#{category_json["id"]}>"
+            end
+          end
         end
         puts "✓ Imported #{Category.count} categories"
-
-        puts "Importing category relationships"
-        data.each do |vertical_json|
-          joins = Serializers::Data::CategorySerializer.deserialize_for_join_insert_all(vertical_json)
-          CategoriesProperty.insert_all(joins)
-        end
-        puts "✓ Imported #{CategoriesProperty.count} category ↔ property relationships"
       end
     end
   end

test/integration/all_data_files_import_test.rb

@@ -3,68 +3,31 @@
 require_relative "../test_helper"
 
 class AllDataFilesImportTest < ActiveSupport::TestCase
-  include Minitest::Hooks
-
-  def before_all
-    @raw_attributes_data = YAML.load_file("#{Application.root}/data/attributes/attributes.yml")
-    DB::Seed.attributes_from(@raw_attributes_data)
-
-    # this will be replaced by values.yml
-    @unique_raw_values_data = @raw_attributes_data
-      .flat_map { _1.fetch("values") }
-      .uniq { _1.fetch("id") }
-
-    # Categories are only successfully parseable if attributes are already present
-    category_files = Dir.glob("#{Application.root}/data/categories/*.yml")
-    @raw_verticals_data = category_files.map { YAML.load_file(_1) }
-    DB::Seed.categories_from(@raw_verticals_data)
-  end
-
-  test "AttributeValues are correctly imported from attributes.yml" do
-    assert_equal @unique_raw_values_data.size, PropertyValue.count
+  def teardown
+    Category.destroy_all
+    Property.destroy_all
+    PropertyValue.destroy_all
   end
 
-  test "AttributeValues are consistent with attributes.yml" do
-    @unique_raw_values_data.each do |raw_value|
-      deserialized_value = Serializers::Data::PropertyValueSerializer.deserialize(raw_value)
-      real_value = PropertyValue.find(raw_value.fetch("id"))
-
-      assert_equal deserialized_value, real_value
-    end
-  end
-
-  test "AttributeValues are all valid" do
-    PropertyValue.all.each do |value|
-      assert_predicate value, :valid?
-    end
-  end
+  test "DB::Seed imports everything from all data/ files" do
+    raw_attributes_data = YAML.load_file("#{Application.root}/data/attributes/attributes.yml")
+    DB::Seed.attributes_from(raw_attributes_data)
 
-  test "Attributes are correctly imported from attributes.yml" do
-    assert_equal @raw_attributes_data.size, Property.count
-  end
-
-  test "Attributes are consistent with attributes.yml" do
-    @raw_attributes_data.each do |raw_attribute|
+    assert_equal raw_attributes_data.size, Property.count
+    raw_attributes_data.each do |raw_attribute|
       deserialized_attribute = Serializers::Data::PropertySerializer.deserialize(raw_attribute)
       real_attribute = Property.find(raw_attribute.fetch("id"))
 
       assert_equal deserialized_attribute, real_attribute
     end
-  end
-
-  test "Attributes are all valid" do
-    Property.all.each do |attribute|
-      assert_predicate attribute, :valid?
-    end
-  end
 
-  test "Categories are fully imported from categories/*.yml" do
-    assert_equal @raw_verticals_data.size, Category.verticals.count
-    assert_equal @raw_verticals_data.map(&:size).sum, Category.count
-  end
+    category_files = Dir.glob("#{Application.root}/data/categories/*.yml")
+    raw_verticals_data = category_files.map { YAML.load_file(_1) }
+    DB::Seed.categories_from(raw_verticals_data)
 
-  test "Categories are consistent with categories/*.yml" do
-    @raw_verticals_data.flatten.each do |raw_category|
+    assert_equal raw_verticals_data.size, Category.verticals.count
+    assert_equal raw_verticals_data.map(&:size).sum, Category.count
+    raw_verticals_data.flatten.each do |raw_category|
       deserialized_category = Serializers::Data::CategorySerializer.deserialize(raw_category)
       real_category = Category.find(raw_category.fetch("id"))
 
@@ -73,10 +36,4 @@ def before_all
       assert_equal deserialized_category.children, real_category.children
     end
   end
-
-  test "Categories are all valid" do
-    Category.all.each do |category|
-      assert_predicate category, :valid?
-    end
-  end
 end