[Routing] Fix URL generator instantiation #34192
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tobion
approved these changes
Oct 31, 2019
3035356 to
9aa66e2
Compare
|
Good catch, thanks @X-Coder264. |
nicolas-grekas
added a commit
that referenced
this pull request
Nov 4, 2019
…eMC) This PR was merged into the 4.3 branch. Discussion ---------- [Routing] Fix URL generator instantiation | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | License | MIT After merging my fix (PR #31023) from the 4.2 branch to 4.3 (which was master at the time) one code path which instantiates the url generator was missed. This new code path was added via #28865 and was missed during the merge because there was no merge conflict there as that code path didn't exist on 4.2. So the current state on the 4.3 branch is: - https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L356 (default locale is properly passed to the constructor) - https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L372 (default locale is **NOT** properly passed to the constructor) - https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L378 (default locale is properly passed to the constructor) I noticed this in my app because tests started to fail on the branch where we are upgrading our app from SF 4.2 to 4.3 due to this. This PR fixes the inconsistency (now the default locale is passed on all three places) which fixes the bug. Commits ------- 9aa66e2 Add tests to ensure defaultLocale is properly passed to the URL generator 16c9baf Fix URL generator instantiation
|
Thank you @HypeMC too for the tests. |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After merging my fix (PR #31023) from the 4.2 branch to 4.3 (which was master at the time) one code path which instantiates the url generator was missed. This new code path was added via #28865 and was missed during the merge because there was no merge conflict there as that code path didn't exist on 4.2.
So the current state on the 4.3 branch is:
https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L356 (default locale is properly passed to the constructor)
https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L372 (default locale is NOT properly passed to the constructor)
https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L378 (default locale is properly passed to the constructor)
I noticed this in my app because tests started to fail on the branch where we are upgrading our app from SF 4.2 to 4.3 due to this. This PR fixes the inconsistency (now the default locale is passed on all three places) which fixes the bug.