VincentLanglet
commented
4 years ago Thanks for the report @gremo. Did you tried others version to help to find the commit with the bug ?
Closed gremo closed 4 years ago
VincentLanglet
commented
4 years ago Thanks for the report @gremo. Did you tried others version to help to find the commit with the bug ?
gremo
commented
4 years ago @VincentLanglet I only spotted that the bug was introduced in 3.66.0 (no specific commits).
VincentLanglet
commented
4 years ago @gremo It works correctly for me
gremo
commented
4 years ago Can you put here a minimal example (entity and admin)? I am pretty sure that it’s Working only downgrading and without the red text like you shown ☹️
VincentLanglet
commented
4 years ago Can you put here a minimal example (entity and admin)?
Create a one-to-may relation for the News entity, named Images, towards Image, and with Assert\Valid.
Add a required field with an Assert\NotNull constraint on Image.
Add a form type CollectionType in order to manage it. Create an admin for the Image entity.
I am pretty sure that it’s Working only downgrading and without the red text like you shown ☹️
If you create a repository with your issue ; I will be able to debug it.
gremo
commented
4 years ago Ok, I'll try to ugrade again and double check the issue, than I'll create a minimal example. Can you please show yourshow sonata-project/* --latest?
VincentLanglet
commented
4 years ago Sure,
sonata-project/admin-bundle 3.69.0 3.71.0 The missing Symfony Admin Generator
sonata-project/block-bundle 3.18.5 4.2.0 Symfony SonataBlockBundle
sonata-project/cache 2.0.1 2.0.1 Cache library
sonata-project/core-bundle 3.20.0 3.20.0 Symfony SonataCoreBundle (abandoned)
sonata-project/doctrine-extensions 1.6.0 1.6.0 Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.18.0 3.19.0 Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/exporter 2.2.0 2.2.0 Lightweight Exporter library
sonata-project/form-extensions 0.1.2 1.5.0 Symfony form extensions
sonata-project/twig-extensions 0.1.1 1.3.0 Sonata twig extensionsI'll be happy to try to debug your example :)
gremo
commented
4 years ago Ok, done it, not so "minimal"... but here we are:
git clone https://github.com/gremo/sonata-admin-6166.git
cd sonata-admin-6166/
yarn install
composer installThen, create your .env.local as usual (DATABASE_URL and optional DATABASE_VERSION). Then
php bin/console doctrine:database:create
php bin/console doctrine:schema:update --force
php bin/console fos:user:create --super-admin admin admin@localhost password
yarn encore devGo to http://localhost:8000/admin/app/news/create
Let me know! Thanks!
VincentLanglet
commented
4 years ago The issue was introduced by https://github.com/sonata-project/SonataAdminBundle/pull/6066 Which is improved by https://github.com/sonata-project/SonataAdminBundle/pull/6171 But still doesn't solve.
The main goal of these PR was to set the parent subject when creating a new entity.
Before this, $this->getSubject()->getNews() was returning null in the NewsHasImageAdmin.
But now it returns the news you're creating/editing.
This allow to have a specific form (you can filter some ChoiceType depending on some News values), which can be useful when editing.
This create an issue with the handleRequest. Before when you create and submit a row 0, since the News entity had no NewsHasImage value, it add the row 0. But now, when you submit the row 0, since the row 0 was already set to the News entity, it remove the row 0 and re-add a new row, the row 1 with the same values. This means that the form error which was associated to the field 0 doesn't find the field anymore and is displayed in the top of the page.
I need some more investigations. If you can give me any idea/help ; I take it.
gremo
commented
4 years ago So, that was my fault? Why your example seems working?
The main goal of these PR was to set the parent subject when creating a new entity. Before this, $this->getSubject()->getNews() was returning null in the NewsHasImageAdmin. But now it returns the news you're creating/editing. This allow to have a specific form (you can filter some ChoiceType depending on some News values), which can be useful > > > when editing.
Perfectly clear. yes, I was aware that it was not possible to access the parent subject from a nested admin. I solved the issue some times ago for a project, but it was a dirty workaround, messing with event listeners (if I recall correctly).
I need some more investigations. If you can give me any idea/help ; I take it.
My understandings of Sonata internals are not even close to yours, so I doubt, I'm sorry :(
VincentLanglet
commented
4 years ago So, that was my fault? Why your example seems working?
Its working when you're editing an existant NewsHasImage entity (since the index does not change). But it doesn't work when you're trying to add a new one.
I reproduced the issue on my personnal project.
I need some more investigations. If you can give me any idea/help ; I take it.
My understandings of Sonata internals are not even close to yours, so I doubt, I'm sorry :(
I don't know if it's Sonata or Symfony related. My understanding are not the best too ; I essentially dump data until I find the solution.
Removing this line https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Form/Type/AdminType.php#L112 Fix the issue. But we lost the fact the parent subject is set to the new row...
Everything is happening here: https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Controller/CRUDController.php#L313-L314
We're setting a News with no NewsHasImage relation. Then we're handle the request with a NewHasImage field submitted. And then the form field has the index 1 instead of 0.
This is maybe related to https://github.com/sonata-project/form-extensions/blob/1.x/src/EventListener/ResizeFormListener.php I saw that this listener is triggered during the handleRequest. But I currently doesn't understand everything in this file.
VincentLanglet
commented
4 years ago @gremo I'm looking for a solution.
I think I have one. Can you try https://github.com/sonata-project/SonataAdminBundle/pull/6171 ? It seems to work for me.
gremo
commented
4 years ago Thanks @VincentLanglet for helping us. I'm quite busy at work right now, but I should test your fix during this weekend.
VincentLanglet
commented
4 years ago @gremo Did you find time to try it ? I'll soon finish this PR.
gremo
commented
4 years ago @VincentLanglet right now. I dirty cloned your branch directly into the vendor folder. Seems working fine, however as I said... I don't know if something else will break, my understanding of internals isn't good:
Sidenote: why you see the red error message but I can't? 😄
VincentLanglet
commented
4 years ago Sidenote: why you see the red error message but I can't? 😄
I think I have an old fix in my project. Maybe I can make a PR for Sonata.
gremo
commented
4 years ago You should, It exists since months or years, can't remember... probably never worked 😂
gremo
commented
4 years ago @VincentLanglet thank you.
VincentLanglet
commented
4 years ago You're welcome. I also opened a PR for the design fix https://github.com/sonata-project/SonataAdminBundle/pull/6195
Environment
Sonata packages
Symfony packages
PHP version
Subject
With CollectionType inline table, now errors are bubbled into parent form. Didn't happened in
Steps to reproduce
Create a one-to-may relation for the
Newsentity, namednewsHasImages, towardsNewsHasImage. Add a form typeCollectionTypein order to manage it. Create an admin for theNewsHasImageentity and add a required file (for example, aMediafield).Add a new row (inline-table) and save without selecting a media.
Expected results
Error are shown right under the field "media".
Actual results
Errors are bubbled.
Hint
The same code works with the following:Introduced with version 3.66.0,