Skip to content

[DoctrineBridge] Allow to ignore specific nullable fields in UniqueEntity #49293

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.

Sign up for GitHub

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

Jump to bottom
Merged
nicolas-grekas merged 1 commit into from
May 19, 2023
Merged

[DoctrineBridge] Allow to ignore specific nullable fields in UniqueEntity #49293

nicolas-grekas merged 1 commit into from
May 19, 2023

Conversation

@VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Feb 8, 2023

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #...
License MIT
Doc PR symfony/symfony-docs#... TODO

When the UniqueEntity is applied on multiple things the only option was

  • ignore all nullable values
  • do not ignore any nullable values

With this feature, it would be possible to ignore nullable values for specific field only and do not ignore nullable values for others.

@carsonbot carsonbot added this to the 6.3 milestone Feb 8, 2023
@VincentLanglet VincentLanglet force-pushed the uniqueEntity branch 4 times, most recently from e083fb1 to beee940 Compare February 8, 2023 09:48
@carsonbot
Copy link

carsonbot commented Mar 2, 2023

Hey!

I think @malarzm has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Mar 2, 2023

Hi @nicolas-grekas, Would you have some time to review this PR ?
Thanks a lot

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Mar 23, 2023

Hi @fabpot maybe you have some time to take a look or redirect me to the right reviewer ?

Thanks

malarzm
malarzm reviewed Mar 27, 2023
View reviewed changes
Copy link
Contributor

@malarzm malarzm left a comment

Choose a reason for hiding this comment

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

Frankly I have a hard time figuring out a use case for this. Could you please share one?

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Mar 28, 2023

Frankly I have a hard time figuring out a use case for this. Could you please share one?

I have an entity of UserRoles which are related to

  • An Organization
  • A Project
  • A User
  • An email

For a specific Organization and Project, only one UserRole should exists.
But UserRole with no relation to a project can exists, it will be only related to a Project, it's only allowed one time.
And the User is nullable, because it's not set until the email is validated.

For the same User/Organization, you cannot have multiple null project
but for the same Organization/Project, you can have multiple null user (pending)

UniqueEntity(fields={organization, project, user}, ignoreNull={user})

malarzm
malarzm approved these changes Mar 28, 2023
View reviewed changes
Copy link
Contributor

@malarzm malarzm left a comment

Choose a reason for hiding this comment

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

Thanks for the use case, it makes more sense now!

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Apr 8, 2023

Would you have time to take a look/review this @nicolas-grekas @fabpot ?
Thanks a lot !

fabpot
fabpot approved these changes Apr 10, 2023
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

LGTM

@nicolas-grekas nicolas-grekas changed the title [Doctrine-bridge] Allow to ignore specific nullable fields in UniqueEntity [DoctrineBridge] Allow to ignore specific nullable fields in UniqueEntity May 19, 2023
@nicolas-grekas
Copy link
Member

nicolas-grekas commented May 19, 2023

Thank you @VincentLanglet.

@nicolas-grekas nicolas-grekas merged commit 95aa09e into symfony:6.3 May 19, 2023
@nicolas-grekas nicolas-grekas mentioned this pull request May 19, 2023
Closed
@OskarStark OskarStark mentioned this pull request May 22, 2023
Closed
@fabpot fabpot mentioned this pull request May 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

+1 more reviewer

@malarzm malarzm malarzm approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

DoctrineBridge Feature Status: Reviewed

Projects

None yet

Milestone

6.3

Development

Successfully merging this pull request may close these issues.

5 participants

@VincentLanglet @carsonbot @nicolas-grekas @fabpot @malarzm