Allow Symfony uid as primary key for Entities managed with Sonata #1738

jordisala1991 · Apr 15, 2023

Subject

I am targeting this branch, because this is BC.

Closes sonata-project/SonataAdminBundle#7327.

Changelog

### Added
- Support for `symfony/uid` as primary keys using `Uuid` or `Ulid`

### Fixed
- `ModelFilter` for related entities using compound ids
- `ModelFilter` when filtering for not equals and using an inverse side relation

jordisala1991 · Apr 15, 2023

This is still WIP @VincentLanglet , but I wanted to get your opinion and also see if builds passes or not. Currently I saw the following problems with Uuid and Ulids:

Batch delete when selecting item by item does not work ✅
Filter when using uuids as a primary key filter (string) does not work ✅
Filter when the uuid is on a foreign key does not work (ModelTypeFilter) ✅
Filter when the uuid is on a foreign key does not work (ModelAutoCompleteFilter) 🔴

Last one I am fixing right now. Besides that, everything else seems to work...

the thing with uuid is that we can't rely on the __toString method to make the query to the database. We have to rely on the third parameter of the setParameter to tell doctrine which type it is, so it can properly deal with the value. At the same time we have to keep the __toString conversion on the modelManager, since when we print Uuids to the end user, we should print the normal string representation, which might not be the same as is stored (binary in some cases)

Also, I have made 2 more changes:

