Fix URL generation when explicitly passing params to link helpers

[5t111111]

This resolves #873

As I described in this comment, params may have nested ActionController::Parameters .

Although Kaminari converts ActionController::Parameters to Hash in initializer of Kaminari::Helpers::Tag in order to pass it to url_for helper for URL generation, ActionController::Parameters in explicitly passed params remain as it was because merge! won't replace other hash's values if keys are already exist.

So, I have simply changed merge! to reverse_merge! to have an expected result. How does this sound to you?

In addition to that, on the edge Rails url_for helper seems to have been improved to take ActionController::Parameters as an argument. I am not sure if Kaminiari requires additional changes for Rails' changes.

[yuki24]

This would slightly change the behavior - the expected behavior is that if both template.params and params (the argument of this method) have the same key, the one in the argument params should take the precedence.

[5t111111]

Thanks for your review. Ah true, in the real use-cases you expect that params in the argument should take the precedence.

Then what if params in the argument is converted to Hash in the same way with teamplate.params ?

If it looks good to you, let me confirm if it works.

[yuki24]

👍 on converting params to a raw hash object.

Also, as you mentioned earlier, this only occurs with unintended usage, and I wouldn't feel comfortable maintaining this bit of code forever (less code can do the same as more code). It should show a warning to inform that param: params could be removed.

[yuki24]

I'm not sure whether or not there is a case where you'd have to pass in an ActionController::Parameters object explicitly. I think we should wait until we come up with a use case where this patch is absolutely neccesary.

In addition to that, on the edge Rails url_for helper seems to have been improved to take ActionController::Parameters as an argument

Hopefully @amatsuda will take care of this before Rails 5.1 comes out.

[5t111111]

I don't have any idea where you have got no choice but to pass ActionController::Parameters explicitly as params argument either.

As I described in this comment, maybe this issue occurs only with unintended usage of an API, and I was able to fix my app's codes somehow to avoid that.

So I am just not sure if you regard this issue as a kind of regression because, in StackOverflow, some blogs, etc., there are quite a few example codes that passing params explicitly like link_to_next_page @items, 'Next Page', params: params which had always worked before v1.0.0.

[yuki24]

I'm sorry for not getting back to you sooner. I think we should merge this in. Would you mind updating this PR if you are still interested?

[5t111111]

@yuki24 I've updated this PR for:

• Converting passed params to a Hash in exactly the same way with template.params
• Displaying a deprecation warning when params are explicitly passed

However, I am atill a bit worried if Kaminari::Helpers::Tag's initializer is the right place to show a warning message. Please let me know if you have any suggestion.

[yuki24]

Thanks for the request! also was great meeting you at the RubyKaigi 👍🏻