SpinaCMS / spina-blog

Blog engine for Spina CMS
MIT License
11 stars 25 forks source link

custom view file location #39

Closed aseroff closed 1 year ago

aseroff commented 1 year ago

I'm using the changes in this pull request in my live application and figured I'd see if you wanted to add them to the project. This allows spina view files to be unified into a blog directory under app/views/<theme>/ instead of having custom blog views in a separate app/views/spina/blog/ location. It also respects a layout_template attribute on the blog's Spina::Page record, which it previously did not do.

There may be a cleaner way to accomplish all this that I'm not aware of, hooking into SpinaCMS's engine code, which if true, I'm sure would be preferable.

aseroff commented 1 year ago

It looks like the Spina frontend concern has a similar method, but /pages/ is hardcoded into the path, so it can't be reused for the blog without making changes to the Spina project, so I feel like handling it here in this way is probably fine, unless you wanted to me to abstract it into an extension of the frontend concern.

wakproductions commented 1 year ago

@simmerz I have reason to believe this breaks existing programs out there using spina-blog

[9accc4bb-b172-4560-9335-45330254fc52] ActionView::MissingTemplate (Missing template default/blog/posts/index with {:locale=>[:en], :formats=>[:html], :variants=>[], :handlers=>[:raw, :erb, :html, :builder, :ruby, :slim]}.

Searched in:
  * "/app/app/views"
  * "/usr/local/bundle/bundler/gems/spina-blog-3f29dbbd8070/app/views"
  * "/usr/local/bundle/bundler/gems/Spina-dd8a1a214af0/app/views"
  * "/usr/local/bundle/gems/view_component-3.6.0/app/views"
  * "/usr/local/bundle/gems/kaminari-core-1.2.2/app/views"
  * "/usr/local/bundle/gems/actiontext-7.0.7.2/app/views"
  * "/usr/local/bundle/gems/actionmailbox-7.0.7.2/app/views"

Did you mean?  spina/admin/blog/posts/index
               spina/blog/posts/index
               spina/admin/page_select_options/index
               spina/admin/blog/categories/index
               rails/conductor/action_mailbox/inbound_emails/index
               spina/admin/users/index):

Users should be made aware that to fix this, the templates have to be moved from views/spina/blog/ to view/<theme (usually "default")>/blog/. Maybe it should search both paths for the template file to maintain backward compatibility?

simmerz commented 1 year ago

That's a good shout. I hadn't considered that when merging it in

simmerz commented 1 year ago

Perhaps this would be better done with prepend_view_path

aseroff commented 1 year ago

Sorry for introducing this breaking change!

Perhaps this would be better done with prepend_view_path

I tried this approach, however, the rendered view defaults to spina/blog/_model_/_action_ so my desire to just have blog views in app/views/_theme_/blog is not solved by prepending the theme into the searched view paths (because of the /spina/ directory). Continuing to specify the rendered view without the leading spina is an option, but then would need to be added to the end of the view_path that checks the spina-blog gem, which I wasn't able to do.

simmerz commented 1 year ago

@aseroff could you add a PR that does a search for the template in the old location if the new one doesn't exist and add a deprecation for that? We'll remove it in a future release, but I'd rather not make a hard breaking change at this point.

aseroff commented 1 year ago

@simmerz https://github.com/SpinaCMS/spina-blog/pull/47