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

handling __toString in getValueFromType #937

Merged
merged 4 commits into from
Dec 25, 2019

Conversation

oleg-andreyev
Copy link
Contributor

@oleg-andreyev oleg-andreyev commented Oct 5, 2019

Subject

ModelManager::getValueFromType may return pure-binary value if using with conjunction with https://github.com/ramsey/uuid-doctrine

Error example:

Doctrine\DBAL\DBALException:
Unknown database type uuid_binary_ordered_time requested, Doctrine\DBAL\Platforms\MySQL80Platform may not support it.

  at vendor/doctrine/dbal/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php:434
  at Doctrine\DBAL\Platforms\AbstractPlatform->getDoctrineTypeMapping('uuid_binary_ordered_time')
     (vendor/sonata-project/doctrine-orm-admin-bundle/src/Model/ModelManager.php:637)
  at Sonata\DoctrineORMAdminBundle\Model\ModelManager->getValueFromType(object(Uuid), object(UuidBinaryOrderedTimeType), 'uuid_binary_ordered_time', object(MySQL80Platform))
     (vendor/sonata-project/doctrine-orm-admin-bundle/src/Model/ModelManager.php:346)
  at Sonata\DoctrineORMAdminBundle\Model\ModelManager->getIdentifierValues(object(Image))
     (vendor/sonata-project/doctrine-orm-admin-bundle/src/Model/ModelManager.php:383)
  at Sonata\DoctrineORMAdminBundle\Model\ModelManager->getNormalizedIdentifier(object(Image))
     (vendor/sonata-project/doctrine-orm-admin-bundle/src/Model/ModelManager.php:400)
  at Sonata\DoctrineORMAdminBundle\Model\ModelManager->getUrlsafeIdentifier(object(Image))
     (vendor/sonata-project/admin-bundle/src/Admin/AbstractAdmin.php:2402)
  at Sonata\AdminBundle\Admin\AbstractAdmin->getUrlsafeIdentifier(object(Image))
     (vendor/sonata-project/admin-bundle/src/Admin/AbstractAdmin.php:1231)
  at Sonata\AdminBundle\Admin\AbstractAdmin->generateObjectUrl('edit', object(Image), array())
     (vendor/sonata-project/admin-bundle/src/Controller/CRUDController.php:1273)
  at Sonata\AdminBundle\Controller\CRUDController->redirectTo(object(Image))
     (vendor/sonata-project/admin-bundle/src/Controller/CRUDController.php:639)
  at Sonata\AdminBundle\Controller\CRUDController->createAction()
     (vendor/symfony/http-kernel/HttpKernel.php:151)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:68)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:198)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (public/index.php:25)

I am targeting this branch, because patching of getValueFromType is compatible

Changelog

### Changed
- Added check and call of `toString` and `__toString` when calling `getValueFromType` on value-object such as Uuid

tests/Fixtures/Entity/UuidBinaryEntity.php Show resolved Hide resolved
src/Model/ModelManager.php Show resolved Hide resolved
tests/Fixtures/DoctrineType/Uuid.php Outdated Show resolved Hide resolved
tests/Fixtures/Entity/UuidBinaryEntity.php Outdated Show resolved Hide resolved
@greg0ire greg0ire requested a review from OskarStark October 6, 2019 15:59
@greg0ire
Copy link
Contributor

greg0ire commented Oct 6, 2019

Are there docs that need to be added? If I understand well, I should know about the necessity of implementing __toString if I want to use Sonata + a non-string identifier, correct?

@oleg-andreyev
Copy link
Contributor Author

@greg0ire if custom identifier is an value object (e.g: Uuid) then yes, __toString must be implemented

@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

tests/Fixtures/DoctrineType/Uuid.php Outdated Show resolved Hide resolved
@core23 core23 added the patch label Dec 23, 2019
@oleg-andreyev oleg-andreyev requested a review from core23 December 24, 2019 22:47
@OskarStark OskarStark merged commit 46d14ed into sonata-project:3.x Dec 25, 2019
@OskarStark
Copy link
Member

Thanks 🎄

@oleg-andreyev
Copy link
Contributor Author

@core23 @OskarStark glad to help! Thanks for your time!

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.

5 participants