Fixes #13235 - content from application layout extracted into a partial

[tstrachota]

Extracting the content_for part of application layout into a separate partial will enable for using in with different parent layouts. This is key for rendering the standard foreman views with layout containing bastion_katello assets.

Intended usage:

# katello/layouts/foreman_with_bastion.html.erb
<%= content_for(:content) do %>
  <%= render :partial => 'layouts/application_content' %>
<% end %>

<%= render :file => 'bastion/layouts/assets' %>

[tstrachota]

[test]

[ohadlevy]

what's the alternative? deface ? if you already use content_for, cant you simply replace the content div? or not use the layout in the first place? I guess I'm not 100% clear on how its used / why.

[tstrachota]

@ohadlevy I actually want to reuse contents of the content_for in a different layout. Please see https://github.com/Katello/katello/pull/5710/files#diff-6af49641613a044997bb75949b885104 for example usage.

[dLobatog]

Can't you use deface and:

Deface::Override.new(
  :virtual_path => 'layouts/application',
  :name => 'bastion_assets',
  :insert_after => 'erb[loud]:contains("layouts/base")',
  :text => "<%= render :file => 'bastion/layouts/assets' %>")

? I just tried and it loads the assets just fine. It can be done before the content_for too if you need that.

[dLobatog]

@tstrachota I replied inline with an example on how to do this with deface, you say you want to use application_content with another wrapper, but I don't see that on https://github.com/Katello/katello/pull/5710/files#diff-6af49641613a044997bb75949b885104 ?

If it's just to add the assets, I don't think it's needed. If you want to use a different wrapper that's not content_tag(:content), can't you use Deface's :surround and match the main div?

[tstrachota]

@dLobatog https://github.com/Katello/katello/pull/5710/files#diff-6af49641613a044997bb75949b885104 uses foreman's original content_for in bastion's asset layout. You got it right, it's there just to add bastion and bastion_katello assets.

I played with deface too but without any success. I'm not any deface expert so I have couple of questions you could help me with:

• Did you try your example with installed katello? When bastion_katello is added to a page with url that doesn't match any of the defined angular states it keeps reloading it (probably bug in bastion). Did you encounter such behaviour?
• I was able to replace the layout for the whole application (:virtual_path => 'layouts/application') but I'm not sure how to limit it only to smart_proxies/show. Without that you get the issue from the previous bullet.

Although I don't understand the benefits of using deface over splitting the partials, I'm happy to throw this PR away if someone helps me to find a working deface solution.

[tstrachota]

Just for the record, on the meeting yesterday we talked about using javascript helper instead of custom layout. I don't think that's a good idea since there's quite some in-place js code https://github.com/Katello/bastion/blob/master/app/views/bastion/layouts/assets.html.erb

[dLobatog]

@tstrachota The problem I see on the Katello PR is that it goes as far as to require a controller extension just to add the bastion assets. Aside from that, I worry that thinking we cannot add assets & disable stuff via deface lead us to maintain things like a "database of pages where turbolinks has top be disabled": #3086 -
The burden of adding this stuff should be primarily on the plugin developer, not on core I'd say. Especially when deface allows to do that simply. Making these conventions in core also tie core with plugins so much that it'll be very difficult to get rid of them if we need to.

You can add it just on smart_proxies/show with something like this:

Deface::Override.new(
  :virtual_path => 'smart_proxies/show',
  :name => 'bastion_assets',
  :insert_after => '.proxy-show',
  :text => "<%= render :file => 'bastion/layouts/assets' %>")

In case you need your assets to be loaded after the whole layout (no need for it I think?):

Deface::Override.new(
  :virtual_path => 'layouts/application',
  :name => 'bastion_assets',
  :insert_after => 'erb[loud]:contains("layouts/base")',
  :text => "<% if current_page?(:controller => 'smart_proxies',
                                :action => 'show', 
                                :id => (params[:id] || 0)) %>
            <%= render :file => 'bastion/layouts/assets' %>
            <% end %>")

Let me know if I'm missing something, both examples above seemed to work fine on my devel environment.

[tstrachota]

@dLobatog did the examples work for you also with katello? I didn't manage to get them working.
bastion/layouts/assets is not a partial containing only the assets but complete layout. Again, I know very little about deface, but you can't use :insert_after in such case, can you?

I was able to get it working with

Deface::Override.new(
  :virtual_path => 'layouts/application',
  :name => 'bastion_assets',
  :replace => 'erb[loud]:contains("layouts/base")',
  :text => "<% if current_page?(:controller => 'smart_proxies',
                                :action => 'show',
                                :id => (params[:id] || 0)) %>
            <%= render :file => 'bastion/layouts/assets' %>
            <% else %>
            <%= render :file => 'layouts/base' %>
            <% end %>")

but that's not very nice.

[dLobatog]

Merged as 647e331, thanks @tstrachota!