Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with createFromArray method #242

Open
alexbabich1990 opened this issue Oct 28, 2020 · 2 comments
Open

Problem with createFromArray method #242

alexbabich1990 opened this issue Oct 28, 2020 · 2 comments
Labels

Comments

@alexbabich1990
Copy link

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?

@franzose franzose added the bug label Oct 28, 2020
@franzose
Copy link
Owner

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

@alexbabich1990
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants