Deleting an item from the top does not remove the corresponding Entity but the bottom one

[Starojitski]

I have a list of objects that refer to other objects

[ CourseParticipant: id1 ]->[Student A, id11]
[ CourseParticipant: id2 ]->[Student B, id22]
[ CourseParticipant: id3 ]->[Student C, id33]

after attempting to delete [ CourseParticipant: id1 ] instead of this:

[ CourseParticipant: id2 ]->[Student B, id200]
[ CourseParticipant: id3 ]->[Student C, id300]

I get :

[ CourseParticipant: id1 ]->[Student B, id200]
[ CourseParticipant: id2 ]->[Student C, id300]

The thing is that i have other entity ParticipantResults that associate to CourseParticipant and should cascade delete on deletion of CourseParticipant.

[ParticipantResults: id111 ]->[ CourseParticipant: id1 ]
[ParticipantResults: id222 ]->[ CourseParticipant: id2 ]
[ParticipantResults: id333 ]->[ CourseParticipant: id3 ]

And what happens after (attempted) deletion of [ CourseParticipant: id1 ]:

[ParticipantResults: id111 ]->[ CourseParticipant: id1 ]
[ParticipantResults: id222 ]->[ CourseParticipant: id2 ]

Now if i want to know results of Student B i get those of removed Student A. And the results of Student C are gone

[ninsuo]

Can you try to reproduce your issue on the demo website?

This is hard to help you without more code, at least your form type and the javascript code that loads the collection.

Thanks!

[Starojitski]

The demo site does not use entities. The fruits don't have ids. But before we dive into the code, can you explain something? i have a collection that in json would look like this

[
    {"id": 1, "user": 7, "role": "manager"},
    {"id": 2, "user": 9, "role": "student"},
    {"id": 3, "user": 16, "role": "student"}
]

When i remove the first item and post changes the data that is sent to the server looks like this:

------ FormBoundary
Content-Disposition: form-data; name="track_manage_type[trackUsers][0][User]" 9
------ FormBoundary
Content-Disposition: form-data; name="track_manage_type[trackUsers][0][role]" student

------ FormBoundary
Content-Disposition: form-data; name="track_manage_type[trackUsers][1][User]" 16
------ FormBoundary
Content-Disposition: form-data; name="track_manage_type[trackUsers][1][role]" student

------ FormBoundary
Content-Disposition: form-data; name="track_manage_type[_token]" WDWUTz7S08-uRV-J0FVP7fBSMYCkt-GKbZUFc26R7_Y
------ FormBoundary

I see no ids of the three entities. How does the backend knows which entity i actually removed. It seems that the HandleRequest() just overwrites the first 2 entities in the collection with the properties of these two items. And it does not remove the last entity in the collection

[
    {"id": 1, "user": 9, "role": "student"},
    {"id": 2, "user": 16, "role": "student"},
    {"id": 3, "user": 16, "role": "student"}
]

I am trying to figure out who to blame:

• me - because i have not RTFM
• your code - because it does not send ids
• or symfony form - because there is no one else left :)

[durdev]

@Starojitski Probably you will have to use this example: https://symfony-collection.fuz.org/symfony3/basic/positionField

The description says: "This is very useful when each element of your collection has its own database relationships."

[sandermarechal]

