Skip to content

[Validator] Improve documentation for UniqueEntity constraint #21197

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

Open
wants to merge 1 commit into
base: 7.3
Choose a base branch
from

Conversation

ker0x
Copy link
Contributor

@ker0x ker0x commented Jul 9, 2025

Replace #14458 and #20166

I'm targeting 7.3 because i don't know if changes made in 7.1 are transferred to 7.3. Otherwise, let me know and I'll change it.

@ker0x ker0x requested a review from xabbuh as a code owner July 9, 2025 12:19
@carsonbot carsonbot changed the title [DoctrineBridge][Validator] Improve documentation for UniqueEntity constraint [Validator] Improve documentation for UniqueEntity constraint Jul 9, 2025
@ker0x ker0x force-pushed the fix/unique-entity branch 2 times, most recently from 604f8fe to b1a3e1a Compare July 9, 2025 12:24
@OskarStark OskarStark changed the title [Validator] Improve documentation for UniqueEntity constraint [Validator] Improve documentation for UniqueEntity constraint Jul 9, 2025
@ker0x ker0x force-pushed the fix/unique-entity branch from b1a3e1a to 7f65194 Compare July 9, 2025 15:48
Comment on lines +119 to +120
This constraint can also check **uniqueness on any PHP class** and not only on
Doctrine entities. Consider the following Doctrine entity::
Copy link
Contributor

Choose a reason for hiding this comment

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

The phrase "any PHP class" implies that the constraint can be applied to any arbitrary PHP class, regardless of its relationship to Doctrine.

Suggested change
This constraint can also check **uniqueness on any PHP class** and not only on
Doctrine entities. Consider the following Doctrine entity::
This constraint can also check **uniqueness on any PHP class** that is mapped to a Doctrine entity, and not only on the entities themselves. Consider the following Doctrine entity::

@@ -103,29 +100,6 @@ between all of the rows in your user table:
}
}
// src/Form/Type/UserType.php
namespace App\Form\Type;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this was deleted?

)]
class UserDto
{
public ?int $id = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the id parameter. It might be misleading and suggest that an identifier is required, just like in an entity. Additionally, someone might assume that during an update, the validator would automatically match this class to the corresponding entity based on the id—as it currently works with Doctrine entities.
Update without identifierFieldNames will not work.

@@ -269,9 +353,97 @@ the combination value is unique (e.g. two users could have the same email,
as long as they don't have the same name also).

If you need to require two fields to be individually unique (e.g. a unique
``email`` and a unique ``username``), you use two ``UniqueEntity`` entries,
``email`` and a unique ``username``), you should use two ``UniqueEntity`` entries,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should is more like a suggestion, but there is no choice here. You must use it that way, if you require it.


**type**: ``array`` **default**: ``[]``

The ``identifierFieldNames`` option allows you to specify the field
Copy link
Contributor

Choose a reason for hiding this comment

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

I've noticed that some people define this parameter even when working with entities. However, entities automatically know which property is the identifier. It would be worth adding a note that in such cases, there's no need to define identifierFieldNames.
Additionally, it's only required during updates.

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