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

fix phpdoc for ModelToIdPropertyTransformer::$property + add test #6783

Merged
merged 1 commit into from
Jan 19, 2021

Conversation

dmaicher
Copy link
Contributor

Subject

I am targeting this branch, because its BC.

Related to #6779 (fix needs to be merged into master first).

Changelog

### Fixed
- fixed phpdoc for `Sonata\AdminBundle\Form\DataTransformer\ModelToIdPropertyTransformer::$property` which can be `string[]` as well

@VincentLanglet
Copy link
Member

@phpstan-template P of string|string[] is not supported, so maybe @phpstan-template P ?

@VincentLanglet
Copy link
Member

Rebasing 3.x should fix the build

@VincentLanglet VincentLanglet requested a review from a team January 19, 2021 09:39
Copy link
Member

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Thank you David!

@OskarStark OskarStark merged commit 5263515 into sonata-project:3.x Jan 19, 2021
@franmomu
Copy link
Member

Just an idea, since ModelToIdPropertyTransformer apparently is only used in ModelAutocompleteType (and the user usually does not interact with ModelToIdPropertyTransformer), what about to only allow array in ModelToIdPropertyTransformer and apply a normalizer to ModelAutocompleteType option (to OptionsResolver::setNormalizer) in case is just a string, change it to array.

So this way, the user can use string|string[] for property and the ModelAutocompleteType is the one in charge of normalize it to string[].

@dmaicher dmaicher deleted the issue-6779 branch January 19, 2021 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants