SpinaCMS/Spina

Page has #slug db column, but also a #slug method, so it's never set?

chiperific opened this issue · 6 comments

From migration:
https://github.com/SpinaCMS/Spina/blob/main/db/migrate/1_create_spina_tables.rb#L63

From the model:
https://github.com/SpinaCMS/Spina/blob/main/app/models/spina/page.rb#L57

The docs mention a background job that gets run when a slug is set for a resource, but I don't see anything in the code base for page updates. It seems like all the path work is handled via #materialize_path

I can manually set the slug, but it doesn't seem to ever be programmatically set. Is this accurate / intended?

What is the purpose of the Spina::Page#slug field?


Bigger context: There are a few places in the gem that just rely on finding a Page with the name "homepage", but there's nothing explicit in the instructions about making sure to have a page named "homepage", so of course I tried to break a custom theme by not creating any page named "homepage". :)
Which of course lead me to seeing what else made things weird.

# config/initializers/themes/mytheme.rb
...
theme.custom_pages = [
    # normal, expected cases:
    { name: 'offerings', title: 'Offerings', deletable: false, view_template: 'offerings' },
    { name: 'contact', title: 'Contact', deletable: false, view_template: 'contact' },
    { name: 'about', title: 'About', deletable: false, view_template: 'about' },
    # bad, weird, strange cases:
    { name: 'root', title: 'Root', deletable: false, view_template: 'home', homepage: true },
    { name: '', title: '', view_template: 'about' },
    { name: 'admin', title: 'Admin', view_template: 'root' },
    { name: '404', title: '404', view_template: 'about' },
    { name: 'no_template', title: 'No Template', view_template: '' }
  ]
...
> ary = []
> Spina::Page.all.map { |page| ary << { page.name => page.materialized_path } }
> ary.map { |i| puts i }
=> {"about"=>"/about"},  
  {"root"=>"/root"},    
  {"admin"=>"/admin"},  
  {"404"=>"/404"},      
  {"no_template"=>"/no-template"},
  {"offerings"=>"/offerings"},
  {"contact"=>"/contact"}

The nice thing is my custom '/admin' page didn't break the engine.

I'm working on a PR that will bake in "homepage" standards a bit more directly.

This is awesome! Thanks for digging in.

The slug field on the page model is a relic from the past that hasn’t been cleaned up yet.

Named/custom pages are there to make it easy to do Spina::Page.find_by(name: “some_name”), nothing more.

Thanks!

Might be a separate issue, but I do think there's a challenge with how brittle "homepage" is.
E.g.
Spina::Frontend#page
Spina::Page#homepage?

Here are my ideas (increasing in intensity):

  1. Document in the setup guide (and in the demo & default initializers) that a page named "homepage" is needed to serve as the root page for the engine, either as a theme.custom_page or made from the Admin portal
  2. Have Spina::Account.after_save :bootstrap_website ensure the existence/creation of a Page named "homepage" (and a view template)
  3. Add attributes to Page so that a page record can be identified as the homepage of a Theme (and thus routed correctly by Frontend and Babosa via #materialize_path

I'm a good few hours into idea 3 and it's shaping up nicely, but I don't want to spend another few hours on a PR you're not interested in.

I agree that the current code is inelegant and feels brittle. That said, I don't recall this ever being an issue where people accidentally removed their homepage. There's an undocumented rule that you have to have a custom page called homepage, and I think it simply hasn't occurred that someone wanted to name it differently.

We can definitely improve this. Out of your ideas I like option 2 best as option 3 feels a bit too overengineered. What would implementing option 3 allow us to do that we can't do now?

The changes I've been working on do 2 main things:

1. Associate pages with themes

  • Multi-theme apps shouldn't necessarily share pages.
  • Navigations have no concise way of limiting pages to the active themes.

2. Mark a page as the homepage for a theme

  • Multi-theme apps shouldn't have to have the same homepage record and template.
  • The User should be able to identify which page is the homepage for the current theme.

I add Page#is_homepage? and I store some things in the Page#json_attributes field:

#<Spina::Page:0x0000000108209938                                   
 id: 22,                                                           
 show_in_menu: true,                                               
 deletable: false,                                                 
 name: "root",                                                     
 skip_to_first_child: false,                                       
 view_template: "home",                                            
 layout_template: nil,                                             
 draft: false,                                                     
 link_url: nil,                                                    
 ancestry: nil,
 position: 4,
 active: false,
 resource_id: nil,
 json_attributes: {"themes"=>["mommyblog", "demo", "sweetsshop"], "en_content"=>[], "homepage_for_themes"=>["mommyblog"]},
 is_homepage: true> 

I re-work Spina::Frontend#page and add a few methods and a scope to Spina::Page