DoctrineExtensions

DoctrineExtensions copied to clipboard

I've tried this example in my application (copy-paste) and it doesn't print what it's supposed to.

Expected output (according to docs):

0: item 0
1: item 1
2: item 2

Actual output:

0: item 1
0: item 0
1: item 2

Version: Gedmo 2.3.9 + Doctrine 2.4.4

Is something missing in the docs or is it a bug?

It works as expected if I add flush after each persist so I think it really is a bug.

The problem is probably that SortableListener::getMaxPosition() doesn't check entities sheduled for insertion.

what if make flush after first and last persist?

In that case the output is different but still buggy:

0: item 0
1: item 1
1: item 2

I can confirm this behaviour. Anyone working on this "bug"?

can you make unit test?

I don't know how to contribute the right way. I there is any advice share it with me plz. For now i will paste them here:

 /**
     * @test
     */
    public function shouldFixIssue1130()
    {
        $item1 = new Node();
        $item1->setName('Node1');
        $item1->setPath('category 1');
        $this->em->persist($item1);

        $item2 = new Node();
        $item2->setName('Node2');
        $item2->setPath('category 1');
        $this->em->persist($item2);

        $item0 = new Node();
        $item0->setName('Node0');
        $item0->setPath('category 1');
        $item0->setPosition(0);
        $this->em->persist($item0);

        $this->em->flush();
        $this->em->clear(); // to reload from database  

        $repo = $this->em->getRepository(self::NODE);
        $nodes = $repo->getBySortableGroups(array('path' => 'category 1'));

        $this->assertEquals('Node0', $nodes[0]->getName());
        $this->assertEquals('Node1', $nodes[1]->getName());
        $this->assertEquals('Node2', $nodes[2]->getName());

        for ($i = 0; $i < count($nodes); $i++) {
            $this->assertSame($i, $nodes[$i]->getPosition());
        }
    }

    /**
     * @test
     */
    public function shouldFixPositionTooHighAfterInsert()
    {
        $item1 = new Node();
        $item1->setName('Node1');
        $item1->setPath('category 1');
        $this->em->persist($item1);

        $this->em->flush();

        $item2 = new Node();
        $item2->setName('Node2');
        $item2->setPath('category 1');
        $this->em->persist($item2);


        $item0 = new Node();
        $item0->setName('Node0');
        $item0->setPath('category 1');
        $item0->setPosition(0);
        $this->em->persist($item0);

        $this->em->flush();
        $this->em->clear(); // to reload from database  

        $repo = $this->em->getRepository(self::NODE);
        $nodes = $repo->getBySortableGroups(array('path' => 'category 1'));

        $this->assertEquals('Node0', $nodes[0]->getName());
        $this->assertEquals('Node1', $nodes[1]->getName());
        $this->assertEquals('Node2', $nodes[2]->getName());

        for ($i = 0; $i < count($nodes); $i++) {
            $this->assertSame($i, $nodes[$i]->getPosition());
        }
    }

Results:

1) Gedmo\Sortable\SortableTest::shouldFixIssue1130
Failed asserting that 0 is identical to 1.

/Users/thore/Projects/doctrine-laravel/tests/Gedmo/Sortable/SortableTest.php:603

2) Gedmo\Sortable\SortableTest::shouldFixPositionTooHighAfterInsert
Failed asserting that 1 is identical to 2.

A quick and Dirty Solution: Within OnFLush in SortableListener around Line 80:

$orderedObjects = [];
        foreach ($ea->getScheduledObjectInsertions($uow) as $object) {
            $meta = $om->getClassMetadata(get_class($object));
            if ($config = $this->getConfiguration($om, $meta->name)) {
                if(! is_null($meta->getReflectionProperty($config['position'])->getValue($object)))
                {
                    array_unshift($orderedObjects, $object);  
                    continue;  
                }
                $orderedObjects[] = $object;
            }
        }
        foreach ($orderedObjects as $object) {
            $meta = $om->getClassMetadata(get_class($object));
            if ($config = $this->getConfiguration($om, $meta->name)) {
                $this->processInsert($ea, $config, $meta, $object);
            }
        }

solution: Put all objects with a specified position in front of the processed array, so the deltas will effect the memory. this is a quick and dirty fix to the implementation inside porcessInsert

Next failing Test (my code and the initial code fail here):

/**
     * @test
     */
    public function shouldFixPositionOtherThenZero()
    {

        $item0 = new Node();
        $item0->setName('Node0');
        $item0->setPath('category 1');
        $this->em->persist($item0);


        $item2 = new Node();
        $item2->setName('Node2');
        $item2->setPath('category 1');
        $this->em->persist($item2);

        $item1 = new Node();
        $item1->setName('Node1');
        $item1->setPath('category 1');
        $item1->setPosition(1);
        $this->em->persist($item1);

        $this->em->flush();
        $this->em->clear(); // to reload from database  

        $repo = $this->em->getRepository(self::NODE);
        $nodes = $repo->getBySortableGroups(array('path' => 'category 1'));

        $this->assertEquals('Node0', $nodes[0]->getName());
        $this->assertEquals('Node1', $nodes[1]->getName());
        $this->assertEquals('Node2', $nodes[2]->getName());

        for ($i = 0; $i < count($nodes); $i++) {
            $this->assertSame($i, $nodes[$i]->getPosition());

        }
    }

I would flash() after each persist()

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.

Still not solved afaik.

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.

I had the same problem. The SortableListener was registered twice but has to run only once.

I had the same problem. The SortableListener was registered twice but has to run only once.

Could you advise on how to do so please ?

I had the same problem. The SortableListener was registered twice but has to run only once.

Could you advise on how to do so please ?

I just get all listeners of event "onFlush" and look for the existence of SortableListener.

$listeners = array_map(
    static fn($listener) => get_class($listener),
    $em->getEventManager()->getListeners('onFlush')
);
if ( ! in_array(SortableListener::class, $listeners, true)) {
    $em->getEventManager()->addEventSubscriber(new SortableListener());
}