-
-
Notifications
You must be signed in to change notification settings - Fork 489
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
Make the code compatible with DBAL 3 and 4 by swithing the type when… #1685
base: 5.x
Are you sure you want to change the base?
Conversation
There is the error
|
6f8282e
to
cec9f6a
Compare
…the Mapping is loaded
cec9f6a
to
85202a9
Compare
@VincentLanglet hopefully this is good to go! |
/** | ||
* @psalm-suppress InvalidPropertyAssignmentValue | ||
*/ | ||
$metadata->fieldMappings['roles']['type'] = 'array'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it the right way to change the value ?
The property is documented as READ-ONLY
https://github.com/doctrine/orm/blob/3d9af3187f59930972e7fcb90a1d8059a37b8032/src/Mapping/ClassMetadata.php#L334-L339
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
public function setAttributeOverride(string $fieldName, array $overrideMapping): void
should be used instead. (https://github.com/doctrine/orm/blob/3.1.x/src/Mapping/ClassMetadata.php#L1737)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VincentLanglet https://github.com/doctrine/orm/blob/3.1.x/src/Mapping/ClassMetadata.php#L1765-L1767 it is not possible to change the type using this way.
This solution is a hack but to be honest having a second class BaseUser3
just to change the type is not good either. This can end up, with BaseUser4
etc ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any recommendation for such situation @greg0ire ? Is it the best way ?
The doc need to be updated too, in order to remove reference of BaseUser3 https://github.com/sonata-project/SonataUserBundle/blob/a6169f318ad30a46ced640fad58a686e695e86f0/docs/reference/installation.rst#doctrine-orm-configuration |
Make the code compatible with DBAL 3 and 4 by swithing the type when the Mapping is loaded
I am targeting this branch, because because it introduces a non necessary BC break. For reference: #1677
Changelog
To do