sehughes / django-treebeard

Automatically exported from code.google.com/p/django-treebeard
Apache License 2.0
0 stars 0 forks source link

Allow add_child/add_sibling not to save() the node #16

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I've run into this a couple of times - some times I need 
{{{add_child}}}/{{{add_sibling}}}/{{{add_root}}} to return an unsaved node with 
everything set up for 
saving. This could be easily achieved using a {{{commit=False}}} named 
parameter to the method.

This would be consistent with the way model forms behave:
[{{form.save(commit=False)}}}

Is there a reason this couldn't be added?

Original issue reported on code.google.com by eallik on 7 Jun 2009 at 1:13

GoogleCodeExporter commented 9 years ago
eallik:

I agree with you that the behavior you describe would be a lot more 
"djangonic". But what happens with 
something like this?

{{{
root = MyNode.add_root()
child1 = root.add_child()
child2 = child1.add_child()
child3 = child2.add_child()
child4 = child3.add_child()
child5 = child4.add_child()
child5.move(child2)
child1.delete()
}}}

Having that _without_ the db would mean storing a copy of the tree (or part of 
it) in memory and then 
commiting the changes (something particulary horrible with Nested Sets trees). 
That's adding (even more) 
complexity to the library. Or imagine you're adding a subtree to a node, 
treebeard stores it in RAM until 
you're ready to commit, but before that, another thread removes or changes the 
parent node. What happens 
now? Making treebeard handling all that would be both complex and error prone 
for non-expert users. The 
current behavior IMO handles most use cases very well.

Original comment by gpicon on 14 Jun 2009 at 3:16

GoogleCodeExporter commented 9 years ago
(ugh no code formatting on issue comments)

Original comment by gpicon on 14 Jun 2009 at 3:18

GoogleCodeExporter commented 9 years ago
I agree with what you've said, but I'm still requesting you to add the option 
to specify commit=False for more 
advanced users. The docs could be explicit that developers need to know what 
they're doing when using this. I 
personally need this a lot as it saves me a roundtrip to the database. I don't 
see a reason why a manual 
commit=False would be a bad thing - the problems you've describe can happen to 
any model, not just tree 
nodes.

Actually, even a check could be implemented - if a node is unsaved, certain 
operations cannot be done on it, 
for example adding children. But this would be optional how I see it.

Original comment by eallik on 14 Jun 2009 at 3:34