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: UniqueEntity::$service is considered final #777

Merged
merged 5 commits into from
Aug 24, 2023

Conversation

divine
Copy link
Contributor

@divine divine commented Jul 26, 2023

Fix #741.

@divine divine changed the base branch from 4.7.x to 4.6.x July 26, 2023 22:54
@franmomu
Copy link
Contributor

Thanks @divine! what about using the suggested approach by symfony:

Public and protected properties are now considered final; instead of overriding a property, consider setting its value in the constructor

That way a user can use Unique with their own service without overriding the class (I guess it unlikely to happen, but it could happen)

@divine
Copy link
Contributor Author

divine commented Jul 27, 2023

Does it look better now?

Thanks!

Co-authored-by: Fran Moreno <franmomu@gmail.com>
Comment on lines 21 to 31
* @param bool|array|string $ignoreNull The combination of fields that ignore null values
*/
public function __construct(
$fields,
string $message = null,
string $service = 'doctrine_odm.mongodb.unique',
string $em = null,
string $entityClass = null,
string $repositoryMethod = null,
string $errorPath = null,
bool|string|array $ignoreNull = null,
Copy link
Contributor

@franmomu franmomu Jul 28, 2023

Choose a reason for hiding this comment

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

Suggested change
* @param bool|array|string $ignoreNull The combination of fields that ignore null values
*/
public function __construct(
$fields,
string $message = null,
string $service = 'doctrine_odm.mongodb.unique',
string $em = null,
string $entityClass = null,
string $repositoryMethod = null,
string $errorPath = null,
bool|string|array $ignoreNull = null,
*/
public function __construct(
$fields,
string $message = null,
string $service = 'doctrine_odm.mongodb.unique',
string $em = null,
string $entityClass = null,
string $repositoryMethod = null,
string $errorPath = null,
bool $ignoreNull = null,

Now I see this, union types are introduced in PHP >= 8.0, so I guess we should stick to Symfony 5.4 constructor: https://github.com/symfony/symfony/blob/8492f10d29e929a83ccf6c306d40dbad69c9c80b/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntity.php#L55

Adding a "simple" test would be awesome, we would have realized of this. (Test added)

@franmomu franmomu added the bug label Jul 29, 2023
@malarzm malarzm added this to the 4.6.1 milestone Aug 24, 2023
@malarzm malarzm merged commit 0135a16 into doctrine:4.6.x Aug 24, 2023
@malarzm
Copy link
Member

malarzm commented Aug 24, 2023

Thanks @divine and @franmomu!

@divine
Copy link
Contributor Author

divine commented Oct 7, 2023

Hello,

Any ETA for a new release with this fix?

Thanks!

@malarzm
Copy link
Member

malarzm commented Oct 8, 2023

Here you go :) https://github.com/doctrine/DoctrineMongoDBBundle/releases/tag/4.6.1

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.

UniqueEntity::$service is considered final
4 participants