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 180 forks source link

Storing new Inlines not working in version 2 #312

Closed 21adrian1996 closed 2 years ago

21adrian1996 commented 2 years ago

I am trying to store a new object in an SortableInlineAdminMixin:

from django.contrib import admin

@admin.register(MyOtherModel)
class MyOtherModelAdmin(admin.ModelAdmin, SortableAdminBase):
    inlines = [MyInline]

class MyInline(SortableInlineAdminMixin, admin.TabularInline):
    model = MyModel
    extra = 3
    ordering = ('order',)

Throws the error getattr(): attribute name must be string in adminsortable2/admin.py, line 461, in save_new on line order_field_value = getattr(obj, self.default_order_field, None)

The configuration works with 1.0.4 but not with 2.0.4.

Is there a new configuration or what am I missing?

jrief commented 2 years ago

Could you please fork the testapp and configure it in a way, so that I can reproduce that error. Please read section on how to report bugs.

germanp commented 2 years ago

I'm having the same issue. But it works ok when the inline model is a ManyToManyField. I also tried setting the default_order_field in the inline.

ahmedsafadii commented 2 years ago

same issue

ahmedsafadii commented 2 years ago

there is no request even goes in the network when finish the draging

jrief commented 2 years ago

Inlines have to be saved explicitly. This was already the case for version 1.

ahmedsafadii commented 2 years ago

@jrief any fix?

jrief commented 2 years ago

@ahmedsafadii I have no intentions to change this, so it probably will remain forever.

im-n1 commented 1 year ago

Just ran into this issue and I can confirm there is a real bug.

In __init__() method here you allow default_order_field to be None. But later in save_new() method here you call this getattr(obj, self.default_order_field, None) which in case of default_order_field is None raises an exception since getattr() requires to be a string:

In [2]: getattr(object(), "")
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Input In [2], in <cell line: 1>()
----> 1 getattr(object(), "")

AttributeError: 'object' object has no attribute ''

In [3]: getattr(object(), None)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Input In [3], in <cell line: 1>()
----> 1 getattr(object(), None)

TypeError: getattr(): attribute name must be string
jrief commented 1 year ago

@im-n1 could you please give me full instructions on how to reproduce this bug.

im-n1 commented 1 year ago

Sadly I can't because it happens on a project I'm just a small part and I don't have a full understanding of the whole party. However just looking into your code the issue is obvious. I guess it can be fixed right away.

jrief commented 1 year ago

@im-n1 then fork this repository, adopt one of the examples so that you can reproduce it there, and tell me where I can find it on GitHub.

If you want me to fix rarely occurring bugs on OSS, then you have to invest some time as well.

alchemistOfWeb commented 1 year ago

I had the simular problem. I solved it by putting SortableAdminBase as first in inheritance. incorrect: class MyOtherModelAdmin(admin.ModelAdmin, SortableAdminBase): correct class MyOtherModelAdmin(SortableAdminBase, admin.ModelAdmin):

im-n1 commented 1 year ago

Thats what I already have. But it doesn't fix the obvious bug in code.

jrief commented 1 year ago

@im-n1 how about my proposal from December?

im-n1 commented 1 year ago

@jrief sadly I have no time to do that.

topiaruss commented 1 year ago

Surely ordering = ('order',) should be under class Meta?

class Meta:
    ordering = ("order")
BigglesZX commented 1 year ago

In case it's helpful to anyone, I also experienced this error this morning, and as per @alchemistOfWeb's comment, my case was due to inheritance order of my admin class (in combination with django-solo which provides SingletonModelAdmin).

Incorrect:

@admin.register(models.Homepage)
class HomepageAdmin(SingletonModelAdmin, SortableAdminBase):
    # ...

Correct:

@admin.register(models.Homepage)
class HomepageAdmin(SortableAdminBase, SingletonModelAdmin):
    # ...

Hope this helps.

jrief commented 1 year ago

@BigglesZX would you like to create a page in the docs section with recipes, when combining django-admin-sortable2 with other libraries?

BigglesZX commented 1 year ago

@jrief Sure, I'll try and look at that later today.