[Sortable] [Symfony] Strange behaviour when adding new elements to collection

[kacperjurak]

Hi,

I use Symofny collections and render entity form with related collection of another entities. These entities have hidden field 'position', and i sort them using jQuery UI. After sorting i set (via JS) new 'position' values. Then i submit the form. Symfony runs setPosition() on every of these entities and then execute persist() and flush(). Everything works well if i sort existing collection, but when i add (via JS) another collection element (or elements) and submit (with proper 'position' values 0, 1, 2, 3....) then strange things happen. Some of 'position' fields in database are duplicated. It looks like one can sort entities only if they are flushed to DB, but Symfony call setPosition() on entities before persist() and flush().
Also the second thing - i think if there is mistake in positions (for example duplicated values) then Sortable should be able to 'repair' it and change numbering.

Best Regards,
Kacper

[l3pp4rd]

Hi, I think you need to determine what is the exact issue and why first. I do not have time to dive in and try to recreate the use case you are facing. If you find an issue with sortable, replicate it with a failing test case in a pull request. I remember there was an issue opened with javascript forms, maybe it is similar try to search through the issues.

[fancyweb]

@l3pp4rd

Hello, I'm just facing the exact same problems atm. I have a Sortable entity that is used in a collection and I manage its position manually via an hidden field. I did some digging about these bugs :

The insert problem

When you insert a new entity and you change positions of others at the same time, you end up with duplicated positions because the updates are processed before the inserts (cf https://github.com/Atlantic18/DoctrineExtensions/blob/master/lib/Gedmo/Sortable/SortableListener.php#L69). So the maxPosition in the "processUpdate" function will be wrong because it doesn't count the entities that will be inserted. If you just insert an entity and let it at the last position there is no problem.

Example : You have a collection of three entities, you insert a new one and you put this new one to "position 0". So one of the existing entity is now at "position 3". The updates are ran first so the calculated maxPosition at this moment is 2 (because it just does a MAX query and there was only three entities in the collection). So the "position 3" is not accepted and is brought back to "position 2" by a min function call... So you end up with two "position 2" in the db.

The update problem

When you just change positions of existing entities you can end up with duplicated positions. This bug comes from the relocation that gets applied while it is not needed. I don't have more informations because I don't have the time to understand the relocation system...

Example : You have a collection of four entities. You move the first one ("position 0") to "position 2", the second one ("position 1") to "position 0" and the third one ("position 2") to "position 1". You end up with two "position 0" in the db and zero "position 1".

[l3pp4rd]

thanks @fancyweb hope someone who works with it can contribute. I have stopped working with php over a year now.

[fancyweb]

I added failing tests on my fork (fancyweb@42e5d29) if it can help.

[raitocz]

I assume that this is the same problem, right?

<?php

namespace AppBundle\Entity\Manual;

use AppBundle\Entity\Identifier;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Validator\Constraints as Assert;
use Gedmo\Mapping\Annotation as Gedmo;

/**
 * @ORM\Table(name="manual_pages")
 * @ORM\Entity(repositoryClass="Gedmo\Sortable\Entity\Repository\SortableRepository")
 */
class Manual
{
    use Identifier;

    /**
     * @ORM\Column(type="string")
     * @Assert\NotBlank(message="Toto pole musí být vyplněno")
     */
    private $title;

    /**
     * @ORM\Column(type="text")
     * @Assert\NotBlank(message="Toto pole musí být vyplněno")
     */
    private $content;

    /**
     * @ORM\OneToMany(targetEntity="AppBundle\Entity\Manual\ManualImage", mappedBy="manual")
     * @ORM\OrderBy({"position"="ASC"})
     */
    private $images;

    /**
     * @Gedmo\SortablePosition
     * @ORM\Column(type="integer", nullable=false)
     */
    private $position;

    /**
     * @return mixed
     */
    public function getPosition()
    {
        return $this->position;
    }

    /**
     * @param mixed $position
     */
    public function setPosition($position)
    {
        $this->position = $position;
    }


    /**
     * @return mixed
     */
    public function getTitle()
    {
        return $this->title;
    }

    /**
     * @param mixed $title
     */
    public function setTitle($title)
    {
        $this->title = $title;
    }

    /**
     * @return ManualImage[]
     */
    public function getImages()
    {
        return $this->images;
    }

    /**
     * @param ManualImage[] $images
     */
    public function setImages($images)
    {
        $this->images = $images;
    }

    /**
     * @return mixed
     */
    public function getContent()
    {
        return $this->content;
    }

    /**
     * @param mixed $content
     */
    public function setContent($content)
    {
        $this->content = $content;
    }


}

When i proceed to change position by one the sorting behavior is acting OK:

$entity->setPosition($entity->getPosition() + 1);
// or
$entity->setPosition($entity->getPosition() - 1);

But when I've implemented JS drag&drop to change positions the whole thing gets weird. For example, having this table:

id    | position
1     | 0
2     | 1
3     | 2
4     | 3
5     | 4
6     | 5

when I do for row with id 6 this:

$newPosition = $entity->getPosition() - 5; // = 0
$entity->setPosition($newPosition);

the table changes to this:

id    | position
1     | 2
2     | 3
3     | 4
4     | 5
5     | 5
6     | 0

There is nothing for position 1 but position 5 is occupied twice.

[Hartiikz]

Same issue... If someone have an idea to fix this, i'm more than interrested !

[vcraescu]

Same issue here.

[ksn135]

👍

[jiripetrzelka]

I have encountered a similar problem as raitocz describes - duplicate positions after update. I have determined that the preUpdate method of the SortableListener was called twice because the SortableListener was registered twice. Then I have removed these lines from config.yml:

stof_doctrine_extensions:
orm:
default:
sortable: true

The listener is then registered only once and everything works fine. (I made sure that there is no stof_doctrine_extensions in other config entries.) It seems that the Listener gets registered in some other way and this config is not necessary.

(I'm using Symfony 3.3.13.)

[raitocz]

@Werken Sounds good, I will give it a try on my projects and then confirm that

[christingruber]

@jiripetrzelka Works like a charm!

[Herz3h]

@jiripetrzelka ty very much !

[Arkemlar]

Disabling the listener just disables sortable behavior in my case and this bug is still exists.. wow. Wondering why did I not face it earlier.

[Herz3h]

There is situation where you need to add it to config.yml, and then situation where you need to remove it from there (to avoid bug in this thread). I have dropped usage of this extension :/

[martijndwars]

The solution by @jiripetrzelka works when I update existing entities. However, when I create a new entity, I set the position to -1 in the constructor. According to the documentation, this means that upon persisting the entity, it is added to the end of the list. However, now that the sortable extension is disabled, it is simply persisted with position -1. This seems to be one of the cases where you should not disable the extension.

[Antal1609]

The solution provided by @jiripetrzelka does not work with Symfony 4. Omitting it, setting it to false or leaving it in the config file does not change the outcome.

[jaapromijn]

I spent a day on this problem in an existing project. Seems that the problem in our case was caused by the persist on an existing entity (Because that is how the update entity function in the EasyAdmin package works):

    protected function updateEntity($entity)
    {
        $this->em->persist($entity);
        $this->em->flush();
    }

Just using the $this->em->flush(); instead seems to fix the problem.

[FireLizard]

I don't no why but inserting a new entity with position = -1 at first then (after $em->flush()) update this entity with the needed position it works.
Directly set the needed position and only call flush once does not work.

[github-actions]

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.