update the extending models guide for rails 4 and strong paramaters

[jess]

Is this how you'd recommend doing it?

[parndt]

@ugisozols what do you think?

[ugisozols]

I think we need some kind of extra_params method which then gets merged into page_params. This would allow users to only add stuff they need by overriding that method instead of the whole page_params.

[parndt]

What about

def page_params
  super.permit(:extra_param)
end

Does that work? It wouldn't allow the user to remove existing params, just add new ones.

[jess]

I'd prefer the extra method too.

I tried the super.permit(:extra_params) and get:

super: no superclass method `page_params' for #<Refinery::Admin::PagesController:0x007fdbdbaa6630>

I guess because we're actually overriding that method as opposed to sub classing it??

What if did the extra_params like this:

  def page_params
    params.require(:page).permit(
      *extra_params, :browser_title, :draft, :link_url, :menu_title, :meta_description,
      :parent_id, :skip_to_first_child, :show_in_menu, :title, :view_template,
      :layout_template, :main_photo_id, parts_attributes: [:id, :title, :body, :position]
      ).merge(extra_params)
  end

  def extra_params
    []
  end

Then we'd just need to overwrite extra_params.

The only problem with that might be if another engine/gem needed to also permit params, how could it be done from more than one place?

It'd be cool if we could just call a method from our decorator like add_extra_params :extra_param and that method would just continue adding to an instance variable. Similar to a before filter. But if I try to define that method in the main admin pages controller class, it won't work because our decorator gets loaded first and I get a no method error.

[parndt]

Ah, of course.. it's not a subclass.

This is the sort of thing that alias_method_chain was invented for.. even though it's deemed an anti-pattern.

def page_params_with_my_ones
  page_params_without_my_ones.permit(:extra_params)
end

alias_method_chain :page_params, :my_ones

I'd rather not have to add even more "magic method" APIs to Refinery if we can help it.

Sometimes I can see, with Devise, why you have to inherit all of their controllers to modify how it works at all.

[jess]

cool, I wasn't that familiar with alias_method_chain.

I got this to work:

Refinery::Admin::PagesController.class_eval do
  def page_params_with_my_params
    page_params_without_my_params.merge(params.require(:page).permit(:main_photo_id))
  end

  alias_method_chain :page_params, :my_params
end

From reading a few posts, it does seem to be abused, but under the right circumstances, it's has a valid use. I'd say this is a good recommendation for it since we don't want to completely overwrite the method and we cannot subclass it.

[ugisozols]

Ah yes, I totally forgot about alias_method_chain. 👍

@jess keen to update guide mentioning alias_method_chain?

[jess]

yes, I'll be glad to...

[jess]

what do you think?

[ugisozols]

Looks good. @parndt?

[parndt]

@jess looks good, can you please squash into a single commit for this change?

[jess]

@parndt sure, I've done that here cd407e3, do I need to issue a new PR? Or, can I force over these changes?

[parndt]

@jess you just need to force push to this branch guide_update on your fork.

[jess]

@parndt thanks for the tip, all done