Simplify ingredient creation

app/decorators/alchemy/element_editor.rb

@@ -26,7 +26,7 @@ def contents
     # @return Array<Alchemy::IngredientEditor>
     def ingredients
       element.definition.fetch(:ingredients, []).map do |ingredient|
-        Alchemy::IngredientEditor.new(find_or_create_ingredient(ingredient[:role]))
+        Alchemy::IngredientEditor.new(find_or_create_ingredient(ingredient))
       end
     end
 
@@ -121,9 +121,12 @@ def create_content(name)
       Alchemy::Content.create(element: element, name: name)
     end
 
-    def find_or_create_ingredient(role)
-      element.ingredients.find { |i| i.role == role } ||
-        Ingredient.create(element: element, role: role)
+    def find_or_create_ingredient(definition)
+      element.ingredients.detect { |i| i.role == definition[:role] } ||
+        element.ingredients.create!(
+          role: definition[:role],
+          type: Alchemy::Ingredient.normalize_type(definition[:type]),
+        )
     end
   end
 end

app/models/alchemy/element/element_ingredients.rb

@@ -172,8 +172,11 @@ def ingredient_error_messages
 
       # Builds ingredients for this element as described in the +elements.yml+
       def build_ingredients
-        self.ingredients = ingredient_definitions.map do |attributes|
-          Ingredient.build(role: attributes[:role], element: self)
+        ingredient_definitions.each do |attributes|
+          ingredients.build(
+            role: attributes[:role],
+            type: Alchemy::Ingredient.normalize_type(attributes[:type]),
+          )
         end
       end
     end

app/models/alchemy/ingredient.rb

@@ -6,12 +6,13 @@ class DefinitionError < StandardError; end
 
     include Hints
 
-    self.abstract_class = true
     self.table_name = "alchemy_ingredients"
 
     belongs_to :element, touch: true, class_name: "Alchemy::Element", inverse_of: :ingredients
     belongs_to :related_object, polymorphic: true, optional: true
 
+    before_validation(on: :create) { self.value ||= default_value }
+
     validates :type, presence: true
     validates :role, presence: true
 
@@ -33,32 +34,6 @@ class DefinitionError < StandardError; end
     scope :videos, -> { where(type: "Alchemy::Ingredients::Video") }
 
     class << self
-      # Builds concrete ingredient class as described in the +elements.yml+
-      def build(attributes = {})
-        element = attributes[:element]
-        raise ArgumentError, "No element given. Please pass element in attributes." if element.nil?
-        raise ArgumentError, "No role given. Please pass role in attributes." if attributes[:role].nil?
-
-        definition = element.ingredient_definition_for(attributes[:role])
-        if definition.nil?
-          raise DefinitionError,
-            "No definition found for #{attributes[:role]}. Please define #{attributes[:role]} on #{element[:name]}."
-        end
-
-        ingredient_class = Ingredient.ingredient_class_by_type(definition[:type])
-        ingredient_class.new(
-          type: Ingredient.normalize_type(definition[:type]),
-          value: default_value(definition),
-          role: definition[:role],
-          element: element,
-        )
-      end
-
-      # Creates concrete ingredient class as described in the +elements.yml+
-      def create(attributes = {})
-        build(attributes).tap(&:save)
-      end
-
       # Defines getter and setter method aliases for related object
       #
       # @param [String|Symbol] The name of the alias
@@ -78,20 +53,6 @@ def related_object_alias(name, class_name:)
         end
       end
 
-      # Returns an ingredient class by type
-      #
-      # Raises NameError if there is no such class in the
-      # +Alchemy::Ingredients+ module namespace.
-      #
-      # If you add custom ingredient class,
-      # put them in the +Alchemy::Ingredients+ module namespace
-      #
-      # @param [String] The ingredient class name to constantize
-      # @return [Class]
-      def ingredient_class_by_type(ingredient_type)
-        normalize_type(ingredient_type).constantize
-      end
-
       # Modulize ingredient type
       #
       # Makes sure the passed ingredient type is in the +Alchemy::Ingredients+
@@ -112,22 +73,6 @@ def translated_label_for(role, element_name = nil)
           default: Alchemy.t("ingredient_roles.#{role}", default: role.humanize),
         )
       end
-
-      private
-
-      # Returns the default value from ingredient definition
-      #
-      # If the value is a symbol it gets passed through i18n
-      # inside the +alchemy.default_ingredient_texts+ scope
-      def default_value(definition)
-        default = definition[:default]
-        case default
-        when Symbol
-          Alchemy.t(default, scope: :default_ingredient_texts)
-        else
-          default
-        end
-      end
     end
 
     # Compatibility method for access from element
@@ -220,5 +165,19 @@ def preview_ingredient?
     def hint_translation_attribute
       role
     end
+
+    # Returns the default value from ingredient definition
+    #
+    # If the value is a symbol it gets passed through i18n
+    # inside the +alchemy.default_ingredient_texts+ scope
+    def default_value
+      default = definition[:default]
+      case default
+      when Symbol
+        Alchemy.t(default, scope: :default_ingredient_texts)
+      else
+        default
+      end
+    end
   end
 end

lib/alchemy/upgrader/tasks/ingredients_migrator.rb

@@ -30,7 +30,10 @@ def create_ingredients
                     next unless content
 
                     essence = content.essence
-                    ingredient = Alchemy::Ingredient.build(role: ingredient_definition[:role], element: element)
+                    ingredient = element.ingredients.build(
+                      role: ingredient_definition[:role],
+                      type: Alchemy::Ingredient.normalize_type(ingredient_definition[:type]),
+                    )
                     belongs_to_associations = essence.class.reflect_on_all_associations(:belongs_to)
                     if belongs_to_associations.any?
                       ingredient.related_object = essence.public_send(belongs_to_associations.first.name)

