jrief / django-admin-sortable2

Generic drag-and-drop ordering for objects in the Django admin interface
https://django-admin-sortable2.readthedocs.io/en/latest/
Other
770 stars 181 forks source link

Selected ordering is not preserved when using "Save as new" #402

Closed ldeluigi closed 1 week ago

ldeluigi commented 3 months ago

After some debugging I found the source of the issue: https://github.com/jrief/django-admin-sortable2/blob/3cbfa1a0a08078a6ec4dfc0e7ec67bfb9d02aeb0/adminsortable2/admin.py#L489-L491

Basically when saving as new the order passed with the form submission gets discarded because its value is greater than or equal to 0, as per order_field_value >= 0.

Instead, when saving as new after a customization of the field ordering, the selected order should be honored.

nicolochiellini commented 2 months ago

Hello, i came across the same bug while "saving as new" an instance with sorted inlines. The inlines of the new instance are not sortable. I've tried to fix it with "reoder" management command but nothing happen, still unsortable. I've tried to reoder only the inlines but nothing: index are good but still unsortable.

for order, obj in enumerate(articolo_obj.elementoarticolo_set.iterator(), start=1):
    setattr(obj, 'indice', order)
    obj.save()

Somebody has an idea how to do a quick fix while waiting for a patch?

jrief commented 1 week ago

Thanks @ldeluigi for this pull request. Please be patient, I need some time to test this. I am just wondering, if this might cause some regressions, otherwise that comparison wouldn't be the way it is in the first place.

jrief commented 1 week ago

@nicolochiellini @ldeluigi I just tested this in my local environment. In my setups the value of order_field_value is always None and therefore it always works. How is it possible that in your use-case that value is negative?

Anyway, I'm going to merge this, although I have some fears that it might cause a regression.

ldeluigi commented 1 week ago

I have no idea, I tested with the testapp in this repository!

ldeluigi commented 1 week ago

PS. The value is not negative, I just needed the if condition to not pass if it is 0 or greater

ldeluigi commented 1 week ago

The negative check is in case someone messes with the template by overriding in some strange way that would cause negative values to appear

ldeluigi commented 1 week ago

In other words, when saving as new, the order_field_value already contains the desired value and should not be overwritten, that's why I inverted the if condition

jrief commented 1 week ago

Let's see if anyone complains about this after upgrading to version 2.2.4.