Sonata type collection delete row triggers constraints validation

[manuel-bitcraft]

I have a problem with sonata admin, specifically in a type collection field. Here's the scenario:

I have an entity called "Group" with a one to many relationship to the entity "Member"

In sonata admin I set up the admin classes for the 2 entities.

In the "Group admin" i create a collection type with the Member entity this way

->add('members', 'sonata_type_collection', [
    'by_reference'          => false,
    'label'                 => 'Members',
    'type_options'          => ['delete' => true],
    'btn_add'               => "Add Member",
    'required'              => false,
    'constraints'           => $validation['members'],
],
[
    'edit'              => 'inline',
    'inline'            => 'table'
])

The member entity has 4 fields, one of which is required.

->add('firstName', TextType::class, [
    'label'         => 'First Name'
    'constraints'   => [
        new NotBlank(['message' => 'Please enter the name.']),
    ]
])

If I click the "Add Member" button in the edit view, it adds a new row as expected. At that point, if I change my mind and decide to delete the newly added record, without adding the name, on save it's returning a validation error telling me 'Please enter the name.' on the field.

Shouldn't the delete action have priority over validation?

[RomainSanchez]

Same here

[greg0ire]

Can one of you bisect that bug down to a particular commit?

[RomainSanchez]

Can one of you bisect that bug down to a particular commit?

It would be the introduction of sonata_type_collection inline edit i suppose since the problem is that an input with required attribute is inserted in the form and is not removed when the user changes his mind wich prevents submitting the form

I'll have a look but this looks more like a design problem than a bug

[RomainSanchez]

Maybe this issue should move to DoctrineORMAdminBundle ?

[greg0ire]

Maybe… I don't know.

[manuel-bitcraft]

As RomainSanchez said, I think this was always the case. I can open an issue on the DoctrineORMAdminBundle if you think is better. Thanks

[greg0ire]

without adding the name

If you want to delete a row, there's a button for that right? You don't expect an empty value to be accepted, do you ? Did you use allow_delete or whatever the right option for this is?

[RomainSanchez]

If you want to delete a row, there's a button for that right?

It is a checkbox that needs the form to be submitted before the item is removed.So when you add an item and want to remove it (wich end users do all the time) checking the checkbox would need to remove the empty inputs from the dom

[greg0ire]

checking the checkbox would need to remove the empty inputs from the dom

Exactly. First thing to do on the js submit() event

[RomainSanchez]

I'll see if i can find time in the next few days to make a PR for this

[jordisala1991]

With html5 validation enabled you will still get a message before submit, not sure if we can solve this changing only the order of actions in submit() in javascript

[greg0ire]

Indeed

[jordisala1991]

IMO the problem is, add is done in ajax, remove is done on reload... not sure if this has an easy fix tho.

[RomainSanchez]

If it is a new entry you remove the inputs from the dom on click and if it is an existing one you remove from the dom and replace with a hidden input/hidden inputs that will "submit the deletion" or maybe trigger an ajax request to delete the item even before form submission

[greg0ire]

Well if you remove the input immediately when checking the checkbox (or cliking a button), problem solved. Also, you could just make toggle disable on checking the checkbox

[RomainSanchez]

you could just make toggle disable on checking the checkbox

I'd say that is the way to go so things are consistent between deleting a freshly created entry and deleting an existing entry

[RomainSanchez]

After giving it a shot several questions arose:

1: disabling via javascript works fine but in wich template should the script be included? 2: Does the script need to handle the case where iCheck would be disabled ? 3: should the script itself live in AdminBundle or DoctrineORMBundle (I need to add classes to form elements in DoctrineORMBundle templates as well) 4: Should the changes be made on other storage bundles as well (mondoDB, PhpCr)?

[greg0ire]

1. the create/edit page should always have it IMO
2. no idea what iCheck is
3. I think it could benefit phpcr too, so AdminBundle… @dbu, thoughts?
4. If you work on the admin bundle, not sure…

[RomainSanchez]

no idea what iCheck is

ICheck is the js plugin used for checkboxes.

"sonata_admin" => array:9 [▼
    "name" => "phones"
    "admin" => ContactAdmin {#3294 ▶}
    "value" => PersistentCollection {#4036 ▶}
    "edit" => "inline"
    "inline" => "table"
    "field_description" => FieldDescription {#3775 ▶}
    "block_name" => false
    "options" => array:15 [▼
      "use_icheck" => true

[greg0ire]

For 2, I'm unsure, I'd say it should work too…

[RomainSanchez]

If iCheck is enabled i need to listen to their ifChanged event instead of change but window.SONATA_CONFIG has the info so problem solved.

[dbu]

inline editing is useful for the phpcr admin too, so if we can fix the infrastructure for that, please do it here in the base admin bundle

[braianj]

Hi, is there I documentation on how to use it? Didn't find it

[MatTheCat]

I cannot find any activity on this matter after #4441 got merged so I guess this issue should be reopened.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[kirya-dev]

Im suggest solving in any issue: Before set data we delete fisrt. @VincentLanglet you are closed not that issue. Problem solving here: https://github.com/sonata-project/SonataAdminBundle/issues/6391#issuecomment-709232369

[jordisala1991]

I have followed @kirya-dev suggestions:

https://github.com/sonata-project/form-extensions/pull/414

Can someone confirm it works for their use case? cc / @VincentLanglet