Pass url on each request and parse to build params hash

[rickychilcott]

Enhancement

Description

This PR sends the current url (window.location.href) down the wire on each request. This URL is then parsed and the context for the query parameters is passed to the appropriate controller. Specifically, this will allow params to be used inside of helper methods or view templates properly

Fixes #54

Note: this changes up the test suite for channel tests quite a bit and restructures how and where controllers and their renders are resolved. This is to primarily simplify the object's roles and to make testing much easier.

Checklist

• My code follows the style guidelines of this project
• Checks (StandardRB & Prettier-Standard) are passing

[julianrubisch]

So I ran into issues testing this pull request locally.

In an index view (a list of records), when futurizeing a single record (in my example a hovercard), the following error occurs:

[ActionView::Template::Error - No route matches {"action"=>"show", "channel"=>"Futurism::Channel", ...  missing required keys: [:id]]

Apparently the current window.location.href overrides the controller params and causes the render to fail.

This is supported by the fact that commenting out

ActionDispatch::Http::Parameters::PARAMETERS_KEY => params.merge(query_hash)`

fixes the problem.

I think we need a test case covering this scenario and solve it. I'll try to think about how to tackle that.

[julianrubisch]

Follow up: tried params.reverse_merge(query_hash), didn't help :(

[rickychilcott]

A chat in Discord yielded some new ideas. https://discord.com/channels/629472241427415060/683317130598285363/770177899927044106

I'll attempt to write a failing test and fix in the next few days.

[julianrubisch]

hey @rickychilcott just so you know - I've picked this up again and try to produce a failing test! I'll keep you posted here.

[rickychilcott]

Thanks Julian.

With the new kid in the house and being back to work, I've not been able to find time to pick this back up. If you don't have something by the end of the week, I can put some cycles in during a lazy day from the holidays.

[julianrubisch]

Nevermind. I think I’m 50% there. Once we have a failing test it’ll be easier to find a solution.

[julianrubisch]

so, just to record my progress, I'm one step further towards understanding what is actually going on:

the problem occurs when the rendered partial contains another partial that uses an association.

in my case the futurized partial is a _client_hovercard that contains a _user_label partial that will render details of the associated user. while debugging I found that the renderer is fed { action: :show, id: <User#...> }, i.e. an object instead of the ID. will try to create a test case from this.

[julianrubisch]

alright I pushed the fix to your PR here... seemingly renderer doesn't like when "channel" is in the params hash, the other stuff were red herrings 🤦

I'll still try to create a test case for this...

[rickychilcott]

That. Is. Weird. Good find.

[julianrubisch]

Still struggling to grok what is actually happening and why :-/

[julianrubisch]

Progress: I've tracked this down to this line:

https://github.com/rails/rails/blob/1c35b0a34a44e591e7bb56c988369057bf824390/actionview/lib/action_view/renderer/partial_renderer.rb#L357

where template.render fails.

And: the problem obviously lies within route generation, so it's only occurring when there's some path helper involved

[julianrubisch]

It seems like I finally do have a fix... thanks @RolandStuder for all the help!

Seemingly the path_params were missing from the constructed params hash, and merge order was critical, too.

@rickychilcott could you do another quick pass of eyes on this before I merge it? Thanks!

[rickychilcott]

I tested this locally with my little test harness, in addition to the test suite, and this fix seems to be good.

Thanks for your continued work on this @julianrubisch and for the assistance @RolandStuder!