lazychaser / laravel-nestedset

Effective tree structures in Laravel 4-8
3.66k stars 472 forks source link

adding a multi trees in 1 table feature? #79

Closed boynet closed 8 years ago

boynet commented 8 years ago

I have a use case of like 2000 categories with endless sub categories but it never gonna be too big most will be 3-8 deep sub categories like must 100 items in each tree instead of creating 1 big tree with 2000 children's I want to create from each category a new nestedset in same table. column like nestedset_id that it will create new nestedset based on nestedset_id? so for example this nestedset: https://upload.wikimedia.org/wikipedia/commons/thumb/4/41/NestedSetModel.svg/701px-NestedSetModel.svg.png

we can simplify it by saying that mens is new nestedset and womens is new nestedset so like the women set will be left=1 and right=12 and men's sets will be 1-8 so a change in the men nestedset will not change anything in the women sets and vise versa so we should get pretty good insert performance

lazychaser commented 8 years ago

This can be achieved using global scopes. I have to note that while this is applicable to splitting tree by gender, I wouldn't recommend to devide it depeer since you won't be able to select ancestors\descendants quickly

boynet commented 8 years ago

thanks I will look Into it

adding global sceope like "where nd_id == 1" will work it out?

lazychaser commented 8 years ago

Something like this:

public function apply(Builder $builder, Model $model)
{
        return $builder->where('nestedset_id', '=', $model->nd_id);
}

But note that now you can't do this: Node::firstOrFail() (or any other query), you need to specify nd_id attribute:

Node::newInstance([ 'nb_id' => 1 ])->firstOrFail();
maxvaser commented 8 years ago

@lazychaser Thanks for the great library!

Adding the ability to easily make multiple NS trees on the same table would be so useful for things like creating multiple menus or multitenancy. I have tried multiple ways to get the above examples to work. It only seems to work if I hard code the FK ID into that global scope where statement. I couldn't get regular query scoping to work either.

Could you add something similar to the way the Baum Nested Set library implements a scope column? It works, but is not as good as yours in all the other methods like getting roots or moving nodes up or down.

https://github.com/etrepat/baum#scope-support

Scope support

The Baum library provides a simple method to provide Nested Set "scoping" which restricts what we consider part of a nested set tree. This should allow for multiple nested set trees in the same database table.

To make use of the scoping funcionality you may override the scoped model attribute in your subclass. This attribute should contain an array of the column names (database fields) which shall be used to restrict Nested Set queries:

class Category extends Baum\Node {
  ...
  protected $scoped = array('company_id');
  ...
}

In the previous example, company_id effectively restricts (or "scopes") a Nested Set tree. So, for each value of that field we may be able to construct a full different tree.

$root1 = Category::create(['name' => 'R1', 'company_id' => 1]);
$root2 = Category::create(['name' => 'R2', 'company_id' => 2]);

$child1 = Category::create(['name' => 'C1', 'company_id' => 1]);
$child2 = Category::create(['name' => 'C2', 'company_id' => 2]);

$child1->makeChildOf($root1);
$child2->makeChildOf($root2);

$root1->children()->get(); // <- returns $child1
$root2->children()->get(); // <- returns $child2

All methods which ask or traverse the Nested Set tree will use the scoped attribute (if provided).

lazychaser commented 8 years ago

@maxvaser scoping (multitenancy) is a more general feature, it has nothing to do with nested sets. I will look into this problem. But I conviced that global scope is the solution.

If you need several menus, you can just reserve root nodes as entry points.

maxvaser commented 8 years ago

@lazychaser Thank you very much! I think you're right. I think that's probably what the protected $scoped = array('company_id'); does if it's defined on the Baum library.

I couldn't figure out how to make a query like the example above on the global scope Node::newInstance([ 'nb_id' => 1 ])->firstOrFail(); work like the example below or how to create nodes that way.

It would be nice to be able to define a column(s) as to which to treat as the key for independent nested set trees. as it currently stands I could get almost all of it to work with something like this using a query scope on the Company and Menu models to define the hasMany relationships, But couldn't get the ordering to work correctly:

