Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX release] Ensure query param only link-to's work in error states. #17971

Merged
merged 2 commits into from
Jun 14, 2019

Conversation

2hu12
Copy link
Contributor

@2hu12 2hu12 commented Apr 25, 2019

Try to fix #17963, I'm not sure this is the right way and it's weird, but the old "qualifiedRouteName" property works in this way and "_route" is used similarly to "qualifiedRouteName".

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix seems reasonable (thank you!), but I think we need to add a previously failing test to ensure we don't regress again...

@2hu12 2hu12 force-pushed the patch-1 branch 3 times, most recently from cd43696 to 21edc29 Compare May 7, 2019 15:30
@2hu12
Copy link
Contributor Author

2hu12 commented May 7, 2019

I try to add one test in "packages/@ember/-internals/glimmer/tests/integration/components/link-to/query-params-curly-test.js", but with no success. The test need an {{#link-to}} with no route parameter and the bug could be reproduced when transition to an error route which might have a rejected model, but when it encounters an error like model() { RSVP.reject(new Error('some error')) } in test, the whole test just terminated.

@rwjblue Do you have any idea on how to write this previously failing test? Thank you!

@villander
Copy link
Contributor

@rwjblue @dgeb any idea about this? We are with ember-engines CI broken.

I'll be happy if you can add any tip to finish this, thanks

@villander
Copy link
Contributor

we are trying fix this on engines side as well - ember-engines/ember-engines#647

@villander
Copy link
Contributor

@2hu12 unfortunately I don't was able to write tests for this =/

2hu12 and others added 2 commits June 14, 2019 13:50
… route.

Reproduce failure in emberjs#17963: `<LinkTo>` without `@route` param but with
`@query` breaks when navigating to error route.

Co-authored-by: Robert Jackson <me@rwjblue.com>
@rwjblue
Copy link
Member

rwjblue commented Jun 14, 2019

I just rebased and pushed a previously failing test (using the one @dgeb started over in #18061), once CI is green we should be good to go.

@rwjblue rwjblue merged commit 37a1942 into emberjs:master Jun 14, 2019
@villander
Copy link
Contributor

villander commented Jun 14, 2019

thanks @rwjblue

@rwjblue rwjblue changed the title Try to fix #17963 [BUGFIX release] Ensure query param only link-to's work in error states. Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<LinkTo> without @route param but with @query breaks when navigating to error route
4 participants