spec/controllers/alchemy/admin/elements_controller_spec.rb

@@ -237,7 +237,7 @@ module Alchemy
         end
 
         let(:element) { create(:alchemy_element, :with_ingredients) }
-        let(:ingredient) { element.ingredients.first }
+        let(:ingredient) { element.ingredient_by_role(:headline) }
         let(:ingredients_attributes) { { 0 => { id: ingredient.id, value: "Title" } } }
         let(:element_params) { { tag_list: "Tag 1", public: false, ingredients_attributes: ingredients_attributes } }
 

spec/controllers/alchemy/admin/ingredients_controller_spec.rb

@@ -95,11 +95,12 @@
     end
 
     it_behaves_like "having crop action", model_class: Alchemy::Ingredient do
+      let(:picture) { build_stubbed(:alchemy_picture) }
+
       let(:croppable_resource) do
-        Alchemy::Ingredient.build(
-          type: "Alchemy::Ingredients::Picture",
+        Alchemy::Ingredients::Picture.new(
           element: element,
-          attachment: attachment,
+          picture: picture,
           role: "picture",
         )
       end

spec/decorators/alchemy/element_editor_spec.rb

@@ -50,6 +50,42 @@
     end
   end
 
+  describe "#ingredients" do
+    let(:element) { create(:alchemy_element, :with_ingredients) }
+
+    subject(:ingredients) { element_editor.ingredients }
+
+    it "returns a ContentEditor instance for each ingredient defined" do
+      aggregate_failures do
+        ingredients.each do |ingredient|
+          expect(ingredient).to be_an(Alchemy::IngredientEditor)
+        end
+      end
+    end
+
+    context "with a ingredient defined but not existing yet" do
+      let(:element) { create(:alchemy_element, name: "headline") }
+
+      before do
+        expect(element).to receive(:definition).at_least(:once) do
+          {
+            name: "headline",
+            ingredients: [
+              {
+                role: "headline",
+                type: "Headline",
+              },
+            ],
+          }.with_indifferent_access
+        end
+      end
+
+      it "creates the missing ingredient" do
+        expect { subject }.to change { element.ingredients.count }.by(1)
+      end
+    end
+  end
+
   describe "#to_partial_path" do
     subject { element_editor.to_partial_path }
 

spec/decorators/alchemy/ingredient_editor_spec.rb

@@ -4,7 +4,7 @@
 
 RSpec.describe Alchemy::IngredientEditor do
   let(:element) { build(:alchemy_element, name: "element_with_ingredients") }
-  let(:ingredient) { Alchemy::Ingredients::Text.build(role: "headline", element: element) }
+  let(:ingredient) { Alchemy::Ingredients::Text.new(role: "headline", element: element) }
   let(:ingredient_editor) { described_class.new(ingredient) }
 
   describe "#ingredient" do
@@ -190,7 +190,7 @@
         let(:element) { build(:alchemy_element, name: "all_you_can_eat_ingredients") }
 
         let(:ingredient) do
-          Alchemy::Ingredients::Html.build(
+          Alchemy::Ingredients::Html.new(
             role: "html",
             element: element,
           )

spec/helpers/alchemy/admin/ingredients_helper_spec.rb

@@ -4,7 +4,7 @@
 
 describe Alchemy::Admin::IngredientsHelper do
   let(:element) { build_stubbed(:alchemy_element, name: "element_with_ingredients") }
-  let(:ingredient) { Alchemy::Ingredients::Text.build(role: "headline", element: element) }
+  let(:ingredient) { Alchemy::Ingredients::Text.new(role: "headline", element: element) }
   let(:ingredient_editor) { Alchemy::IngredientEditor.new(ingredient) }
 
   describe "#ingredient_label" do

spec/helpers/alchemy/elements_block_helper_spec.rb

@@ -116,7 +116,7 @@ module Alchemy
 
         context "with element having ingredients" do
           let(:element) { create(:alchemy_element, :with_ingredients) }
-          let(:ingredient) { element.ingredients.first }
+          let(:ingredient) { element.ingredient_by_role(:headline) }
 
           it "should return the ingredients value" do
             Alchemy::Deprecation.silenced do

spec/models/alchemy/element_ingredients_spec.rb

@@ -70,7 +70,7 @@
     let!(:element) { create(:alchemy_element, :with_ingredients) }
 
     context "with role existing" do
-      let(:ingredient) { element.ingredients.first }
+      let(:ingredient) { element.ingredient_by_role(:headline) }
 
       context "with blank value" do
         before do
@@ -100,19 +100,15 @@
     let!(:element) { create(:alchemy_element, :with_ingredients) }
 
     context "with role existing" do
-      let(:ingredient) { element.ingredients.first }
+      let(:ingredient) { element.ingredient_by_role(:headline) }
 
       context "with blank value" do
-        before do
-          expect(ingredient).to receive(:value) { nil }
-        end
-
         it { expect(element.has_value_for?(:headline)).to be(false) }
       end
 
       context "with value present" do
         before do
-          expect(ingredient).to receive(:value) { "Headline" }
+          ingredient.value = "Headline"
         end
 
         it "should return ingredient" do
@@ -163,7 +159,6 @@
       expect(element.ingredient_error_messages).to eq([
         "Please enter a headline for all you can eat",
         "Text is invalid",
-        "Please select something else",
       ])
     end
   end