typeddjango / django-stubs

PEP-484 stubs for Django
MIT License
1.58k stars 437 forks source link

admin.TabularInline: Signature of "has_add_permission" incompatible with supertype "BaseModelAdmin" #469

Open JoaRiski opened 4 years ago

JoaRiski commented 4 years ago

What's wrong

The signature for the admin.TabularInline.has_add_permission method seems to be wrongly typed, resulting in the following error when a subclass declares a type according to the Django documentation:

admin.TabularInline: Signature of "has_add_permission" incompatible with supertype "BaseModelAdmin"

Example signature resulting in this error:

    def has_add_permission(self, request: HttpRequest, obj: Model) -> bool:
        return False

The problem seems to be caused by the TabluarInline directly inheriting the types from here https://github.com/typeddjango/django-stubs/blob/19c73a106dc918606f8a9c3db018ccd3f18c1e06/django-stubs/contrib/admin/options.pyi#L109

When in reality the InlineModelAdmin has a different signature for the has_add_permission function: https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#django.contrib.admin.InlineModelAdmin.has_add_permission

How is that should be

The has_add_permission should be typed specifically for the InlineModelAdmin class rather than inheriting from BaseModelAdmin.

It would be a good idea to verify other has_x_permission methods at the same time, as the inline admin functions might differ from the base model admin

System information

JoaRiski commented 4 years ago

Loosely related to #203

sobolevn commented 4 years ago

@JoaRiski PRs are appreciated!

JoaRiski commented 4 years ago

@sobolevn I don't have the time to properly test it, but opened #470

lachlancannon commented 2 years ago

I think the types are wrong in addition to this. If we look at the Django docs https://docs.djangoproject.com/en/4.0/ref/contrib/admin/#django.contrib.admin.InlineModelAdmin.has_add_permission the inlines are defined as being passed the parent object, not the current object, but the types generated by django-stubs complain if I don't specify the type as InlineType | None

e.g.

class ScheduledChangeLogFieldInline(admin.TabularInline[ScheduledChangeLogField]):      
    model = ScheduledChangeLogField                                                   
    fields = ["field_name", "old_value", "new_value"]                                 

    def has_change_permission(                                                           
        self, request: HttpRequest, parent: ScheduledChangeLogField | None = None      
    ) -> bool:                                                                                     
        return False

If I change parent to ScheduledChangeLog, which it should be, I get:

base/admin.py:106: error: Argument 2 of "has_change_permission" is incompatible with supertype "BaseModelAdmin"; supertype defines the argument type as "Optional[ScheduledChangeLogField]"  [override]

The same applies to has_delete_permission incidentally.

bobwhitelock commented 2 years ago

Is this the same issue as https://github.com/typeddjango/django-stubs/issues/874, and if so was this also fixed by https://github.com/typeddjango/django-stubs/pull/909?