diff --git a/app/decorators/alchemy/element_editor.rb b/app/decorators/alchemy/element_editor.rb index a0affbf90b..60500e07e8 100644 --- a/app/decorators/alchemy/element_editor.rb +++ b/app/decorators/alchemy/element_editor.rb @@ -26,7 +26,7 @@ def contents # @return Array 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 diff --git a/app/models/alchemy/element/element_ingredients.rb b/app/models/alchemy/element/element_ingredients.rb index 95b2e4ff56..a4de014bcd 100644 --- a/app/models/alchemy/element/element_ingredients.rb +++ b/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 diff --git a/app/models/alchemy/ingredient.rb b/app/models/alchemy/ingredient.rb index 58ce87f7b0..925820d2cd 100644 --- a/app/models/alchemy/ingredient.rb +++ b/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 diff --git a/lib/alchemy/upgrader/tasks/ingredients_migrator.rb b/lib/alchemy/upgrader/tasks/ingredients_migrator.rb index e603e7744f..ab977ff569 100644 --- a/lib/alchemy/upgrader/tasks/ingredients_migrator.rb +++ b/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) diff --git a/spec/controllers/alchemy/admin/elements_controller_spec.rb b/spec/controllers/alchemy/admin/elements_controller_spec.rb index 64f15b6e75..34249a7014 100644 --- a/spec/controllers/alchemy/admin/elements_controller_spec.rb +++ b/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 } } diff --git a/spec/controllers/alchemy/admin/ingredients_controller_spec.rb b/spec/controllers/alchemy/admin/ingredients_controller_spec.rb index bebe87a40d..ca7cf993ec 100644 --- a/spec/controllers/alchemy/admin/ingredients_controller_spec.rb +++ b/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 diff --git a/spec/decorators/alchemy/element_editor_spec.rb b/spec/decorators/alchemy/element_editor_spec.rb index a23cd85143..49e0290cdf 100644 --- a/spec/decorators/alchemy/element_editor_spec.rb +++ b/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 } diff --git a/spec/decorators/alchemy/ingredient_editor_spec.rb b/spec/decorators/alchemy/ingredient_editor_spec.rb index e8126661f2..088dd20966 100644 --- a/spec/decorators/alchemy/ingredient_editor_spec.rb +++ b/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, ) diff --git a/spec/helpers/alchemy/admin/ingredients_helper_spec.rb b/spec/helpers/alchemy/admin/ingredients_helper_spec.rb index cff0000a47..e2f704e402 100644 --- a/spec/helpers/alchemy/admin/ingredients_helper_spec.rb +++ b/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 diff --git a/spec/helpers/alchemy/elements_block_helper_spec.rb b/spec/helpers/alchemy/elements_block_helper_spec.rb index b9827adb1d..c081854724 100644 --- a/spec/helpers/alchemy/elements_block_helper_spec.rb +++ b/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 diff --git a/spec/models/alchemy/element_ingredients_spec.rb b/spec/models/alchemy/element_ingredients_spec.rb index 2ed3797f4c..5832b3d107 100644 --- a/spec/models/alchemy/element_ingredients_spec.rb +++ b/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 diff --git a/spec/models/alchemy/ingredient_spec.rb b/spec/models/alchemy/ingredient_spec.rb index 507fe16612..8cb5c41ffe 100644 --- a/spec/models/alchemy/ingredient_spec.rb +++ b/spec/models/alchemy/ingredient_spec.rb @@ -45,86 +45,6 @@ end end - describe ".build" do - subject { described_class.build(attributes) } - - context "without element" do - let(:attributes) { {} } - - it { expect { subject }.to raise_error(ArgumentError) } - end - - context "with element" do - context "without role given" do - let(:attributes) { { element: element } } - - it { expect { subject }.to raise_error(ArgumentError) } - end - - context "with role given" do - let(:attributes) { { element: element, role: "headline" } } - - it { is_expected.to be_an(Alchemy::Ingredients::Text) } - end - - context "with default defined" do - let(:attributes) { { element: element, role: "headline" } } - - context "defined as String" do - it "sets default value" do - expect(subject.value).to eq("Hello World") - end - end - - context "defined as Symbol" do - let(:attributes) { { element: element, role: "text" } } - - it "sets translated default value" do - expect(subject.value).to eq("Dapibus nostra massa phasellus viverra rhoncus fringilla") - end - end - end - - context "with undefined role given" do - let(:attributes) { { element: element, role: "foo" } } - - it { expect { subject }.to raise_error(Alchemy::Ingredient::DefinitionError) } - end - end - end - - describe ".create" do - subject { described_class.create(attributes) } - - let(:attributes) { { element: element, role: "headline" } } - - it { expect { subject }.to change(Alchemy::Ingredients::Text, :count).by(1) } - - it "returns self" do - is_expected.to be_an(Alchemy::Ingredients::Text) - end - end - - describe ".ingredient_class_by_type" do - subject { described_class.ingredient_class_by_type(ingredient_type) } - - context "with a known ingredient class" do - let(:ingredient_type) { "Text" } - - it "returns full ingredient constant" do - expect(subject).to eq(Alchemy::Ingredients::Text) - end - end - - context "with unkown ingredient class" do - let(:ingredient_type) { "Foo" } - - it do - expect { subject }.to raise_error(NameError) - end - end - end - describe ".normalize_type" do subject { described_class.normalize_type("Text") } @@ -134,14 +54,14 @@ end describe "#settings" do - let(:ingredient) { Alchemy::Ingredients::Text.build(role: "headline", element: element) } + let(:ingredient) { Alchemy::Ingredients::Text.new(role: "headline", element: element) } it "returns the settings hash from definition" do expect(ingredient.settings).to eq({ "linkable" => true }) end context "if settings are not defined" do - let(:ingredient) { Alchemy::Ingredients::Text.build(role: "text", element: element) } + let(:ingredient) { Alchemy::Ingredients::Text.new(role: "text", element: element) } it "returns empty hash" do expect(ingredient.settings).to eq({}) @@ -150,7 +70,7 @@ end describe "#settings_value" do - let(:ingredient) { Alchemy::Ingredients::Text.build(role: "headline", element: element) } + let(:ingredient) { Alchemy::Ingredients::Text.new(role: "headline", element: element) } let(:key) { :linkable } let(:options) { {} } @@ -189,7 +109,7 @@ end context "with ingredient having no settings" do - let(:ingredient) { Alchemy::Ingredients::Richtext.build(role: "text", element: element) } + let(:ingredient) { Alchemy::Ingredients::Richtext.new(role: "text", element: element) } context "and empty options" do let(:options) { {} } @@ -208,7 +128,7 @@ end describe "#partial_name" do - let(:ingredient) { Alchemy::Ingredients::Richtext.build(role: "text", element: element) } + let(:ingredient) { Alchemy::Ingredients::Richtext.new(role: "text", element: element) } subject { ingredient.partial_name } @@ -218,7 +138,7 @@ end describe "#to_partial_path" do - let(:ingredient) { Alchemy::Ingredients::Richtext.build(role: "text", element: element) } + let(:ingredient) { Alchemy::Ingredients::Richtext.new(role: "text", element: element) } subject { ingredient.to_partial_path } @@ -228,7 +148,7 @@ end describe "#has_validations?" do - let(:ingredient) { Alchemy::Ingredients::Text.build(role: "headline", element: element) } + let(:ingredient) { Alchemy::Ingredients::Text.new(role: "headline", element: element) } subject { ingredient.has_validations? } @@ -248,7 +168,7 @@ end describe "#has_hint?" do - let(:ingredient) { Alchemy::Ingredients::Text.build(role: "headline", element: element) } + let(:ingredient) { Alchemy::Ingredients::Text.new(role: "headline", element: element) } subject { ingredient.has_hint? } @@ -268,7 +188,7 @@ end describe "#deprecated?" do - let(:ingredient) { Alchemy::Ingredients::Text.build(role: "headline", element: element) } + let(:ingredient) { Alchemy::Ingredients::Text.new(role: "headline", element: element) } subject { ingredient.deprecated? } @@ -298,7 +218,7 @@ end describe "#preview_ingredient?" do - let(:ingredient) { Alchemy::Ingredients::Text.build(role: "headline", element: element) } + let(:ingredient) { Alchemy::Ingredients::Text.new(role: "headline", element: element) } subject { ingredient.preview_ingredient? } @@ -320,7 +240,7 @@ describe "#has_tinymce?" do subject { ingredient.has_tinymce? } - let(:ingredient) { Alchemy::Ingredient.build(role: "headline", element: element) } + let(:ingredient) { Alchemy::Ingredients::Headline.new(role: "headline", element: element) } it { is_expected.to be(false) } end diff --git a/spec/models/alchemy/ingredients/headline_spec.rb b/spec/models/alchemy/ingredients/headline_spec.rb index 0376f5cb26..11ddb23195 100644 --- a/spec/models/alchemy/ingredients/headline_spec.rb +++ b/spec/models/alchemy/ingredients/headline_spec.rb @@ -60,7 +60,7 @@ end it "should have the size and level fields filled with correct defaults" do - ingredient = Alchemy::Ingredient.create(element: element, role: "headline") + ingredient = described_class.create(element: element, role: "headline") expect(ingredient.size).to eq(3) expect(ingredient.level).to eq(2) end diff --git a/spec/views/alchemy/ingredients/boolean_editor_spec.rb b/spec/views/alchemy/ingredients/boolean_editor_spec.rb index 29e13af785..fcf60831e9 100644 --- a/spec/views/alchemy/ingredients/boolean_editor_spec.rb +++ b/spec/views/alchemy/ingredients/boolean_editor_spec.rb @@ -7,7 +7,7 @@ let(:element_editor) { Alchemy::ElementEditor.new(element) } let(:ingredient) do - Alchemy::Ingredients::Boolean.build(role: "boolean", type: "Boolean", element: element) + Alchemy::Ingredients::Boolean.new(role: "boolean", element: element) end before do @@ -28,17 +28,17 @@ end context "with default value given in ingredient settings" do - before do - expect(element).to receive(:ingredient_definition_for) { ingredient_definition } - allow_any_instance_of(Alchemy::Ingredients::Boolean).to receive(:definition) { ingredient_definition } - end - - let(:ingredient_definition) do - { - role: "boolean", - type: "Boolean", - default: true, - }.with_indifferent_access + let(:element) { create(:alchemy_element, name: "all_you_can_eat_ingredients") } + + let(:ingredient) do + allow_any_instance_of(Alchemy::Ingredients::Boolean).to receive(:definition) do + { + role: "boolean", + type: "Boolean", + default: true, + }.with_indifferent_access + end + Alchemy::Ingredients::Boolean.create!(role: "boolean", element: element) end it "checks the checkbox" do diff --git a/spec/views/alchemy/ingredients/datetime_editor_spec.rb b/spec/views/alchemy/ingredients/datetime_editor_spec.rb index 0aaf9d634a..56bcbea4cc 100644 --- a/spec/views/alchemy/ingredients/datetime_editor_spec.rb +++ b/spec/views/alchemy/ingredients/datetime_editor_spec.rb @@ -5,7 +5,7 @@ RSpec.describe "alchemy/ingredients/_datetime_editor" do let(:element) { build_stubbed(:alchemy_element, name: "all_you_can_eat_ingredients") } let(:element_editor) { Alchemy::ElementEditor.new(element) } - let(:ingredient) { Alchemy::Ingredients::Datetime.build(role: "datetime", element: element) } + let(:ingredient) { Alchemy::Ingredients::Datetime.new(role: "datetime", element: element) } before do allow(element_editor).to receive(:ingredients) { [Alchemy::IngredientEditor.new(ingredient)] }