order_insertion_by field not always being checked correctly.

[GoogleCodeExporter]

The pre_save signal handler does not check if the order_insertion_by value
has been changed. order_insertion_by is only checked when the instance is
first inserted into the DB and when the parent is changed.

Original issue reported on code.google.com by Gerry.Ei...@gmail.com on 10 Jan 2009 at 5:40

[GoogleCodeExporter]

agree. it works for me if I add after line 110 in mptt/signals.py:

       old_order = getattr(instance._default_manager.get(pk=instance.pk),
                            'order')
        order = getattr(instance, 'order')

        if parent != old_parent or order != old_order:
            setattr(instance, opts.parent_attr, old_parent)

Original comment by michael.martinides@gmail.com on 1 Feb 2009 at 9:43

[GoogleCodeExporter]

Could the new rebuild function be called when items are updated? Like in 
issue#13

Original comment by gregory....@gmail.com on 15 Dec 2009 at 5:10

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Don't know whether this is helpful or if this bug is going to be fixed anytime 
soon,
but my patch here works with any fields defined in order_insertion_by.

However, if the parent stays the same while the node should be moved to the end
(according to order_insertion_by), it is actually not moved – that seems to 
be a bug
in move_node()? 

        # PATCH replaces these lines:
#        old_parent = getattr(instance._default_manager.get(pk=instance.pk),
#                             opts.parent_attr)
#
#        if parent != old_parent:
        saved_instance = instance._default_manager.get(pk=instance.pk)
        old_parent = getattr(saved_instance, opts.parent_attr);
        was_changed = parent != old_parent
        if not was_changed and opts.order_insertion_by:
            for field in opts.order_insertion_by:
                if getattr(instance, field) != getattr(saved_instance, field):
                    was_changed = True
                    break
        if was_changed:
        # /PATCH

Original comment by samuel.l...@gmail.com on 11 Feb 2010 at 4:17

[GoogleCodeExporter]

I see 5 people starred this issue. Can someone describe a use-case for changing 
`order_insertion_by` after the model has been registered?

Otherwise this is likely to be closed as wontfix.

Original comment by craig.ds@gmail.com on 4 Sep 2010 at 12:54

• Changed state: Chatting

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

As far as I understand, the issue here is not changing the value of 
`order_insertion_by`, but the value of a field listed in `order_insertion_by`.

Use case: Suppose you have a tree of categories ordered by name. You add a 
category, and later change its name and save it again. Now the category's 
position won't be updated by mptt's `pre_save` handler, because it only checks 
whether the category's parent has been changed.

Original comment by samuel.l...@gmail.com on 6 Sep 2010 at 12:23

[GoogleCodeExporter]

Thanks Samuel, I missed that when reading this the first time

Original comment by craig.ds@gmail.com on 6 Sep 2010 at 7:54

• Changed state: Accepted

[GoogleCodeExporter]

Original comment by craig.ds@gmail.com on 6 Sep 2010 at 9:18

• Added labels: Milestone-Release0.4

[GoogleCodeExporter]

Initial fixes: http://github.com/django-mptt/django-mptt/commit/796e9094

 * Not sure on the best way to do this - please critique
 * not merged yet, needs cleanup. Might still do it a completely different way

Original comment by craig.ds@gmail.com on 9 Sep 2010 at 12:47

[GoogleCodeExporter]

Cleaned up that branch a bit, prevented it from traversing foreign keys, and 
merged it.

Please test the latest from master and report any issues.

Original comment by craig.ds@gmail.com on 12 Sep 2010 at 10:24

• Changed state: Fixed