Skip to content

Commit

Permalink
Use the already loaded page to find elements
Browse files Browse the repository at this point in the history
The former implementation loads a new page from the database,
instead of using the already loaded page from the controller.

Only load a page if we pass a page layout string.

This way we can take advantage of eager loading and reduce the amount
of objects in memory
  • Loading branch information
tvdeyen committed Nov 18, 2019
1 parent 12cc75c commit a87adf2
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 19 deletions.
31 changes: 17 additions & 14 deletions lib/alchemy/elements_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,15 @@ def elements(page:)

attr_reader :page, :options

def find_elements(page)
elements = Alchemy::Element
.where(page_id: page_ids(page))
.merge(Alchemy::Element.not_nested)
.where(fixed: !!options[:fixed])
.order(position: :asc)
.available
def find_elements(page_or_layout)
@page = get_page(page_or_layout)
return Alchemy::Element.none unless page

if options[:fixed]
elements = page.fixed_elements
else
elements = page.elements
end

if options[:only]
elements = elements.named(options[:only])
Expand All @@ -76,15 +78,16 @@ def find_elements(page)
elements
end

def page_ids(page)
case page
def get_page(page_or_layout)
case page_or_layout
when Alchemy::Page
page_or_layout
when String
Alchemy::Language.current.pages.where(
page_layout: page,
Alchemy::Page.find_by(
language: Alchemy::Language.current,
page_layout: page_or_layout,
restricted: false
).pluck("#{Alchemy::Page.table_name}.id")
when Alchemy::Page
page.id
)
end
end

Expand Down
14 changes: 9 additions & 5 deletions spec/libraries/elements_finder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,16 @@
let!(:visible_element) { create(:alchemy_element, public: true, page: page) }
let!(:hidden_element) { create(:alchemy_element, public: false, page: page) }

let(:page_2) { create(:alchemy_page, :public, page_layout: 'standard') }
let!(:visible_element_2) { create(:alchemy_element, public: true, page: page_2) }
let!(:hidden_element_2) { create(:alchemy_element, public: false, page: page_2) }
it 'returns all public elements from page with given page layout' do
is_expected.to eq([visible_element])
end

it 'returns all public elements from all pages with given page layout' do
is_expected.to eq([visible_element, visible_element_2])
context 'that is not found' do
subject { finder.elements(page: 'foobaz') }

it 'returns empty active record relation' do
is_expected.to eq(Alchemy::Element.none)
end
end
end

Expand Down

0 comments on commit a87adf2

Please sign in to comment.