search

tricknotes / ember-cli-rails

Unify your EmberCLI and Rails Workflows
http://thoughtbot.github.io/ember-cli-rails/
MIT License
713 stars 205 forks source link

Redirecting with trailing slash with query parameters #595

Closed mbackermann closed 2 years ago

mbackermann commented 2 years ago

Fixes the bug when redirecting with a trailing slash with query parameters resulting in an invalid URL eg: https://app.com?query=foo would redirect to https://app.com?query=foo/, which is invalid

This PR fixes this issue adding the trailing slash before the query parameters eg: https://app.com?query=foo would be redirected to https://app.com/?query=foo

mbackermann commented 2 years ago

by the way, this test is failing on master branch as well

seanpdoyle commented 2 years ago

Thank you for proposing this change!

by the way, this test is failing on master branch as well

The CI execution for the most recent PR merged into main (https://github.com/thoughtbot/ember-cli-rails/pull/592) passed. In that time, there haven't been any PRs opened, and no other commits have been merged.

Do you have a sense of what might be at the root of the test failures?

mbackermann commented 2 years ago

No problem ;)

Thank you for proposing this change!

by the way, this test is failing on master branch as well

The CI execution for the most recent PR merged into main (#592) passed. In that time, there haven't been any PRs opened, and no other commits have been merged.

Do you have a sense of what might be at the root of the test failures?

I investigated it a little bit yesterday, but I couldn't find the root cause. I tested with different ruby versions, but I got the problem on all of them. Before working on the fix, I ran bin/rake and the test was already failing. It's weird because the first thing I checked was the last commit and I could see that it passed all tests.

I will invest more time today to try to find it.

mbackermann commented 2 years ago

I found the issue. It's on the version of https://github.com/ember-cli/ember-new-output.git. I ran the tests using version 4.0, which was the version used on the last commit here and the test passed

seanpdoyle commented 2 years ago

Thank you!