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

Automatic tree structure generation from passed array #34

Closed
ghost opened this issue Oct 2, 2013 · 24 comments
Closed

Automatic tree structure generation from passed array #34

ghost opened this issue Oct 2, 2013 · 24 comments
Labels

Comments

@ghost
Copy link

ghost commented Oct 2, 2013

Okay, as you asked me to further explain my needs and also suggested to @kapooostin to create a new issue, I'll do this now. (here is the source: #26)

So, when I create a new node/entity I wanna specify the direct parent and the package should create all the corresponding closure-table (or tree)-entries completely automatically.

That for, I had that solution:
My entities or "nodes"-table had a "parent_id"-column.

// Parent-ID
$table->integer('parent_id')->unsigned()->nullable();
$table->foreign('parent_id')->references('id')->on('nodes');

Any my corresponding model had this boot()-method:

public static function boot()
    {
        parent::boot();
        static::created(function($model)
        {
            $model->createTreeEntry();
            $model->save();
        });
    }

And this is the "createTreeEntry()"-method:

/**
* Creates all the necessary entries in the NodeTree
*/
public function createTreeEntry()
{
    $tree = static::$nodeTree;
    $ancestors = array_reverse($this->getAncestors());

    foreach($ancestors as $ancestor)
    {
        $tree::create(
            array(
                'ancestor' => $ancestor->id,
                'descendant' => $this->id,
                'level' => $this->calculateLevel()              
            )
        );
    }
}

So, whenever I created a new node, I specified it's immediate parent (or null if it was a root) and then all the corresponding entries where created completely automatically.

like this:

Node::create(
'parent_id' => '7'
'description' => 'Child of node with id 7 (contact)'
);

That´s something I really find convenient and need, but I haven't yet figured out on how I would implement this into "ClosureTable 2" since you took a different approach on a few areas.

Would be great if we could find a way to implement this automatic approach.

Kindest regards,
The Dancing Bard

@franzose
Copy link
Owner

franzose commented Oct 3, 2013

Direct ancestor? If I understand you right, then you might want take a look at the appendChild() method. First, you retrieve ancestor node. For example, Node::find(35). Then, you do Node::appendChild(new Node(['description' => 'Child of the node with id 35'])). Is that what you want?

On the other hand, if you want to create a tree from an array, something like @kapooostin suggested, then it's a different story. :) Your comments appreciated.

@ghost
Copy link
Author

ghost commented Oct 3, 2013

I know about "appendChild()", but what I would need would be something like "appendAncestor()" (or appendThisToNode())

But then again you would need to make another query manually to set everything right.

And I definitely would like to see this to be automatic like it is in my package right now.
It just seems rather unnecessarily complicated to me, that first you create an Entity then query for another and append the first one to the latter.

I think this should be as comfortable as possible.

Hence I provided code as to how I did it ;)
I'm sorry, currently I just lack the time to dive into development any more. Too much going on in real life.

@franzose
Copy link
Owner

franzose commented Oct 3, 2013

Oh, I understand your idea for now!

@ghost
Copy link
Author

ghost commented Oct 4, 2013

@franzose Great.
I'm sorry, English isn't my mothertongue and when explaining (technical) stuff I guess it (and I) gets too complicated to follow very easily :(

@franzose
Copy link
Owner

franzose commented Oct 4, 2013

@thedancingbard, your conversation is good ) and the English is not ‘mine’ too :) it was misunderstanding of the conception.

@ghost
Copy link
Author

ghost commented Oct 4, 2013

@franzose: So do you consider to implement this feature?

I don't know how to implement this automatism without a "parent_id"-column in the "node/entity"-table.

@franzose
Copy link
Owner

franzose commented Oct 4, 2013

@thedancingbard, now i'm trying to fix other things, but this is interesting and useful too. And I will implement it someday. I think it can be implemented by playing with the closure table somehow, not the entity table.

@vjandrea
Copy link
Contributor

vjandrea commented Oct 4, 2013

