From 517a90921a59c04393622308a3dad0e4af03632a Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Thu, 8 Jan 2026 19:19:11 +0100 Subject: [PATCH] feat(element): Ensure nested elements share parent's page_version Add validation that nested elements must have the same page_version_id as their parent element. This enforces data integrity and enables performance optimizations that rely on all ancestors being in the same page version. --- app/models/alchemy/element.rb | 8 +++++++ config/locales/alchemy.en.yml | 4 ++++ .../test_support/factories/element_factory.rb | 4 ++-- .../alchemy/admin/elements_controller_spec.rb | 16 +++++++------ .../alchemy/api/elements_controller_spec.rb | 3 ++- spec/libraries/elements_finder_spec.rb | 3 ++- spec/models/alchemy/element_spec.rb | 24 +++++++++++++++++++ 7 files changed, 51 insertions(+), 11 deletions(-) diff --git a/app/models/alchemy/element.rb b/app/models/alchemy/element.rb index bf721cd949..33bb4f50ea 100644 --- a/app/models/alchemy/element.rb +++ b/app/models/alchemy/element.rb @@ -96,6 +96,7 @@ class Element < BaseRecord validates_presence_of :name, on: :create validates_format_of :name, on: :create, with: NAME_REGEXP + validate :validate_same_page_version_as_parent after_initialize :set_default_public_on, if: :new_record? @@ -326,6 +327,13 @@ def set_default_public_on self.public_on ||= Time.current end + def validate_same_page_version_as_parent + return unless parent_element + return if page_version_id == parent_element.page_version_id + + errors.add(:page_version_id, :must_match_parent) + end + def generate_nested_elements definition.autogenerate.each do |nestable_element| if nestable_elements.include?(nestable_element) diff --git a/config/locales/alchemy.en.yml b/config/locales/alchemy.en.yml index f1c3ff8e17..d4c049f9c4 100644 --- a/config/locales/alchemy.en.yml +++ b/config/locales/alchemy.en.yml @@ -811,6 +811,10 @@ en: base: restrict_dependent_destroy: has_many: "There are still %{record} attached to this page. Please remove them first." + alchemy/element: + attributes: + page_version_id: + must_match_parent: "must be the same as the parent element's page version" models: gutentag/tag: one: Tag diff --git a/lib/alchemy/test_support/factories/element_factory.rb b/lib/alchemy/test_support/factories/element_factory.rb index fec9d34555..f288ff0a4c 100644 --- a/lib/alchemy/test_support/factories/element_factory.rb +++ b/lib/alchemy/test_support/factories/element_factory.rb @@ -4,7 +4,7 @@ factory :alchemy_element, class: "Alchemy::Element" do name { "article" } autogenerate_ingredients { false } - association :page_version, factory: :alchemy_page_version + page_version { parent_element&.page_version || association(:alchemy_page_version) } trait :fixed do fixed { true } @@ -21,7 +21,7 @@ end trait :nested do - parent_element { build(:alchemy_element, name: "slider", page_version: page_version) } + parent_element { association(:alchemy_element, name: "slider") } name { "slide" } end diff --git a/spec/controllers/alchemy/admin/elements_controller_spec.rb b/spec/controllers/alchemy/admin/elements_controller_spec.rb index ae78d36701..c1fe09d5db 100644 --- a/spec/controllers/alchemy/admin/elements_controller_spec.rb +++ b/spec/controllers/alchemy/admin/elements_controller_spec.rb @@ -16,7 +16,8 @@ module Alchemy describe "#index" do let!(:page_version) { create(:alchemy_page_version) } let!(:element) { create(:alchemy_element, page_version: page_version) } - let!(:nested_element) { create(:alchemy_element, :nested, page_version: page_version) } + let!(:parent_for_nested) { create(:alchemy_element, :with_nestable_elements, page_version: page_version) } + let!(:nested_element) { create(:alchemy_element, name: "slide", parent_element: parent_for_nested) } let!(:hidden_element) { create(:alchemy_element, page_version: page_version, public: false) } context "with fixed elements" do @@ -36,7 +37,7 @@ module Alchemy it "assigns elements" do get :index, params: {page_version_id: page_version.id} - expect(assigns(:elements)).to eq([element, nested_element.parent_element, hidden_element]) + expect(assigns(:elements)).to eq([element, parent_for_nested, hidden_element]) end end @@ -69,7 +70,7 @@ module Alchemy end context "when nested inside parent element" do - let(:parent) { create(:alchemy_element) } + let(:parent) { create(:alchemy_element, page_version: page_version) } it "touches the cache key of parent element" do parent.update_column(:updated_at, 3.days.ago) @@ -188,12 +189,13 @@ module Alchemy end context "with parent_element_id given" do - let(:element_in_clipboard) { create(:alchemy_element, :nested, page_version: page_version) } - let(:parent_element) { create(:alchemy_element, :with_nestable_elements) } + let(:original_parent) { create(:alchemy_element, :with_nestable_elements, page_version: page_version) } + let(:element_in_clipboard) { create(:alchemy_element, name: "slide", parent_element: original_parent) } + let(:new_parent) { create(:alchemy_element, :with_nestable_elements, page_version: page_version) } it "moves the element to new parent" do - post :create, params: {paste_from_clipboard: element_in_clipboard.id, element: {page_version_id: page_version.id, parent_element_id: parent_element.id}}, xhr: true - expect(Alchemy::Element.last.parent_element_id).to eq(parent_element.id) + post :create, params: {paste_from_clipboard: element_in_clipboard.id, element: {page_version_id: page_version.id, parent_element_id: new_parent.id}}, xhr: true + expect(Alchemy::Element.last.parent_element_id).to eq(new_parent.id) end end end diff --git a/spec/controllers/alchemy/api/elements_controller_spec.rb b/spec/controllers/alchemy/api/elements_controller_spec.rb index 50135749d9..9f44079bd7 100644 --- a/spec/controllers/alchemy/api/elements_controller_spec.rb +++ b/spec/controllers/alchemy/api/elements_controller_spec.rb @@ -11,7 +11,8 @@ module Alchemy before do 2.times { create(:alchemy_element, page_version: page_version) } - create(:alchemy_element, :nested, page_version: page_version) + # Create a slider element which auto-generates a nested slide element + create(:alchemy_element, :with_nestable_elements, page_version: page_version) end it "returns all not nested elements from published versions as json objects" do diff --git a/spec/libraries/elements_finder_spec.rb b/spec/libraries/elements_finder_spec.rb index c46300c08b..84c7fb297d 100644 --- a/spec/libraries/elements_finder_spec.rb +++ b/spec/libraries/elements_finder_spec.rb @@ -63,7 +63,8 @@ end context "with nested elements present" do - let!(:nested_element) { create(:alchemy_element, :nested, page_version: page_version) } + let!(:parent_element) { create(:alchemy_element, :with_nestable_elements, page_version: page_version) } + let!(:nested_element) { create(:alchemy_element, name: "slide", parent_element: parent_element) } it "does not include nested elements" do is_expected.to_not include(nested_element) diff --git a/spec/models/alchemy/element_spec.rb b/spec/models/alchemy/element_spec.rb index ccfe7b2e14..01a7e40aad 100644 --- a/spec/models/alchemy/element_spec.rb +++ b/spec/models/alchemy/element_spec.rb @@ -9,6 +9,30 @@ module Alchemy it { is_expected.to have_one(:page).through(:page_version) } it { is_expected.to have_one(:language).through(:page) } + describe "validations" do + describe "page_version must match parent" do + let(:page_version) { create(:alchemy_page_version) } + let(:other_page_version) { create(:alchemy_page_version) } + let(:parent) { create(:alchemy_element, name: "slider", page_version: page_version) } + + it "is valid when page_version matches parent's page_version" do + element = build(:alchemy_element, name: "slide", parent_element: parent, page_version: page_version) + expect(element).to be_valid + end + + it "is invalid when page_version differs from parent's page_version" do + element = build(:alchemy_element, name: "slide", parent_element: parent, page_version: other_page_version) + expect(element).not_to be_valid + expect(element.errors[:page_version_id]).to include("must be the same as the parent element's page version") + end + + it "is valid for root elements (no parent)" do + element = build(:alchemy_element, page_version: page_version) + expect(element).to be_valid + end + end + end + # to prevent memoization before { ElementDefinition.instance_variable_set(:@definitions, nil) }