Skip to content

Commit

Permalink
Fix Zeitwerk conventions for Refinery::Pages::Finder (#3524)
Browse files Browse the repository at this point in the history
Resolves #3523

The issue stems from the definition of `Refinery::Pages::Finder` and its sibling classes (`Refinery::Pages::FinderByTitle` and so on). These are all defined beside each other in [`pages/lib/refinery/pages/finder.rb`](https://github.com/refinery/refinerycms/blob/aee49a603860bf7d5fdb1532b9add5f4e7f839f4/pages/lib/refinery/pages/finder.rb) which gets required in [`pages/app/models/refinery/page.rb`](https://github.com/refinery/refinerycms/blob/aee49a603860bf7d5fdb1532b9add5f4e7f839f4/pages/app/models/refinery/page.rb#L6).

When reloading, Zeitwerk reloads the model, which in turn loads the finder again. But because the file is named `finder.rb`, only `Refinery::Pages::Finder` gets unloaded and reloaded again. All other classes stay loaded and would be patched by the reload. But because `Refinery::Pages::Finder` has been reloaded and initialized again, it got a new `object_id`, thus triggering the **`superclass mismatch`** error.

The solution, that fixed the error, was to move all sibling-classes of `Refinery::Pages::Finder` into it, making them children of `Refinery::Pages::Finder`. Because they are only called inside `Refinery::Pages::Finder` itself, this *should* not make problems.
  • Loading branch information
richard-kramer committed May 12, 2023
1 parent 4930062 commit 5cbbd14
Showing 1 changed file with 87 additions and 88 deletions.
175 changes: 87 additions & 88 deletions pages/lib/refinery/pages/finder.rb
Expand Up @@ -57,123 +57,122 @@ def translations_conditions(original_conditions)
translations_conditions
end

end
class FinderByTitle < Finder
def initialize(title)
@title = title
@conditions = default_conditions
end

class FinderByTitle < Finder
def initialize(title)
@title = title
@conditions = default_conditions
end
def default_conditions
{ :title => title }
end

def default_conditions
{ :title => title }
private
attr_accessor :title
end

private
attr_accessor :title
end
class FinderBySlug < Finder
def initialize(slug, conditions)
@slug = slug
@conditions = default_conditions.merge(conditions)
end

class FinderBySlug < Finder
def initialize(slug, conditions)
@slug = slug
@conditions = default_conditions.merge(conditions)
end
def default_conditions
{
:locale => Refinery::I18n.frontend_locales.map(&:to_s),
:slug => slug
}
end

def default_conditions
{
:locale => Refinery::I18n.frontend_locales.map(&:to_s),
:slug => slug
}
private
attr_accessor :slug
end

private
attr_accessor :slug
end

class FinderByPath
def initialize(path)
@path = path
end
class FinderByPath
def initialize(path)
@path = path
end

def find
if slugs_scoped_by_parent?
FinderByScopedPath.new(path).find
else
FinderByUnscopedPath.new(path).find
def find
if slugs_scoped_by_parent?
FinderByScopedPath.new(path).find
else
FinderByUnscopedPath.new(path).find
end
end
end

private
attr_accessor :path
private
attr_accessor :path

def slugs_scoped_by_parent?
::Refinery::Pages.scope_slug_by_parent
end
def slugs_scoped_by_parent?
::Refinery::Pages.scope_slug_by_parent
end

def by_slug(slug_path, conditions = {})
Finder.by_slug(slug_path, conditions)
def by_slug(slug_path, conditions = {})
Finder.by_slug(slug_path, conditions)
end
end
end

class FinderByScopedPath < FinderByPath
def find
# With slugs scoped to the parent page we need to find a page by its full path.
# For example with about/example we would need to find 'about' and then its child
# called 'example' otherwise it may clash with another page called /example.
page = parent_page
while page && path_segments.any? do
page = next_page(page)
class FinderByScopedPath < FinderByPath
def find
# With slugs scoped to the parent page we need to find a page by its full path.
# For example with about/example we would need to find 'about' and then its child
# called 'example' otherwise it may clash with another page called /example.
page = parent_page
while page && path_segments.any? do
page = next_page(page)
end
page
end
page
end

private
private

def path_segments
@path_segments ||= path.split('/').select(&:present?)
end
def path_segments
@path_segments ||= path.split('/').select(&:present?)
end

def parent_page
parent_page_segment = path_segments.shift
if parent_page_segment.friendly_id?
by_slug(parent_page_segment, :parent_id => nil).first
else
Page.find(parent_page_segment)
def parent_page
parent_page_segment = path_segments.shift
if parent_page_segment.friendly_id?
by_slug(parent_page_segment, :parent_id => nil).first
else
Page.find(parent_page_segment)
end
end
end

def next_page(page)
slug_or_id = path_segments.shift
page.children.by_slug(slug_or_id).first || page.children.find(slug_or_id)
def next_page(page)
slug_or_id = path_segments.shift
page.children.by_slug(slug_or_id).first || page.children.find(slug_or_id)
end
end
end

class FinderByUnscopedPath < FinderByPath
def find
by_slug(path).first
class FinderByUnscopedPath < FinderByPath
def find
by_slug(path).first
end
end
end

class FinderByPathOrId
def initialize(path, id)
@path = path
@id = id
end
class FinderByPathOrId
def initialize(path, id)
@path = path
@id = id
end

def find
if path.present?
if path.friendly_id?
FinderByPath.new(path).find
else
Page.friendly.find(path)
def find
if path.present?
if path.friendly_id?
FinderByPath.new(path).find
else
Page.friendly.find(path)
end
elsif id.present?
Page.friendly.find(id)
end
elsif id.present?
Page.friendly.find(id)
end
end

private
attr_accessor :id, :path
private
attr_accessor :id, :path
end
end
end
end

0 comments on commit 5cbbd14

Please sign in to comment.