Page menu & allowed children
josh french
back in and trying to update Page Factory to work with the 1.0.0
candidate. What's the thinking behind the allowed_children_cache field
on the page model? Since that only gets updated on save, there's no
way to introduce new page classes into the menu on the fly -- unless
I'm overlooking something. Is there a way to add options to the add-
child menu without having to update each existing page record first?
Jim Gay
I did a major no-no by refactoring this during the release candidate
process, so that's strike one against me.
There were 2 things that I tried to tackle with changes to the "Add Child" menu:
1) Pages should be allowed to limit their children of certain types,
like "Archive" which only needs 1 year/month/day index.
2) We had a lot of menu-controlling code in models, helpers, and views
and it just felt messy.
When I first attempted to solve #1 the code would read a lot of data
from the database; this drastically slowed things down on sites with
large numbers of pages.
#2 is solved by extending each page object with the MenuRenderer
module (and any special modules that exist for the page type, such as
ArchiveMenuRenderer)
I wanted to do a more serious refactoring of our views with decorators
but opted to cut some things to get a release out.
So I stuck with using the "render_node" method in our node_helper.rb
and extending the page with a module containing the methods needed.
https://github.com/radiant/radiant/blob/master/app/helpers/admin/node_helper.rb#L7
The downside of this is that it makes it a little harder to
metaprogram changes into the menu, but that could be solved by
providing a simple hook for additional features.
Are you merely trying to add another page type to the list after the
database already has the cache without that item? Or is there more as
well?
-Jim
--
Write intention revealing code #=> http://www.clean-ruby.com
Jim Gay
Saturn Flyer LLC
571-403-0338
jsntv200
project I was working on. It still seems to be working fine with rc4
but I haven't tested or done anything with it recently. Was going to
send a pull request but got sidetracked.
https://github.com/jsntv200/radiant-page_factory-extension/tree/gemspec
Check it out and let me know if gets things moving forward for you.
cheers
Jason
josh french
> database already has the cache without that item?
with existing pages and decided to add any extension containing a new
Page class, the DB cache wouldn't reflect the addition.
I totally agree with your points #1 and #2, but in the interest of
seeing a release go out (and having something stable to develop
against,) I'd be fine if we decided to back out some of the advanced
menu features and just went with a simple-and-stupid first pass.
On Jan 17, 3:06 pm, Jim Gay <j...@saturnflyer.com> wrote:
> This is all my work, so any problems here are caused by me.
>
> I did a major no-no by refactoring this during the release candidate
> process, so that's strike one against me.
>
> There were 2 things that I tried to tackle with changes to the "Add Child" menu:
> 1) Pages should be allowed to limit their children of certain types,
> like "Archive" which only needs 1 year/month/day index.
> 2) We had a lot of menu-controlling code in models, helpers, and views
> and it just felt messy.
>
> When I first attempted to solve #1 the code would read a lot of data
> from the database; this drastically slowed things down on sites with
> large numbers of pages.
> #2 is solved by extending each page object with the MenuRenderer
> module (and any special modules that exist for the page type, such as
> ArchiveMenuRenderer)
>
> I wanted to do a more serious refactoring of our views with decorators
> but opted to cut some things to get a release out.
> So I stuck with using the "render_node" method in our node_helper.rb
> metaprogram changes into the menu, but that could be solved by
> providing a simple hook for additional features.
>
> Are you merely trying to add another page type to the list after the
> database already has the cache without that item? Or is there more as
> well?
>
> -Jim
>
josh french
that would call the set_allowed_children_cache method on each page at
app startup, in order to bust the cache and load in any newly-
installed page subclasses. But I realized that would trigger the other
before_save hooks and modify every page's timestamp, which seems like
it could be a trainwreck.
Keeping the allowed_children_cache in the DB breaks the development
process for anyone who works with custom page classes and feels like a
dead end to me, so I'd like to propose an alternate implementation:
storing the allowed-children as a class-inheritable variable.
* The allowed_children_cache column is replaced with a class-
inheritable variable.
* At app startup, we set Page@@allowed_children_cache =
[default_child, *Page.descendants].
* Add a Page.allowed_children_cache method which simply reads the
class variable. Page subclasses could modify their inherited
@@allowed_children_cache, or overwrite this method entirely.
* Page#allowed_children_cache calls self.class.allowed_children.
This means that in the base case, each page instance is still looking
at a cached value -- but it would remain open to modification at the
class or instance level.
For instance: at the class level, the ArchiveYearIndexPage class could
read the inherited @@allowed_children_cache and exclude itself,
preventing nested yearly archives. At the instance level, a yearly
archive page could check to see if it already has an
ArchiveMonthlyIndexPage child and exclude it if so, preventing sibling
monthly archives.
It would even be possible to have Page.allowed_children_cache ignore
the cache in dev mode while respecting it in production, although that
might belong in something like PageFactory rather than core.
Thoughts or comments?