ClosureTable icon indicating copy to clipboard operation
ClosureTable copied to clipboard

Problem with createFromArray method

Open alexbabich1990 opened this issue 5 years ago • 2 comments

Hello Hello, i updated the "franzose/ClosureTable" package to ^6 version. I noted one problem, if i use createFromArray() method for creating tree, position always change.

For example: $array = [ [ 'id' => 1, 'title' => 'Much Very White', 'position' => 0, 'url' => 'color-tree', ], [ 'id' => 2, 'title' => 'Car tree', 'url' => 'car-tree', 'position' => 1, ], [ 'id' => 3, 'title' => 'Country tree', 'url' => 'country-tree', 'position' => 2 ], ];

After using createFromArray() method i got:

[ 'id' => 1, 'title' => 'Much Very White', 'position' => 2, 'url' => 'color-tree', ], [ 'id' => 2, 'title' => 'Car tree', 'url' => 'car-tree', 'position' => 0, ], [ 'id' => 3, 'title' => 'Country tree', 'url' => 'country-tree', 'position' => 1 ],

I mean, the position field on all points was changed.

if i update vendor/franzose/closure-table/src/Models/Entity.php

    static::saving(static function (Entity $entity) {
        if ($entity->isDirty($entity->getPositionColumn())) {
            $latest = static::getLatestPosition($entity);

            if (!$entity->isMoved) {
                $latest--;
            }

            $entity->position = max(0, min($entity->position, $latest));
        } elseif (!$entity->exists) {
            $entity->position = static::getLatestPosition($entity);
        }
    });

to

    static::saving(static function (Entity $entity) {
       if (!$entity->exists) {
            $entity->position = static::getLatestPosition($entity);
        }
    });

after that createFromArray() method works correct for me. Could you please check it?

alexbabich1990 avatar Oct 28 '20 14:10 alexbabich1990

Hello, @alexbabich1990! I'll try to look closer into it this week or so.

franzose avatar Oct 28 '20 14:10 franzose

Hello again ;). I use your package in my project and i have unit tests for testing. After upgrading the "franzose/ClosureTable" package to ^6, I have couple errors. I wrote about the first problem yesterday. I notes second problem about the "real_depth" field. This field has been removed from your code in ^6 version.

I updated the franzose/closure-table/src/Models/Entity.php file and now everything works correctly for me.

What i changed:

i updated:

    static::saving(static function (Entity $entity) {
        if ($entity->isDirty($entity->getPositionColumn())) {
            $latest = static::getLatestPosition($entity);

            if (!$entity->isMoved) {
                $latest--;
            }

            $entity->position = max(0, min($entity->position, $latest));
        } elseif (!$entity->exists) {
            $entity->position = static::getLatestPosition($entity);
        }
    });

to

    static::saving(static function (Entity $entity) {
        if ($entity->isDirty($entity->getPositionColumn())) {
            $latest = static::getLatestPosition($entity);

            if($entity->position){
                if ($entity->isMoved) {
                    $latest--;
                }
                $entity->position = max(0, $latest);
            }else{
                if(!$entity->isMoved) {
                    $latest--;
                }
                $entity->position = max(0, min($entity->position, $latest));
            }

        } elseif (!$entity->exists) {
            $entity->position = static::getLatestPosition($entity);
        }

        if ($entity->isMoved === false) {
            $entity->real_depth = $entity->getNewRealDepth($entity->parent_id);
        }

    });

Than i updated:

public function moveTo($position, $ancestor = null)
{
    $parentId = $ancestor instanceof self ? $ancestor->getKey() : $ancestor;

    if ($this->parent_id === $parentId && $this->parent_id !== null) {
        return $this;
    }

    if ($this->getKey() === $parentId) {
        throw new InvalidArgumentException('Target entity is equal to the sender.');
    }

    $this->parent_id = $parentId;
    $this->position = $position;

    $this->isMoved = true;
    $this->save();
    $this->isMoved = false;

    return $this;
}

to:

public function moveTo($position, $ancestor = null) { $parentId = $ancestor instanceof self ? $ancestor->getKey() : $ancestor;

    if ($this->parent_id === $parentId && $this->parent_id !== null) {
        return $this;
    }

    if ($this->getKey() === $parentId) {
        throw new InvalidArgumentException('Target entity is equal to the sender.');
    }

    $this->parent_id = $parentId;
    $this->position = $position;
    $this->real_depth = $this->getNewRealDepth($ancestor);

    $this->isMoved = true;
    $this->save();
    $this->isMoved = false;

    return $this;
}

And i added your old method from L5.4 version.

/**
 * Gets real depth of the new ancestor of the model.
 *
 * @param Entity|int|null $ancestor
 * @return int
 */
protected function getNewRealDepth($ancestor)
{
    if (!$ancestor instanceof EntityInterface) {
        if (is_null($ancestor)) {
            return 0;
        } else {
            return static::find($ancestor)->real_depth + 1;
        }
    } else {
        return $ancestor->real_depth + 1;
    }
}

Maybe I made a mistake somewhere, but my functionality works correctly for me.

Could you please check it and update when you have some free time? Thank you.

alexbabich1990 avatar Oct 29 '20 15:10 alexbabich1990