That demo (https://symfony-collection.fuz.org/symfony3/basic/positionField) is broken. Try it:

• Remove the top field "walk the dog".
• Hit the "+" button on the last row "yawn loudly" to add a new row.
• Click "See actions" and observe broken result

[sandermarechal]

Anyway, I have the same problem. The cause is that this plugin re-numbers the collection index when you add or remove rows. It should not do that (or, it should have an option to stop it doing that, preferably enabled by default).

E.g, if you have a collection like this:

0: Item 0
1: Item 1

Now if you remove the first item, this plugin will re-number the indexes like so:

0: Item 1

If you submit that to the back-end then Symfony will not delete "Item 0". Instead, it will rename "Item 0" to "Item 1", then delete the original "Item 1".

That's not what is supposed to happen and causes havoc with entities.

[ninsuo]

Hello,

Indeed there's an issue in this case.

For entities, if you wish to keep tracking of id you should use this "position_field" option though, as @DaviUchoa noticed.

Will fix this ASAP.

[sandermarechal]

What if my entities do not have a position field? In my case it's just an unordered collection of entities and I have disabled the move up/down buttons.

[ninsuo]

@sandermarechal I understand and am a bit confused, so I'll have a try tonight (at work right now).

@DaviUchoa if you add an empty row after remvoing the first one, there's the row at position 2 missing

[sandermarechal]

@ninsuo I found the source of the bug. In the doAdd function on line 475 you have this:

var freeIndex = elements.length;

When you first remove a row, then add one, it creates a duplicate index.

What you should probably do is when you initialize the collection, find the length and store it in the settings. Everytime you add a row, increment it and use it as the index for the new row.

[ninsuo]

Indeed that's wrong 👍 I'm taking my dinner and will have time to look at your 2 issues. See you later

[ninsuo]

Ok I fixed the first issue: https://github.com/ninsuo/symfony-collection/pull/86

I see that there's also an issue when deleting an item with both allow_down and fade_out enabled.

Fixing it before continuing with your main issue.

[ninsuo]

Ok the display issue is fixed in https://github.com/ninsuo/symfony-collection/pull/87 Now looking at your entity issue.

[ninsuo]

All right. It's a bit too late now so I'll build a "nested collections with doctrine" sample tomorrow. Cheers

[Starojitski]

@DaviUchoa I have tried the 'positionField' approach and it did not help me.

I decided to post a distillation of my code: Students can follow learning tracks:

class Track
{
    /** @ORM\Column(name="id", type="integer"), @ORM\Id, @ORM\GeneratedValue(strategy="AUTO") */
    private $id;

   /**
     * One Track has Many UserTracks.
     * @ORM\OneToMany(targetEntity="AppUserTrack", mappedBy="track", cascade={"persist"})
     */
    private $trackUsers;
..

Many AppUsers can be associated with a Track via OnToMany trackUsers property that is a collection of AppUserTrack entities:

class AppUserTrack
{
    /** @ORM\Column(name="id", type="integer"), @ORM\Id, @ORM\GeneratedValue(strategy="AUTO") */
    private $id;

    /**
     * Many UserTracks have One AppUser.
     * @ORM\ManyToOne(targetEntity="AppUser", inversedBy="userTracks")
     * @ORM\JoinColumn(onDelete="CASCADE", nullable=false)
     */
    private $user;

    /**
     * Many UserTracks have One Track.
     * @ORM\ManyToOne(targetEntity="Track", inversedBy="trackUsers", fetch="EAGER")
     */
    private $track;

    /** @ORM\Column(type="string") */
    private $role;
}

In order to manage track users there is a controller:

    public function manageUsersAction(Track $track, Request $request)
    {
        $form = $this->createForm(TrackManageUsersType::class, $track);
        $form->handleRequest($request);

        if ($form->isSubmitted()) {
            if ($form->isValid()) {
                $this->getDoctrine()->getManager()->flush();
            } 

            return new JsonResponse([]);
        }

        return $this->render('track-manage-users.html.twig', array(
            'track' => $track,
            'form' => $form->createView(),
        ));
    }

where TrackManageUsersType looks like this

class TrackManageUsersType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $track = $builder->getData();
        $builder->add('trackUsers', CollectionType::class, [
                'entry_type' => TrackUserType::class,
                'by_reference' => true,
                'delete_empty' => true,
                'required' => false,
                'allow_add' => true,
                'allow_delete' => true,
                'prototype' => true,
                'prototype_name' => '__track_user__',
            ]);
    }

    public function configureOptions(OptionsResolver $resolver)
    {
        $resolver->setDefaults(array(
            'data_class' => Track::class
        ));
    }

    public function getBlockPrefix()
    {
        return 'track_type';
    }
}

I defined a TrackUserType as this:

class TrackUserType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder
// this was my attempt of using position to preserve the entities from swapping properties
//            ->add('position', HiddenType::class, [
//                'attr' => [
//                    'class' => 'my-position', // selector is the one used on the js side
//                ]
//            ])
            ->add('User', EntityType::class, [
                'choice_label' => 'name',
                'placeholder' => 'Choose User',
                'class' => AppUser::class,
                'query_builder' => function (EntityRepository $er) use ($track) {
                    ...
                }
            ])
            ->add('role', ChoiceType::class, [
                'label_attr' => ['class' => 'hidden'],
                'placeholder' => 'Choose Role',
                'choices' => [
                    'Student' => AppUserTrack::ROLE_STUDENT,
                    'Manager' => AppUserTrack::ROLE_MANAGER
                ],
                'expanded' => false,
                'multiple' => false,
            ]);
    }
   ...
}

All of this is initialized in js:

...
$trackUsers.collection({
//            attempted to use position to preserve the entities from swapping properties
//            position_field_selector: '.my-position',
//            allow_duplicate: true,
            add_at_the_end: true,
            hide_useless_buttons: true,
            allow_up: false,
            allow_down: false,
            add: '<button type="button" class="btn btn-sm btn-default"><span class="fa fa-fw fa-plus"></span></button>',
            remove: '<button type="button" class="btn btn-sm btn-default"><span class="fa fa-fw fa-trash"></span></button>'
        });

I hope you can do something with this!!!!

[ninsuo]

All right, so that's one collection of entities for which you have dependent entities. Going to build a demo; thus if there's a bug I'll see it. A

[sandermarechal]

The first issue still isn't properly fixed. It works when you remove the first row, then add a new row. But now it doesn't work if you remove the last row and then add a new row. The new row now has the same index as the row that was removed (causing Symfony to try to edit that entry, instead of deleting the old one and adding the new one).

The problem is that you are searching for a new freeIndex. You should not do that. When starting the collection you should find the largest index. E.g:

settings.lastIndex = max(indexes); // or something

Then, when adding a new row, always increase the index. E.g:

settings.lastIndex++;
var newRowIndex = settings.lastIndex;
// add new row here

Thanks to your changes in #86 I now know what to look for in your code, so I'll see if I can come up with a PR myself :-)

[dkop]

Deleting of entities must be processed at symfony on backend. Docs are here https://symfony.com/doc/3.4/form/form_collections.html#template-modifications