[BUGFIX release] Sticky query params for nested and for dynamic routes

[jmurphyau]

Fixed query params not being sticky for nested routes

Fixed query params not working for multiple routes that each have a dynamic segment (which fixes #10838)
Previously only 1 dynamic segment was working correctly

[jmurphyau]

Wasn't sure if I was to use [BUGFIX release] since it's not an 'urgent' fix (or is it?)..

Even though this bug exists in release, is this a case where I would use [BUGFIX beta] because the fix will actually be released in beta (not because its fixing a bug that exists/originated from beta?)

[btecu]

If this is an issue affecting 1.13, yes, since there's no more 1.x beta, so it probably makes sense to have RELEASE.

[jmurphyau]

"In general bug fixes are pulled into the beta branch. As such, the prefix is: [BUGFIX beta]. If a bug fix is a serious regression that requires a new patch release, [BUGFIX release] can be used instead."

So in other words if this bugfix will be released in a future 1.13.x it should have [BUGFIX release] - but if say we were at 1.14.0-beta.1 and the bug fix wasn't that serious (which this one doesn't seem to be - or am I understanding that incorrectly?) it would be [BUGFIX beta] and released in 1.14.0-beta.2 and eventually 1.14.0.. And since 2.0.0 is a big release this fix should go into the 1.13 stream..

Have I understood that correctly?

[rwjblue]

@jmurphyau - Yes, I think that is a pretty good explanation of our current situation. Basically, if we were continuing the 1.x series this would definitely be [BUGFIX beta] (releases are only a few weeks out and should be relatively painless to update to), however since there will be no more 1.x releases we should likely include this in a patch release of 1.13.

[rwjblue]

I'd like @machty or @trek to review (they are much more familiar with this section of code than I am).

[machty]

This looks good to me. My only concern is the extreme test bloat. Is there any way it can be consolidated with other tests (possibly generating a different module and tests based on changed config within a loop)?

[jmurphyau]

I was actually thinking about that after I was done - wasn't happy with the tests..

I was thinking the same as you - define the tests once and execute them 6 times, rather than copied/pasted 6 times. Whether that be by function definitions or a loop same same.

I'll find some time and refactor.

[machty]

@jmurphyau 👍

[trek]

I'm 👍

[jmurphyau]

Managed to get the test code down best I could:

• Tests that are duplicated have been split into a function and the function is called twice
• isEnabled('ember-routing-route-configured-query-params') checked have been moved to smarter positions, rather then copy/paste all tests twice

[rwjblue]

I cherry-picked into beta just fine, but when I attempted to cherry-pick this into the stable branch there were a number of conflicts (because stable branch does not have the route QP work that @trek did).

@jmurphyau - Would you be able to submit another PR with the changes that apply to "controller-land" against the stable branch directly?

[jmurphyau]

Ok cool - will do

[jmurphyau]

Done - See PR #11636