From 98082db6a8e28cc0dfb4e66fc997934fb6c5b36e Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 5 May 2020 17:46:19 +0200 Subject: [PATCH 1/4] Remove layout root page We do not need this hidden page. All pages that share the same language and have `layoutpage` set to true are the layoutpages for the current language. --- .../alchemy/admin/layoutpages_controller.rb | 2 +- app/models/alchemy/language.rb | 9 ++--- app/models/alchemy/page.rb | 20 +---------- .../alchemy/admin/layoutpages/index.html.erb | 4 +-- lib/alchemy/seeder.rb | 20 +++++------ .../test_support/factories/page_factory.rb | 4 +-- .../admin/layoutpages_controller_spec.rb | 10 ++++-- spec/models/alchemy/page_spec.rb | 33 ------------------- 8 files changed, 22 insertions(+), 80 deletions(-) diff --git a/app/controllers/alchemy/admin/layoutpages_controller.rb b/app/controllers/alchemy/admin/layoutpages_controller.rb index 23a2e42765..2279f19e42 100644 --- a/app/controllers/alchemy/admin/layoutpages_controller.rb +++ b/app/controllers/alchemy/admin/layoutpages_controller.rb @@ -10,7 +10,7 @@ class LayoutpagesController < Alchemy::Admin::BaseController helper Alchemy::Admin::PagesHelper def index - @layout_root = Page.find_or_create_layout_root_for(@current_language.id) + @layout_pages = Page.layoutpages.where(language: @current_language) @languages = Language.on_current_site end diff --git a/app/models/alchemy/language.rb b/app/models/alchemy/language.rb index ecffc4e3d9..29247dc821 100644 --- a/app/models/alchemy/language.rb +++ b/app/models/alchemy/language.rb @@ -60,8 +60,8 @@ class Language < BaseRecord throw(:abort) end - scope :published, -> { where(public: true) } - scope :with_root_page, -> { joins(:pages).where(Page.table_name => {language_root: true}) } + scope :published, -> { where(public: true) } + scope :with_root_page, -> { joins(:pages).where(Page.table_name => { language_root: true }) } class << self def on_site(site) @@ -110,11 +110,6 @@ def root_page @root_page ||= pages.language_roots.first end - # Layout root page - def layout_root_page - @layout_root_page ||= Page.layout_root_for(id) - end - # All available locales matching this language # # Matching either the code (+language_code+ + +country_code+) or the +language_code+ diff --git a/app/models/alchemy/page.rb b/app/models/alchemy/page.rb index cf8bf98413..8699a0696a 100644 --- a/app/models/alchemy/page.rb +++ b/app/models/alchemy/page.rb @@ -116,7 +116,7 @@ class Page < BaseRecord validates_presence_of :language, on: :create, unless: :root validates_presence_of :page_layout, unless: :systempage? validates_format_of :page_layout, with: /\A[a-z0-9_-]+\z/, unless: -> { systempage? || page_layout.blank? } - validates_presence_of :parent_id, if: proc { Page.count > 1 } + validates_presence_of :parent_id, if: proc { Page.count > 1 }, unless: -> { layoutpage? } before_save :set_language_code, if: -> { language.present? }, @@ -218,24 +218,6 @@ def copy(source, differences = {}) end end - def layout_root_for(language_id) - where({ parent_id: Page.root.id, layoutpage: true, language_id: language_id }).limit(1).first - end - - def find_or_create_layout_root_for(language_id) - layoutroot = layout_root_for(language_id) - return layoutroot if layoutroot - - language = Language.find(language_id) - Page.create!( - name: "Layoutroot for #{language.name}", - layoutpage: true, - language: language, - autogenerate_elements: false, - parent_id: Page.root.id, - ) - end - def copy_and_paste(source, new_parent, new_name) page = copy(source, { parent_id: new_parent.id, diff --git a/app/views/alchemy/admin/layoutpages/index.html.erb b/app/views/alchemy/admin/layoutpages/index.html.erb index 43e7e1e0aa..499cd211c5 100644 --- a/app/views/alchemy/admin/layoutpages/index.html.erb +++ b/app/views/alchemy/admin/layoutpages/index.html.erb @@ -4,7 +4,7 @@ <%= render 'alchemy/admin/partials/language_tree_select' %> <%= toolbar_button( icon: :plus, - url: alchemy.new_admin_page_path(parent_id: @layout_root.id, layoutpage: true), + url: alchemy.new_admin_page_path(language: @current_language, layoutpage: true), hotkey: 'alt+n', dialog_options: { title: Alchemy.t('Add global page'), @@ -31,5 +31,5 @@ <% end %> diff --git a/lib/alchemy/seeder.rb b/lib/alchemy/seeder.rb index dfcfe9fafd..b2d7a47e3e 100644 --- a/lib/alchemy/seeder.rb +++ b/lib/alchemy/seeder.rb @@ -55,12 +55,8 @@ def seed_pages def seed_layoutpages desc "Seeding Alchemy layout pages from #{page_seeds_file}" language = Alchemy::Language.default - layout_root = Alchemy::Page.find_or_create_layout_root_for(language.id) layoutpages.each do |page| - create_page(page, { - parent: layout_root, - language: language, - }) + create_page(page, { language: language }) end end @@ -117,14 +113,14 @@ def create_default_language! default_language = Alchemy::Config.get(:default_language) if default_language Alchemy::Language.create!( - name: default_language["name"], - language_code: default_language["code"], - locale: default_language["code"], + name: default_language["name"], + language_code: default_language["code"], + locale: default_language["code"], frontpage_name: default_language["frontpage_name"], - page_layout: default_language["page_layout"], - public: true, - default: true, - site: Alchemy::Site.default, + page_layout: default_language["page_layout"], + public: true, + default: true, + site: Alchemy::Site.default, ) else raise DefaultLanguageNotFoundError diff --git a/lib/alchemy/test_support/factories/page_factory.rb b/lib/alchemy/test_support/factories/page_factory.rb index 5c16324bd6..b04b085067 100644 --- a/lib/alchemy/test_support/factories/page_factory.rb +++ b/lib/alchemy/test_support/factories/page_factory.rb @@ -11,7 +11,7 @@ parent_id do (Alchemy::Page.find_by(language_root: true) || - FactoryBot.create(:alchemy_page, :language_root, language: language)).id + FactoryBot.create(:alchemy_page, :language_root, language: language)).id end # This speeds up creating of pages dramatically. @@ -47,8 +47,6 @@ end trait :layoutpage do - name { "Footer" } - parent_id { Alchemy::Page.find_or_create_layout_root_for(Alchemy::Language.current.id).id } layoutpage { true } page_layout { "footer" } end diff --git a/spec/controllers/alchemy/admin/layoutpages_controller_spec.rb b/spec/controllers/alchemy/admin/layoutpages_controller_spec.rb index 5e7403f232..266bd6037f 100644 --- a/spec/controllers/alchemy/admin/layoutpages_controller_spec.rb +++ b/spec/controllers/alchemy/admin/layoutpages_controller_spec.rb @@ -21,9 +21,13 @@ module Alchemy context "with a language present" do let!(:language) { create(:alchemy_language) } - it "should assign @layout_root" do - get :index - expect(assigns(:layout_root)).to be_a(Page) + context "and layoutpages present" do + let!(:layoutpages) { create_list(:alchemy_page, 2, :layoutpage, language: language) } + + it "should assign @layout_pages" do + get :index + expect(assigns(:layout_pages)).to match_array(layoutpages) + end end it "should assign @languages" do diff --git a/spec/models/alchemy/page_spec.rb b/spec/models/alchemy/page_spec.rb index 845b2ae5cc..b0c1fd4160 100644 --- a/spec/models/alchemy/page_spec.rb +++ b/spec/models/alchemy/page_spec.rb @@ -399,15 +399,10 @@ module Alchemy end describe ".contentpages" do - let!(:layoutroot) do - Page.find_or_create_layout_root_for(klingon.id) - end - let!(:layoutpage) do create :alchemy_page, :public, { name: "layoutpage", layoutpage: true, - parent_id: layoutroot.id, language: klingon, } end @@ -604,34 +599,6 @@ module Alchemy end end - describe ".find_or_create_layout_root_for" do - subject { Page.find_or_create_layout_root_for(language.id) } - - let!(:root_page) { create(:alchemy_page, :root) } - let(:language) { create(:alchemy_language, name: "English") } - - context "if no layout root page for given language id is present" do - it "creates one" do - expect { - subject - }.to change { Page.count }.by(1) - end - end - - context "if layout root page for given language id is present" do - let!(:page) do - create :alchemy_page, - layoutpage: true, - parent_id: root_page.id, - language_id: language.id - end - - it "returns layout root page" do - is_expected.to eq(page) - end - end - end - describe ".language_roots" do it "should return 1 language_root" do create(:alchemy_page, :public, name: "First Public Child", parent_id: language_root.id, language: language) From d6e750d6b83cdf6f04a51b4d451daad30ffdcb40 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 5 May 2020 18:57:49 +0200 Subject: [PATCH 2/4] Pass the language in new page form We want to be sure that the language we instantiate the page with gets passed with the form --- app/views/alchemy/admin/pages/_new_page_form.html.erb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/alchemy/admin/pages/_new_page_form.html.erb b/app/views/alchemy/admin/pages/_new_page_form.html.erb index b44f5ed276..f717ae233e 100644 --- a/app/views/alchemy/admin/pages/_new_page_form.html.erb +++ b/app/views/alchemy/admin/pages/_new_page_form.html.erb @@ -1,5 +1,6 @@ <%= alchemy_form_for([:admin, @page]) do |f| %> <%= f.hidden_field(:parent_id) %> + <%= f.hidden_field(:language_id) %> <%= f.hidden_field(:layoutpage) %> <%= f.input :page_layout, collection: @page_layouts, From 1fbaa43df953ca95ce9fc2c83da08d67c580b67b Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 5 May 2020 18:59:01 +0200 Subject: [PATCH 3/4] Only set the language from parent if present And use the current page, not the default (that is the current anyway if no current language has been explicitely set). --- app/models/alchemy/page.rb | 8 +- spec/models/alchemy/page_spec.rb | 150 +++++++++++++++---------------- 2 files changed, 76 insertions(+), 82 deletions(-) diff --git a/app/models/alchemy/page.rb b/app/models/alchemy/page.rb index 8699a0696a..9a2107589f 100644 --- a/app/models/alchemy/page.rb +++ b/app/models/alchemy/page.rb @@ -137,8 +137,8 @@ class Page < BaseRecord before_save :set_fixed_attributes, if: -> { fixed_attributes.any? } - before_create :set_language_from_parent_or_default, - if: -> { language_id.blank? }, + before_create :set_language, + if: -> { language.nil? }, unless: :systempage? after_update :create_legacy_url, @@ -547,8 +547,8 @@ def select_page(pages, options = {}) .limit(1).first end - def set_language_from_parent_or_default - self.language = parent.language || Language.default + def set_language + self.language = parent&.language || Language.current set_language_code end diff --git a/spec/models/alchemy/page_spec.rb b/spec/models/alchemy/page_spec.rb index b0c1fd4160..10c0a1ff8e 100644 --- a/spec/models/alchemy/page_spec.rb +++ b/spec/models/alchemy/page_spec.rb @@ -4,13 +4,13 @@ module Alchemy describe Page do - let(:rootpage) { Page.root } - let(:language) { create(:alchemy_language, :english, default: true) } - let(:klingon) { create(:alchemy_language, :klingon) } + let(:rootpage) { Page.root } + let(:language) { create(:alchemy_language, :english, default: true) } + let(:klingon) { create(:alchemy_language, :klingon) } let(:language_root) { create(:alchemy_page, :language_root) } - let(:page) { mock_model(Page, page_layout: "foo") } - let(:public_page) { create(:alchemy_page, :public) } - let(:news_page) { create(:alchemy_page, :public, page_layout: "news", autogenerate_elements: true) } + let(:page) { mock_model(Page, page_layout: "foo") } + let(:public_page) { create(:alchemy_page, :public) } + let(:news_page) { create(:alchemy_page, :public, page_layout: "news", autogenerate_elements: true) } it { is_expected.to have_one(:site) } @@ -18,8 +18,8 @@ module Alchemy context "validations" do context "Creating a normal content page" do - let(:contentpage) { build(:alchemy_page) } - let(:with_same_urlname) { create(:alchemy_page, urlname: "existing_twice") } + let(:contentpage) { build(:alchemy_page) } + let(:with_same_urlname) { create(:alchemy_page, urlname: "existing_twice") } let(:global_with_same_urlname) { create(:alchemy_page, urlname: "existing_twice", layoutpage: true) } context "when its urlname exists as global page" do @@ -190,7 +190,7 @@ module Alchemy context "after_move" do let(:parent_1) { create(:alchemy_page, name: "Parent 1", visible: true) } let(:parent_2) { create(:alchemy_page, name: "Parent 2", visible: true) } - let(:page) { create(:alchemy_page, parent_id: parent_1.id, name: "Page", visible: true) } + let(:page) { create(:alchemy_page, parent_id: parent_1.id, name: "Page", visible: true) } it "updates the urlname" do expect(page.urlname).to eq("parent-1/page") @@ -310,8 +310,8 @@ module Alchemy page_1 = create(:alchemy_page, language: language) page_2 = create(:alchemy_page, language: language, name: "Another page") clipboard = [ - {"id" => page_1.id.to_s, "action" => "copy"}, - {"id" => page_2.id.to_s, "action" => "copy"}, + { "id" => page_1.id.to_s, "action" => "copy" }, + { "id" => page_2.id.to_s, "action" => "copy" }, ] expect(Page.all_from_clipboard_for_select(clipboard, language.id)).to include(page_1, page_2) end @@ -321,7 +321,7 @@ module Alchemy it "should not return any pages" do page_1 = create(:alchemy_page, language: language, page_layout: "contact") clipboard = [ - {"id" => page_1.id.to_s, "action" => "copy"}, + { "id" => page_1.id.to_s, "action" => "copy" }, ] expect(Page.all_from_clipboard_for_select(clipboard, language.id)).to eq([]) end @@ -332,8 +332,8 @@ module Alchemy page_1 = create(:alchemy_page, language: language, page_layout: "standard") page_2 = create(:alchemy_page, name: "Another page", language: language, page_layout: "contact") clipboard = [ - {"id" => page_1.id.to_s, "action" => "copy"}, - {"id" => page_2.id.to_s, "action" => "copy"}, + { "id" => page_1.id.to_s, "action" => "copy" }, + { "id" => page_2.id.to_s, "action" => "copy" }, ] expect(Page.all_from_clipboard_for_select(clipboard, language.id)).to eq([page_1]) end @@ -352,9 +352,8 @@ module Alchemy before do create(:alchemy_page, :public, :locked, locked_by: 53) # This page must not be part of the collection - allow(user.class) - .to receive(:primary_key) - .and_return("id") + allow(user.class).to receive(:primary_key) + .and_return("id") end it "should return the correct page collection blocked by a certain user" do @@ -366,9 +365,8 @@ module Alchemy let(:user) { double(:user, user_id: 123, class: DummyUser) } before do - allow(user.class) - .to receive(:primary_key) - .and_return("user_id") + allow(user.class).to receive(:primary_key) + .and_return("user_id") end it "should return the correct page collection blocked by a certain user" do @@ -380,8 +378,8 @@ module Alchemy describe ".ancestors_for" do let(:lang_root) { Page.language_root_for(Language.default.id) } - let(:parent) { create(:alchemy_page, :public) } - let(:page) { create(:alchemy_page, :public, parent_id: parent.id) } + let(:parent) { create(:alchemy_page, :public) } + let(:page) { create(:alchemy_page, :public, parent_id: parent.id) } it "returns an array of all parents including self" do expect(Page.ancestors_for(page)).to eq([lang_root, parent, page]) @@ -536,7 +534,7 @@ module Alchemy end context "with different page name given" do - subject { Page.copy(page, {name: "Different name"}) } + subject { Page.copy(page, { name: "Different name" }) } it "should take this name" do expect(subject.name).to eq("Different name") @@ -585,14 +583,14 @@ module Alchemy context "with language given" do it "does not set the language from parent" do - expect_any_instance_of(Page).not_to receive(:set_language_from_parent_or_default) + expect_any_instance_of(Page).not_to receive(:set_language) Page.create!(name: "A", parent_id: language_root.id, page_layout: "standard", language: language) end end context "with no language given" do it "sets the language from parent" do - expect_any_instance_of(Page).to receive(:set_language_from_parent_or_default) + expect_any_instance_of(Page).to receive(:set_language) Page.create!(name: "A", parent_id: language_root.id, page_layout: "standard") end end @@ -739,13 +737,13 @@ module Alchemy { "name" => "column_headline", "amount" => 3, - "contents" => [{"name" => "headline", "type" => "EssenceText"}], + "contents" => [{ "name" => "headline", "type" => "EssenceText" }], }, { "name" => "unique_headline", "unique" => true, "amount" => 3, - "contents" => [{"name" => "headline", "type" => "EssenceText"}], + "contents" => [{ "name" => "headline", "type" => "EssenceText" }], }, ]) allow(PageLayout).to receive(:get).and_return({ @@ -965,11 +963,11 @@ module Alchemy describe "#element_definitions" do let(:page) { build_stubbed(:alchemy_page) } subject { page.element_definitions } - before { expect(Element).to receive(:definitions).and_return([{"name" => "article"}, {"name" => "header"}]) } + before { expect(Element).to receive(:definitions).and_return([{ "name" => "article" }, { "name" => "header" }]) } it "returns all element definitions that could be placed on current page" do - is_expected.to include({"name" => "article"}) - is_expected.to include({"name" => "header"}) + is_expected.to include({ "name" => "article" }) + is_expected.to include({ "name" => "header" }) end end @@ -990,9 +988,9 @@ module Alchemy end expect(Element).to receive(:definitions).at_least(:once) do [ - {"name" => "slider", "nestable_elements" => %w(slide)}, - {"name" => "gallery", "nestable_elements" => %w(slide)}, - {"name" => "slide"}, + { "name" => "slider", "nestable_elements" => %w(slide) }, + { "name" => "gallery", "nestable_elements" => %w(slide) }, + { "name" => "slide" }, ] end end @@ -1039,7 +1037,7 @@ module Alchemy context "with elements assigned in page definition" do let(:page_definition) do - {"elements" => %w(article)} + { "elements" => %w(article) } end it "returns an array of the page's element names" do @@ -1108,9 +1106,9 @@ module Alchemy context "with existing public child" do let!(:first_public_child) do create :alchemy_page, :public, - name: "First public child", - language: language, - parent_id: language_root.id + name: "First public child", + language: language, + parent_id: language_root.id end it "should return first_public_child" do @@ -1145,9 +1143,7 @@ module Alchemy context "if page is folded" do before do - expect(page) - .to receive(:folded_pages) - .and_return double(where: double(any?: true)) + expect(page).to receive(:folded_pages).and_return double(where: double(any?: true)) end it "should return true" do @@ -1230,11 +1226,11 @@ module Alchemy it "should copy the source page with the given name to the new parent" do expect(Page).to receive(:copy).with(source, { - parent_id: new_parent.id, - language: new_parent.language, - name: page_name, - title: page_name, - }) + parent_id: new_parent.id, + language: new_parent.language, + name: page_name, + title: page_name, + }) subject end @@ -1254,8 +1250,8 @@ module Alchemy end context "previous and next." do - let(:center_page) { create(:alchemy_page, :public, name: "Center Page") } - let(:next_page) { create(:alchemy_page, :public, name: "Next Page") } + let(:center_page) { create(:alchemy_page, :public, name: "Center Page") } + let(:next_page) { create(:alchemy_page, :public, name: "Next Page") } let(:non_public_page) { create(:alchemy_page, name: "Not public Page") } let(:restricted_page) { create(:alchemy_page, :restricted, :public) } @@ -1329,7 +1325,7 @@ module Alchemy context "template defines one alchemy role" do before do - allow(page).to receive(:definition).and_return({"editable_by" => ["freelancer"]}) + allow(page).to receive(:definition).and_return({ "editable_by" => ["freelancer"] }) end context "user has matching alchemy role" do @@ -1350,7 +1346,7 @@ module Alchemy context "template defines multiple alchemy roles" do before do - allow(page).to receive(:definition).and_return({"editable_by" => ["freelancer", "admin"]}) + allow(page).to receive(:definition).and_return({ "editable_by" => ["freelancer", "admin"] }) end context "user has matching alchemy role" do @@ -1484,13 +1480,12 @@ module Alchemy let(:current_time) { Time.current.change(usec: 0) } let(:page) do create(:alchemy_page, - public_on: public_on, - public_until: public_until, - published_at: published_at, - ) + public_on: public_on, + public_until: public_until, + published_at: published_at) end let(:published_at) { nil } - let(:public_on) { nil } + let(:public_on) { nil } let(:public_until) { nil } before do @@ -1501,19 +1496,19 @@ module Alchemy context "with unpublished page" do it "sets public_on and published_at", aggregate_failures: true do expect(page.published_at).to eq(current_time) - expect(page.public_on).to eq(current_time) + expect(page.public_on).to eq(current_time) expect(page.public_until).to eq(nil) end end context "with already published page" do - let(:past_time) { current_time - 3.weeks } + let(:past_time) { current_time - 3.weeks } let(:published_at) { past_time } - let(:public_on) { past_time } + let(:public_on) { past_time } it "only sets published_at", aggregate_failures: true do expect(page.published_at).to eq(current_time) - expect(page.public_on).to eq(public_on) + expect(page.public_on).to eq(public_on) expect(page.public_until).to eq(nil) end @@ -1531,27 +1526,27 @@ module Alchemy it "resets public_on and sets published_at", aggregate_failures: true do expect(page.published_at).to eq(current_time) - expect(page.public_on).to eq(current_time) + expect(page.public_on).to eq(current_time) expect(page.public_until).to eq(nil) end end context "with not anymore published page" do - let(:past_time) { current_time - 3.weeks } + let(:past_time) { current_time - 3.weeks } let(:published_at) { past_time } - let(:public_on) { past_time } + let(:public_on) { past_time } let(:public_until) { past_time + 1.week } it "resets public_on and published_at and sets public_until to nil", aggregate_failures: true do expect(page.published_at).to eq(current_time) - expect(page.public_on).to eq(public_on) + expect(page.public_on).to eq(public_on) expect(page.public_until).to eq(nil) end end end - describe "#set_language_from_parent_or_default" do + describe "#set_language" do let(:default_language) { mock_model("Language", code: "es") } let(:page) { Page.new } @@ -1563,7 +1558,7 @@ module Alchemy let(:parent) { mock_model("Page", language: default_language, language_id: default_language.id, language_code: default_language.code) } before do - page.send(:set_language_from_parent_or_default) + page.send(:set_language) end describe "#language_id" do @@ -1577,7 +1572,7 @@ module Alchemy before do allow(Language).to receive(:default).and_return(default_language) - page.send(:set_language_from_parent_or_default) + page.send(:set_language) end describe "#language_id" do @@ -1591,7 +1586,7 @@ module Alchemy context "definition has 'taggable' key with true value" do it "should return true" do page = build(:alchemy_page) - allow(page).to receive(:definition).and_return({"name" => "standard", "taggable" => true}) + allow(page).to receive(:definition).and_return({ "name" => "standard", "taggable" => true }) expect(page.taggable?).to be_truthy end end @@ -1599,7 +1594,7 @@ module Alchemy context "definition has 'taggable' key with foo value" do it "should return false" do page = build(:alchemy_page) - allow(page).to receive(:definition).and_return({"name" => "standard", "taggable" => "foo"}) + allow(page).to receive(:definition).and_return({ "name" => "standard", "taggable" => "foo" }) expect(page.taggable?).to be_falsey end end @@ -1607,7 +1602,7 @@ module Alchemy context "definition has no 'taggable' key" do it "should return false" do page = build(:alchemy_page) - allow(page).to receive(:definition).and_return({"name" => "standard"}) + allow(page).to receive(:definition).and_return({ "name" => "standard" }) expect(page.taggable?).to be_falsey end end @@ -1644,11 +1639,11 @@ module Alchemy end context "urlname updating" do - let(:parentparent) { create(:alchemy_page, name: "parentparent", visible: true) } - let(:parent) { create(:alchemy_page, parent_id: parentparent.id, name: "parent", visible: true) } - let(:page) { create(:alchemy_page, parent_id: parent.id, name: "page", visible: true) } - let(:invisible) { create(:alchemy_page, parent_id: page.id, name: "invisible", visible: false) } - let(:contact) { create(:alchemy_page, parent_id: invisible.id, name: "contact", visible: true) } + let(:parentparent) { create(:alchemy_page, name: "parentparent", visible: true) } + let(:parent) { create(:alchemy_page, parent_id: parentparent.id, name: "parent", visible: true) } + let(:page) { create(:alchemy_page, parent_id: parent.id, name: "page", visible: true) } + let(:invisible) { create(:alchemy_page, parent_id: page.id, name: "invisible", visible: false) } + let(:contact) { create(:alchemy_page, parent_id: invisible.id, name: "contact", visible: true) } let(:language_root) { parentparent.parent } context "with activated url_nesting" do @@ -1858,7 +1853,7 @@ module Alchemy describe "#status" do it "returns a combined status hash" do - expect(page.status).to eq({public: true, visible: true, restricted: false, locked: false}) + expect(page.status).to eq({ public: true, visible: true, restricted: false, locked: false }) end end @@ -2030,7 +2025,7 @@ module Alchemy describe "#published_at" do context "with published_at date set" do let(:published_at) { 3.days.ago } - let(:page) { build_stubbed(:alchemy_page, published_at: published_at) } + let(:page) { build_stubbed(:alchemy_page, published_at: published_at) } it "returns the published_at value from database" do expect(page.published_at).to be_within(1.second).of(published_at) @@ -2039,7 +2034,7 @@ module Alchemy context "with published_at is nil" do let(:updated_at) { 3.days.ago } - let(:page) { build_stubbed(:alchemy_page, published_at: nil, updated_at: updated_at) } + let(:page) { build_stubbed(:alchemy_page, published_at: nil, updated_at: updated_at) } it "returns the updated_at value" do expect(page.published_at).to be_within(1.second).of(updated_at) @@ -2093,8 +2088,7 @@ module Alchemy it "returns content ids for all expanded nested elements that have tinymce enabled" do expanded_rtf_contents = expanded_element.contents.essence_richtexts nested_expanded_rtf_contents = nested_expanded_element.contents.essence_richtexts - rtf_content_ids = expanded_rtf_contents.pluck(:id) + - nested_expanded_rtf_contents.pluck(:id) + rtf_content_ids = expanded_rtf_contents.pluck(:id) + nested_expanded_rtf_contents.pluck(:id) expect(richtext_contents_ids.sort).to eq(rtf_content_ids) nested_folded_rtf_content = nested_folded_element.contents.essence_richtexts.first From c52eea672c7b0c713fc2b11bfec706708be3babc Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 5 May 2020 19:21:24 +0200 Subject: [PATCH 4/4] Add upgrader for removing layout root pages --- lib/alchemy/upgrader/five_point_zero.rb | 13 +++++++++++++ lib/tasks/alchemy/upgrade.rake | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/lib/alchemy/upgrader/five_point_zero.rb b/lib/alchemy/upgrader/five_point_zero.rb index 6ddcc6d2b6..1df16b74e3 100644 --- a/lib/alchemy/upgrader/five_point_zero.rb +++ b/lib/alchemy/upgrader/five_point_zero.rb @@ -11,6 +11,19 @@ def install_gutentag_migrations Alchemy::Upgrader::Tasks::HardenGutentagMigrations.new.patch_migrations `bundle exec rake db:migrate` end + + def remove_layout_roots + desc "Remove layout root pages" + layout_roots = Alchemy::Page.where(layoutpage: true).where("name LIKE 'Layoutroot for%'") + if layout_roots.size.positive? + log "Removing #{layout_roots.size} layout root pages." + layout_roots.delete_all + Alchemy::Page.where(layoutpage: true).update_all(parent_id: nil) + log "Done.", :success + else + log "No layout root pages found.", :skip + end + end end end end diff --git a/lib/tasks/alchemy/upgrade.rake b/lib/tasks/alchemy/upgrade.rake index 5a499fbbcb..5a53e3658b 100644 --- a/lib/tasks/alchemy/upgrade.rake +++ b/lib/tasks/alchemy/upgrade.rake @@ -20,6 +20,7 @@ namespace :alchemy do desc "Alchemy Upgrader: Prepares the database." task database: [ "alchemy:upgrade:5.0:install_gutentag_migrations", + "alchemy:upgrade:5.0:remove_layout_roots", "alchemy:install:migrations", "db:migrate", "alchemy:db:seed", @@ -42,6 +43,11 @@ namespace :alchemy do task install_gutentag_migrations: [:environment] do Alchemy::Upgrader::FivePointZero.install_gutentag_migrations end + + desc "Remove layout root pages" + task remove_layout_roots: [:environment] do + Alchemy::Upgrader::FivePointZero.remove_layout_roots + end end end end