Skip to content

Cover RouterService query params during beforeModel redirect#21412

Open
olenderhub wants to merge 1 commit into
emberjs:mainfrom
olenderhub:test/query-param-transition-service-beforeModel
Open

Cover RouterService query params during beforeModel redirect#21412
olenderhub wants to merge 1 commit into
emberjs:mainfrom
olenderhub:test/query-param-transition-service-beforeModel

Conversation

@olenderhub
Copy link
Copy Markdown
Contributor

Adds regression coverage for #17494.

This test covers RouterService#transitionTo with query params when the transition is initiated from an injected service during beforeModel, while another transition is already active.

It verifies that the redirect completes to the target route with the expected URL and controller query param state.

Part of #19609

@olenderhub olenderhub changed the title test: cover RouterService query params during beforeModel redirect Cover RouterService query params during beforeModel redirect May 19, 2026
@olenderhub olenderhub force-pushed the test/query-param-transition-service-beforeModel branch from 31ef067 to 6d777b1 Compare May 19, 2026 20:16
@NullVoxPopuli
Copy link
Copy Markdown
Contributor

looks like tests pass here (and linting is failing)

is it intended that tests pass? was this missing coverage that we should have?

@olenderhub olenderhub force-pushed the test/query-param-transition-service-beforeModel branch from 6d777b1 to cc01001 Compare May 30, 2026 09:19
@olenderhub
Copy link
Copy Markdown
Contributor Author

@NullVoxPopuli

I added this as a regression test for the minimal repro from #17494.

The test passes on current main, so the result seems to be that this behavior has already been fixed. I believe the test still reflects the reported scenario correctly, so I’d like to keep it as missing coverage unless you think there’s a better variant to cover.

I rebase code, so linting pass now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants