Support for controller namespace(s)

[rwojsznis]

Hi!

First of all - thank you for this fine gem, I'm going to use it on my upcoming application :). But - it appears I've found a small bug (or maybe support for it wasn't ever intended?) when it comes for using multiple controllers of the same name under different namespace (eg. for frontend and backed).

Let's say I have a badge rule binded to users#update, but I also have Admin::UsersController also with update action. You can guess what happens - it blows in you face (so to speak).

In merit/lib/merit/controller_extensions.rb you have this after_filter that checks defined rules:

def rules_defined?
  action = "#{controller_name}\##{action_name}"
  AppBadgeRules[action].present? || AppPointRules[action].present?
end

What you should use is controller_path that returns full path (with namespace) and you should be fine... But apparently you won't because it goes deeper to Merit::Action class, as you save action_method and target_model without distinguishing the controller namespace.

I thought I could be able to make a quick fix for this, but I don't think it will be so quick and I don't want to break something else by accident. I hope I didn't mixed up anything :).

Best regards!

[zamith]

I'm also having this problem when integrating Merit with Forem and other engines. Namespacing awareness would be great.

[tute]

Thanks for your feedback. Certainly this is a bug, as rules are in these cases ambiguous.

You are right @emq: the only problem in adding the namespace with controller_path I can see is in https://github.com/tute/merit/blob/master/app/models/merit/action.rb#L41-L42, which may end up calling something like 'admin/users'.singularize.camelize.constantize.find_by_id(target_id) which would raise a NameError.

The two options that I can now think of are:

• Strip out the namespacing, to call just 'users'.singularize.camelize.constantize.find_by_id(target_id)
• If we require the :model_name option in rules with namespaced controllers, it won't break. That functionality gets called only from https://github.com/tute/merit/blob/master/lib/merit/judge.rb#L36, and the option would override the namespaced controller.

A mix of the two seems like the best options: try the namespaced model, catch a possible NameError to try the Rule#model_name option, and display a helpful error message if inexistent or also raises NameError.

Thanks again for your feedback!

[zamith]

Thanks for being on top of things.

The case you mention only raises the NameError if the class does not exist, if there was an Admin::User constant, it would work just fine.

I've manage to get it working by changing controller_name to controller_path in the target_model of the action and on the rules_defined? method.

Then I define the rule as grant_on 'forem/posts#create', :badge => 'commenter', :level => 10, model_name: 'Forem::Post' do |post|, it may work even without the model_name, not sure.

This has worked for me at least with the badges, but I believe it should work with points and rank as well.

[tute]

When the class exists yes, it would work even without the model_name option @zamith. When I make the time to write the tests I will merge your tweak and my fallback with nice error message, and release a new version.

[zamith]

Great! I'll use my patched version until then, but will be on the lookout for the new version.

[tute]

Version 1.2.1 just released with a small API change (see https://github.com/tute/merit/blob/master/UPGRADING.md).
Thanks for the feedback!