In a different approach, following @kapooostin suggestion i worked on this https://gist.github.com/vjandrea/6825675
It actually echoes the code that should be executed, but in this way you can see that it recurses a tree correctly. I'm working to implement it in Entity.php

@franzose
Copy link
Owner

franzose commented Oct 4, 2013

@vjandrea, great, great, great! and i'm struggling with that fucking position column...

@franzose
Copy link
Owner

franzose commented Oct 4, 2013

@vjandrea, I think the method could be called createFromArray. I suppose it's a self-explanating name. How do you think?

@vjandrea
Copy link
Contributor

vjandrea commented Oct 4, 2013

@franzose, i agree for the name of the method. Regarding the issues with the position column, following the documentation, should be declared unsigned not null, so that's why we get exceptions when we try to set negative values.

@franzose
Copy link
Owner

franzose commented Oct 4, 2013

@vjandrea, update your package, I have finally fixed all known (for me) bugs with position...

@vjandrea
Copy link
Contributor

Just a little update, i started working on this branch: https://github.com/vjandrea/ClosureTable/tree/feature_34 to enable these methods:

  • createFromArray() (when ids are not given)
  • updateStructureFromArray() (when ids are given)

and two other methods that i will use for testing import/export of different structures

  • exportJson()
  • exportArray()

As soon as i get rid of the errors in phpunit i'll go on with the development of these functions.

@franzose
Copy link
Owner

Where you would like to put the exportJson and exportArray? As for me, I would implement it in Franzose\ClosureTable\Collection class, similar to toArray and change export to to. And I would, if you will, strip the Structure from the update method. Just to keep similarity. Just imho :)

@franzose
Copy link
Owner

As for JSON and Array, we should discuss them at #55.

@franzose
Copy link
Owner

@vjandrea along the way I ponder about versioning. We have five versions already, but they are all buggy.

@vjandrea
Copy link
Contributor

@franzose, for the names it's fine, so if i understood we shall have:
Entity class: createFromArray() and updateFromArray()
Collection class: toJson() and toArray().
I announced everything here because it's all linked: i want to be able to export a collection to an array and serialize it, or to a JSON string, then destroy everything and import from the serialized data to see if it the import functions works correctly, anyway i agree to discuss toArray and toJson in #55.
About the bugs, i agree to focus on them, anyway i asked you if you could send me the error dump of phpunit to see if i can point the remaining issues, but since you didn't answer on that i decided to go on with the rest.
A final note on version name, i'm used to name branches as feature_(issuenumber) when they're not directly linked to a specific version yet, and to pull in just when 100% ready in the current dev branch. In this way i can avoid clashes of version numbers with upstream repos.

@franzose
Copy link
Owner

i asked you if you could send me the error dump of phpunit to see if i can point the remaining issues, but
Sorry, I forgot to do that due to lack of time. I'll try to do that to the weekend.

@franzose
Copy link
Owner

What do you think about milestone? I haven't used the feature yet. Would it be useful?

@vjandrea
Copy link
Contributor

I have seen milestones used in larger projects because you they allow to fix deadlines and priorities for bug fixing, but i think that in this project it would be overkill at the moment. Anyway if you want to try it to see how it works, why not?

@franzose
Copy link
Owner

@vjandrea, as for reducing of the tests errors, there's nothing special. In make() method, I just changed $attributes[static::POSITION] = $model->guessPositionOnCreate(); to $attributes[static::POSITION] = 0;. I haven't committed that yet, because we have problems with wrong number of the siblings of a root node. I'm trying to fix that.

@franzose
Copy link
Owner

@vjandrea any news?

@vjandrea
Copy link
Contributor

I'm working on it, but i have a hard time debugging because xdebug still doesn't want to work.

@franzose
Copy link
Owner

franzose commented Mar 5, 2014

@kapooostin, @thedancingbard, @vjandrea, I have some news. I have just finished the feature. Please, take a look at v3.0-dev.

@franzose franzose closed this as completed Mar 5, 2014
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