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

[MultiDomainBundle] Fix keying of host configurations #2848

Merged
merged 2 commits into from
Apr 4, 2021
Merged

[MultiDomainBundle] Fix keying of host configurations #2848

merged 2 commits into from
Apr 4, 2021

Conversation

waaghals
Copy link
Contributor

@waaghals waaghals commented Feb 9, 2021

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets N/a

This fixes the configuration for the hosts in the multi domain bundle.

When modifying the configuration from another bundle (using PrependExtensionInterface) the configuration get re-indexed by the Symfony configuration component. This cause the configuration to be added twice.
Once using the origin key (the modified configuration) and once using numeric ids re-indexed by Symfony.

I figured this was attempted previously as the code is commented out. Enabling this as-is will remove the 'host' configuration field. (as it is should only be used as the key, not a value).

By using a different name for the attribute, the original configuration is left as is. And Symfony now uses the correct value as the key.

Copy link

@ProfessorKuma ProfessorKuma left a comment

Choose a reason for hiding this comment

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

Hi @, your PR needs some changes

  • It seems that you should have submitted to the latest minor branch.
  • This PR seems to need a milestone of a patch release.

@ProfessorKuma ProfessorKuma added this to the 5.6.5 milestone Feb 9, 2021
@waaghals waaghals changed the base branch from 5.7 to 5.6 February 9, 2021 13:24
Copy link

@ProfessorKuma ProfessorKuma left a comment

Choose a reason for hiding this comment

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

Hi @, your PR passed all our requirements.

Thank you for contributing!

@acrobat acrobat modified the milestones: 5.6.5, 5.6.6 Mar 17, 2021
@acrobat acrobat merged commit 328fb5d into Kunstmaan:5.6 Apr 4, 2021
@acrobat
Copy link
Member

acrobat commented Apr 4, 2021

Thanks @waaghals!

acrobat added a commit that referenced this pull request Apr 4, 2021
* 5.6:
  [MultiDomainBundle] Fix keying of host configurations (#2848)
  [RedirectBundle] Fix broken wildcard redirects
acrobat added a commit that referenced this pull request Apr 4, 2021
* 5.7:
  [MultiDomainBundle] Fix keying of host configurations (#2848)
  [RedirectBundle] Fix broken wildcard redirects
acrobat added a commit that referenced this pull request Apr 4, 2021
* 5.8:
  [MultiDomainBundle] Fix keying of host configurations (#2848)
  [RedirectBundle] Fix broken wildcard redirects
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.

3 participants