$company = App\Company::find(1);
$menu = $company->menus()->find(5); // menu() is a query scope on the Company Model
$category = $menu->categories()->create(['name' => 'item 1', 'company_id' => 1, 'menu_id' => 5 ]);
// id=16
$category = $menu->categories()->create(['name' => 'item 2', 'company_id' => 1, 'menu_id' => 4 ]); 
// id=17
$category = $menu->categories()->create(['name' => 'item 2', 'company_id' => 1, 'menu_id' => 4 ]); 
// id=18
$category = $menu->categories()->create(['name' => 'item 2', 'company_id' => 1, 'menu_id' => 5 ]);
// id=19
$category = $menu->categories()->create(['name' => 'item 3', 'company_id' => 1, 'menu_id' => 5 ]); 
// id=20

Using this methodology to lock a tree to the company and menu does not work to make an individual tree for that company id and that menu id.

$company = App\Company::find(1);
$menu = $company->menus()->find(5);
$category = $menu->categories()->find(20);  // categories() is the query scope on the Menu model
$category->up();
// or
$category->down();

It moves the node up/down the whole tree and reorganizes the whole table as a nested set. So, you have to move it past other items with a different "menu_id" before it would move up/down if there is another item that was inserted under a different company or menu id.

lazychaser commented 8 years ago

Try global scopes with v4.1.0@dev

maxvaser commented 8 years ago

Ok. I Installed "kalnoy/nestedset": "v4.1.x-dev",

I have a global scope defined and added to: Category extends Node

    public function apply(Builder $builder, Model $model)
    {
        return $builder->where('menu_id', '=', $model->menu_id);
    }

I'm getting an error trying to initiate it with the above example.

// Node::newInstance([ 'nb_id' => 1 ])->firstOrFail();
Category::newInstance([ 'menu_id' => 5 ])->firstOrFail();

"Non-static method Illuminate\Database\Eloquent\Model::newInstance() should not be called statically"

$category = new Category; 
$menu = $category->newInstance([ 'menu_id' => 5 ])->first();
dd($menu);

returns null

If I hard code the id into the global scope it returns a result set.

    public function apply(Builder $builder, Model $model)
    {
        return $builder->where('menu_id', '=', 5);
    }

Am I doing something wrong there?

lazychaser commented 8 years ago

Added scoping feature in 4.1.0@dev.

maxvaser commented 8 years ago

@lazychaser Thank you for adding this enhancement!!!

It works to create() a new node as a root or with parent. But, not for update() where you try to move it from having a parent to a root or from a root to have a parent or if you try to move up() or down().

Here's part of the error output For trying to move a descendant node to a root using the $node->saveAsRoot() method:

ModelNotFoundException in QueryBuilder.php line 40:

    in QueryBuilder.php line 40
    at QueryBuilder->getNodeData('603', true) in QueryBuilder.php line 58
    at QueryBuilder->getPlainNodeData('603', true) in QueryBuilder.php line 452
    at QueryBuilder->moveNode('603', '25') in NodeTrait.php line 577
    at Node->moveNode('25') in NodeTrait.php line 559
    at Node->insertAt('25') in NodeTrait.php line 158
    at Node->actionRoot()
    at call_user_func_array(array(object(Category), 'actionRoot'), array()) in NodeTrait.php line 119
    at Node->callPendingAction() in NodeTrait.php line 50
    at Node::Kalnoy\Nestedset\{closure}(object(Category))
    at call_user_func_array(object(Closure), array(object(Category))) in Dispatcher.php line 221
    at Dispatcher->fire('eloquent.saving: App\Models\Category', array(object(Category)), true) in Dispatcher.php line 164
    at Dispatcher->until('eloquent.saving: App\Models\Category', object(Category)) in Model.php line 1678
    at Model->fireModelEvent('eloquent.saving: App\Models\Category') in Model.php line 1465
    at Model->save() in NodeTrait.php line 375
    at Node->**saveAsRoot()** in CategoriesController.php line 208