We have some other tests for Uuids (not the Uuid from Symfony, but it seems like a generic implementation. I have moved those fake types to another name. (I guess those tests are meant to be used with an implementation similar to ramsey/uuid)
On doctrine bridge 4.4 there are no types for uuid and ulid... so I have to copy that code for testing purposes.

VincentLanglet · Apr 15, 2023

Seems nice

VincentLanglet · Apr 15, 2023

You say it closes sonata-project/SonataAdminBundle#7327 but is there nothing to do on the Mongo side ?

jordisala1991 · Apr 15, 2023

You say it closes sonata-project/SonataAdminBundle#7327 but is there nothing to do on the Mongo side ?

TBH I am not sure how it should be implemented on Mongo side... not even sure if Symfony team or Doctrine team has added UuidTypes, similar to how it is done on Doctrine bridge for the ORM.

IMHO if the issue ends up being persistence related only. After this PR gets merged, we can move the issue to MongoDB persistence to see if there is interest on this implementation.

src/Filter/ModelFilter.php

tests/App/AppKernel.php

jordisala1991 · Apr 16, 2023

I will need to also take a look at why test for another unrelated feature are failing now.

src/Model/ModelManager.php

composer.json

tests/Functional/Admin/BaseAdminTestCase.php

src/Filter/ModelFilter.php

jordisala1991 · Apr 19, 2023

src/Filter/ModelFilter.php

-                    sprintf('%s.%s IS EMPTY', $this->getParentAlias($query, $alias), $this->getFieldName())
-                );
+            if (false === $this->getAssociationMapping()['isOwningSide']) {
+                $nullExpression = sprintf('IDENTITY(%s.%s) IS NULL', $alias, $this->getAssociationMapping()['mappedBy']);


wdyt @VincentLanglet ?

jordisala1991 · Apr 19, 2023

The changelog could be improved, IMO I also fixed:

ModelFilter with compound entity
ModelFilter on the inverse side of the relation (when filtering for not equals)

Edit: Added.

jordisala1991 added the minor label Apr 15, 2023

jordisala1991 mentioned this pull request Apr 15, 2023

Uuid and Ulid support for Sonata sonata-project/SonataAdminBundle#7327

Closed

jordisala1991 force-pushed the feature/work-with-symfony-uid branch 2 times, most recently from 8ba084c to 232c4d2 Compare April 15, 2023 10:25

jordisala1991 changed the title ~~Allow Symfony uid as primary key for Entities managed with Sonata~~ WIP: Allow Symfony uid as primary key for Entities managed with Sonata Apr 15, 2023

jordisala1991 commented Apr 15, 2023

View reviewed changes

src/Filter/ModelFilter.php Outdated Show resolved Hide resolved

jordisala1991 commented Apr 15, 2023

View reviewed changes

src/Filter/ModelFilter.php Outdated Show resolved Hide resolved

VincentLanglet reviewed Apr 16, 2023

View reviewed changes

tests/App/AppKernel.php Outdated Show resolved Hide resolved

jordisala1991 force-pushed the feature/work-with-symfony-uid branch 4 times, most recently from 1bbef60 to d3d3849 Compare April 16, 2023 09:33

jordisala1991 commented Apr 16, 2023

View reviewed changes

src/Model/ModelManager.php Outdated Show resolved Hide resolved

jordisala1991 force-pushed the feature/work-with-symfony-uid branch 2 times, most recently from 2d3bf9d to a5b2853 Compare April 16, 2023 19:05

jordisala1991 commented Apr 18, 2023

View reviewed changes

composer.json Outdated Show resolved Hide resolved

jordisala1991 force-pushed the feature/work-with-symfony-uid branch from 09ecda6 to c287275 Compare April 18, 2023 17:28

jordisala1991 commented Apr 18, 2023

View reviewed changes

tests/Functional/Admin/BaseAdminTestCase.php Show resolved Hide resolved

jordisala1991 force-pushed the feature/work-with-symfony-uid branch from 09ea6b5 to a1412e7 Compare April 18, 2023 17:36

jordisala1991 changed the title ~~WIP: Allow Symfony uid as primary key for Entities managed with Sonata~~ Allow Symfony uid as primary key for Entities managed with Sonata Apr 18, 2023

jordisala1991 commented Apr 18, 2023

View reviewed changes

src/Filter/ModelFilter.php Show resolved Hide resolved

jordisala1991 force-pushed the feature/work-with-symfony-uid branch 3 times, most recently from ef735c6 to ef35fb6 Compare April 19, 2023 07:33

jordisala1991 commented Apr 19, 2023

View reviewed changes

jordisala1991 force-pushed the feature/work-with-symfony-uid branch from ef35fb6 to 0a3b2dc Compare April 19, 2023 07:37

jordisala1991 requested a review from VincentLanglet April 19, 2023 07:40

jordisala1991 added 2 commits April 24, 2023 06:27

Add tests for Uuids and Ulids as primary keys

cb529ac

Fix filter, batch delete

4825ded

jordisala1991 force-pushed the feature/work-with-symfony-uid branch from 0a3b2dc to 4825ded Compare April 24, 2023 06:27

VincentLanglet approved these changes Apr 25, 2023

View reviewed changes

jordisala1991 merged commit 156f0e7 into sonata-project:4.x Apr 25, 2023

jordisala1991 deleted the feature/work-with-symfony-uid branch April 25, 2023 07:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Allow Symfony uid as primary key for Entities managed with Sonata #1738

Allow Symfony uid as primary key for Entities managed with Sonata #1738

Uh oh!

jordisala1991 commented Apr 15, 2023 •

edited

Loading

Uh oh!

jordisala1991 commented Apr 15, 2023 •

edited

Loading

Uh oh!

VincentLanglet commented Apr 15, 2023

Uh oh!

VincentLanglet commented Apr 15, 2023

Uh oh!

jordisala1991 commented Apr 15, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jordisala1991 commented Apr 16, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jordisala1991 Apr 19, 2023

Uh oh!

jordisala1991 commented Apr 19, 2023 •

edited

Loading

Uh oh!

Uh oh!

Search code, repositories, users, issues, pull requests...

Uh oh!

Allow Symfony uid as primary key for Entities managed with Sonata #1738

Allow Symfony uid as primary key for Entities managed with Sonata #1738

Uh oh!

Conversation

jordisala1991 commented Apr 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Subject

Changelog

Uh oh!

jordisala1991 commented Apr 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VincentLanglet commented Apr 15, 2023

Uh oh!

VincentLanglet commented Apr 15, 2023

Uh oh!

jordisala1991 commented Apr 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jordisala1991 commented Apr 16, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jordisala1991 Apr 19, 2023

Choose a reason for hiding this comment

Uh oh!

jordisala1991 commented Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jordisala1991 commented Apr 15, 2023 •

edited

Loading

jordisala1991 commented Apr 15, 2023 •

edited

Loading

jordisala1991 commented Apr 15, 2023 •

edited

Loading

jordisala1991 commented Apr 19, 2023 •

edited

Loading