diff --git a/app/controllers/alchemy/admin/pages_controller.rb b/app/controllers/alchemy/admin/pages_controller.rb index e0b62e681a..c74be87dc3 100644 --- a/app/controllers/alchemy/admin/pages_controller.rb +++ b/app/controllers/alchemy/admin/pages_controller.rb @@ -324,7 +324,7 @@ def process_url(ancestors_path, item) end def load_resource - @page = Page.find(params[:id]) + @page = Page.includes(page_includes).find(params[:id]) end def pages_from_raw_request @@ -402,6 +402,10 @@ def load_languages_and_layouts def set_preview_mode @preview_mode = true end + + def page_includes + Alchemy::EagerLoading.page_includes(version: :draft_version) + end end end end diff --git a/app/controllers/alchemy/pages_controller.rb b/app/controllers/alchemy/pages_controller.rb index 15696acfd6..d92f4e4ea8 100644 --- a/app/controllers/alchemy/pages_controller.rb +++ b/app/controllers/alchemy/pages_controller.rb @@ -104,7 +104,14 @@ def locale_prefix_not_allowed? # If no index page and no admin users are present we show the "Welcome to Alchemy" page. # def load_index_page - @page ||= Language.current_root_page + @page ||= begin + Alchemy::Page. + contentpages. + language_roots. + where(language: Language.current). + includes(page_includes). + first + end render template: "alchemy/welcome", layout: false if signup_required? end @@ -120,10 +127,13 @@ def load_index_page def load_page page_not_found! unless Language.current - @page ||= Language.current.pages.contentpages.find_by( - urlname: params[:urlname], - language_code: params[:locale] || Language.current.code, - ) + @page ||= begin + Alchemy::Page. + contentpages. + where(language: Language.current). + includes(page_includes). + find_by(urlname: params[:urlname]) + end end def enforce_locale @@ -234,5 +244,9 @@ def must_not_cache? def page_not_found! not_found_error!("Alchemy::Page not found \"#{request.fullpath}\"") end + + def page_includes + Alchemy::EagerLoading.page_includes(version: :public_version) + end end end diff --git a/app/jobs/alchemy/publish_page_job.rb b/app/jobs/alchemy/publish_page_job.rb index afe3084b98..d06f5d8284 100644 --- a/app/jobs/alchemy/publish_page_job.rb +++ b/app/jobs/alchemy/publish_page_job.rb @@ -4,7 +4,10 @@ module Alchemy class PublishPageJob < BaseJob queue_as :default - def perform(page, public_on:) + def perform(page_id, public_on:) + page = Alchemy::Page.includes( + Alchemy::EagerLoading.page_includes(version: :draft_version) + ).find(page_id) Alchemy::Page::Publisher.new(page).publish!(public_on: public_on) end end diff --git a/app/models/alchemy/eager_loading.rb b/app/models/alchemy/eager_loading.rb new file mode 100644 index 0000000000..8735d76e38 --- /dev/null +++ b/app/models/alchemy/eager_loading.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Alchemy + # Eager loading parameters for loading pages + class EagerLoading + PAGE_VERSIONS = %i[draft_version public_version] + + class << self + # Eager loading parameters for {ActiveRecord::Base.includes} + # + # Pass this to +includes+ whereever you load an {Alchemy::Page} + # + # Alchemy::Page.includes(Alchemy::EagerLoading.page_includes).find_by(urlname: "my-page") + # + # @param version [Symbol] Type of page version to eager load + # @return [Array] + def page_includes(version: :public_version) + raise UnsupportedPageVersion unless version.in? PAGE_VERSIONS + + [ + :tags, + { + language: :site, + version => { + elements: [ + :page, + :touchable_pages, + { + ingredients: :related_object, + contents: :essence, + }, + ], + }, + }, + ] + end + end + end +end diff --git a/app/models/alchemy/page.rb b/app/models/alchemy/page.rb index d7bfd7f67f..284e79e7fb 100644 --- a/app/models/alchemy/page.rb +++ b/app/models/alchemy/page.rb @@ -465,7 +465,7 @@ def copy_children_to(new_parent) # def publish!(current_time = Time.current) update(published_at: current_time) - PublishPageJob.perform_later(self, public_on: current_time) + PublishPageJob.perform_later(id, public_on: current_time) end # Sets the public_on date on the published version diff --git a/app/models/alchemy/page_version.rb b/app/models/alchemy/page_version.rb index e17c5cf10a..2c84c63d28 100644 --- a/app/models/alchemy/page_version.rb +++ b/app/models/alchemy/page_version.rb @@ -46,7 +46,7 @@ def still_public_for?(time = Time.current) end def element_repository - ElementsRepository.new(elements.includes({ contents: :essence }, :tags)) + ElementsRepository.new(elements) end private diff --git a/lib/alchemy/errors.rb b/lib/alchemy/errors.rb index 2f9caf3ca5..1041aa8c07 100644 --- a/lib/alchemy/errors.rb +++ b/lib/alchemy/errors.rb @@ -11,7 +11,7 @@ class DefaultLanguageNotFoundError < StandardError # Raised if no default language configuration can be found. def message "No default language configuration found!" \ - " Please ensure that you have a 'default_language' defined in Alchemy configuration file." + " Please ensure that you have a 'default_language' defined in Alchemy configuration file." end end @@ -19,7 +19,7 @@ class DefaultSiteNotFoundError < StandardError # Raised if no default site configuration can be found. def message "No default site configuration found!" \ - " Please ensure that you have a 'default_site' defined in Alchemy configuration file." + " Please ensure that you have a 'default_site' defined in Alchemy configuration file." end end @@ -90,4 +90,10 @@ def message "You need to provide a current_user method in your ApplicationController that returns the current authenticated user." end end + + class UnsupportedPageVersion < StandardError + def message + "Unknown Version! Please use one of #{Alchemy::EagerLoading::PAGE_VERSIONS.join(", ")}" + end + end end diff --git a/spec/jobs/alchemy/publish_page_job_spec.rb b/spec/jobs/alchemy/publish_page_job_spec.rb index cc8cecca80..370f6b2bbd 100644 --- a/spec/jobs/alchemy/publish_page_job_spec.rb +++ b/spec/jobs/alchemy/publish_page_job_spec.rb @@ -4,12 +4,12 @@ RSpec.describe Alchemy::PublishPageJob, type: :job do describe "#perfom_later" do - let(:page) { build_stubbed(:alchemy_page) } + let(:page) { create(:alchemy_page) } let(:public_on) { Time.current } it "enqueues job" do expect { - described_class.perform_later(page, public_on: public_on) + described_class.perform_later(page.id, public_on: public_on) }.to have_enqueued_job end @@ -17,7 +17,7 @@ expect_any_instance_of(Alchemy::Page::Publisher).to receive(:publish!).with( public_on: public_on, ) - described_class.new.perform(page, public_on: public_on) + described_class.new.perform(page.id, public_on: public_on) end end end diff --git a/spec/models/alchemy/eager_loading_spec.rb b/spec/models/alchemy/eager_loading_spec.rb new file mode 100644 index 0000000000..4adb3bf4ef --- /dev/null +++ b/spec/models/alchemy/eager_loading_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Alchemy::EagerLoading do + describe ".page_includes" do + context "with no version param given" do + subject { described_class.page_includes } + + it "returns public version includes" do + is_expected.to match_array([ + :tags, + { + language: :site, + public_version: { + elements: [ + :page, + :touchable_pages, + { + ingredients: :related_object, + contents: :essence, + }, + ], + }, + }, + ]) + end + end + + context "with version param given" do + subject { described_class.page_includes(version: :draft_version) } + + it "returns version includes" do + is_expected.to match_array([ + :tags, + { + language: :site, + draft_version: { + elements: [ + :page, + :touchable_pages, + { + ingredients: :related_object, + contents: :essence, + }, + ], + }, + }, + ]) + end + end + + context "with unknown version param given" do + subject { described_class.page_includes(version: :foo_baz) } + + it "returns version includes" do + expect { subject }.to raise_error(Alchemy::UnsupportedPageVersion) + end + end + end +end diff --git a/spec/requests/alchemy/admin/pages_controller_spec.rb b/spec/requests/alchemy/admin/pages_controller_spec.rb index 2f2a5b78b9..f7da85b863 100644 --- a/spec/requests/alchemy/admin/pages_controller_spec.rb +++ b/spec/requests/alchemy/admin/pages_controller_spec.rb @@ -643,45 +643,44 @@ module Alchemy describe "#fold" do let(:page) { create(:alchemy_page) } + let(:user) { create(:alchemy_dummy_user, :as_editor) } before do - allow(Page).to receive(:find).and_return(page) - allow(page).to receive(:editable_by?).with(user).and_return(true) + allow_any_instance_of(described_class).to receive(:current_alchemy_user) { user } end - context "if page is currently not folded" do - before { allow(page).to receive(:folded?).and_return(false) } + subject { patch fold_admin_page_path(page), xhr: true } + context "if page is currently not folded" do it "should fold the page" do - expect(page).to receive(:fold!).with(user.id, true).and_return(true) - patch fold_admin_page_path(page), xhr: true + expect { subject }.to change { page.folded?(user.id) }.from(false).to(true) end end context "if page is already folded" do - before { allow(page).to receive(:folded?).and_return(true) } + before do + page.fold!(user.id, true) + end it "should unfold the page" do - expect(page).to receive(:fold!).with(user.id, false).and_return(true) - patch fold_admin_page_path(page), xhr: true + expect { subject }.to change { page.folded?(user.id) }.from(true).to(false) end end end describe "#unlock" do - subject { post unlock_admin_page_path(page), xhr: true } + let(:page) { create(:alchemy_page) } + let(:user) { create(:alchemy_dummy_user, :as_editor) } - let(:page) { create(:alchemy_page, name: "Best practices") } + subject { post unlock_admin_page_path(page), xhr: true } before do - allow(Page).to receive(:find).with(page.id.to_s).and_return(page) - allow(page).to receive(:editable_by?).with(user).and_return(true) - allow(Page).to receive(:from_current_site).and_return(double(locked_by: nil)) - expect(page).to receive(:unlock!) { true } + page.lock_to!(user) + allow_any_instance_of(described_class).to receive(:current_alchemy_user) { user } end it "should unlock the page" do - is_expected.to eq(200) + expect { subject }.to change { page.reload.locked? }.from(true).to(false) end context "requesting for html format" do