I've also tried $node->makeRoot()->save(); method and tried just setting the parent_id to an empty value and NULL too.

Moving up() or down() methods:

 in QueryBuilder.php line 40
at QueryBuilder->getNodeData('599', true) in QueryBuilder.php line 58
at QueryBuilder->getPlainNodeData('599', true) in QueryBuilder.php line 452
at QueryBuilder->moveNode('599', '17') in NodeTrait.php line 577
at Node->moveNode('17') in NodeTrait.php line 559
at Node->insertAt('17') in NodeTrait.php line 220
at Node->actionBeforeOrAfter(object(Category), false)
at call_user_func_array(array(object(Category), 'actionBeforeOrAfter'), array(object(Category), false)) in NodeTrait.php line 119
at Node->callPendingAction() in NodeTrait.php line 50
at Node::Kalnoy\Nestedset\{closure}(object(Category))
at call_user_func_array(object(Closure), array(object(Category))) in Dispatcher.php line 221
at Dispatcher->fire('eloquent.saving: App\Models\Category', array(object(Category)), true) in Dispatcher.php line 164
at Dispatcher->until('eloquent.saving: App\Models\Category', object(Category)) in Model.php line 1678
at Model->fireModelEvent('eloquent.saving: App\Models\Category') in Model.php line 1465
at Model->save() in NodeTrait.php line 507
at Node->insertBeforeNode(object(Category)) in NodeTrait.php line 525
at Node->up() in CategoriesController.php line 358
lazychaser commented 8 years ago

The system tries to get _lft and _rgt values of your node in a separate query while applying the scopes. Show me the original code where you're trying to saveAsRoot.

Also, you shure that you've removed global scope that was under consideration earlier in this topic?

maxvaser commented 8 years ago

Yes, I've removed the global scoping off the model. On the Category Model I have the getScopeAttributes() method added.

For update to move to root I've tried the following:

$category = Category::scoped([ 'menu_id' => '8' ])->findOrFail($id);
$category->update([
    'title' => $title,
    'slug' =>  str_slug($title)
]);

$category->saveAsRoot();
// or
$category->makeRoot()->save();
// or 
$category->update([
    'title' => $title,
    'slug' =>  str_slug($title),
    'parent_id' => ''  // and NULL
]);

I also tried $category->saveAsRoot(); and $category->makeRoot()->save(); alone and before the update statement.

For up() and down():

$category = Category::scoped([ 'menu_id' => '8' ])->findOrFail($id);
$category->up();
// or
$category->down();

I have verified that $category is returning an object in every instance and it is.

For Create this works without the scoping attribute, but scopes the tree correctly:

$category = Category::create([
     'menu_id' => '8',
    'title' => $title,
    'slug' =>  str_slug($title)
])->saveAsRoot();

This fails and does not work:

$category = Category::scoped([ 'menu_id' => '8' ])->create([
     'menu_id' => '8',
    'title' => $title,
    'slug' =>  str_slug($title)
])->saveAsRoot();

Thanks!

lazychaser commented 8 years ago

You don't need to apply scoping when you're fetching model by primary key (since it's unique).

Calling update saves the model, calling saveAsRoot will save it again (two queires instead of one).

Calling Category::create saves node as root by default, calling saveAsRoot again does nothing.

Category::create is a static method on model, Category::scoped returns query builder, so doing Category::scoped()->crrate() is wrong.

And sorry, I can't reproduce your issue, this test succeedes:

public function testCase()
    {
        $node = MenuItem::scoped([ 'menu_id' => 1 ])->findOrFail(5);

        $node->update([ 'title' => 'test' ]);

        $node->saveAsRoot();
    }

And btw this is the right way of doing this:

public function testCase()
    {
        $node = MenuItem::findOrFail(5);

        $node->fill([ 'title' => 'test' ])->saveAsRoot();
    }
maxvaser commented 8 years ago

I couldn't get it to work if I try to move position or from parents to roots or vice-a-versa no matter what I tried. Not sure why. but, I still the scoping is a very useful feature. Thanks for adding it and your help!