Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InlineModelAdmin should be generic over both its own model type and parent model type #874

Closed
rolandcrosby-check opened this issue Mar 15, 2022 · 0 comments
Labels
bug Something isn't working

Comments

@rolandcrosby-check
Copy link
Contributor

Bug report

What's wrong

InlineModelAdmin has an attribute parent_model, referring to the parent model change form that the inline is attached to, which is currently typed as Any. This (or a TypeVar with upper bound of Type[Model]) would be OK if that were the only attribute or method depending on the parent model type, but InlineModelAdmin also has several methods (currently lacking stubs) that have parameters that should be typed as an instance of the parent model type.

How is that should be

The methods in question that I've noticed are has_{add,change,delete}_permission - in all of these cases the obj argument "is the parent object being edited" according to Django docs.

I think this could be solved as follows but would like to get some input on whether this is correct or if I'm misunderstanding how Python generics and multiple inheritance work:

  • add a second Model-bounded TypeVar to contrib/options/admin.pyi: _ModelU = TypeVar("_ModelU", bound=Model)
  • make InlineModelAdmin generic in both _ModelT (for the model that the inline represents) and _ModelU (for the parent model)
    • could that be done as class InlineModelAdmin(Generic[_ModelT, _ModelU], BaseModelAdmin[_ModelT])? I'm not sure how multiple generic type parameters work, especially when inheriting from another generic superclass.
  • annotate InlineModelAdmin.parent_model as Type[_ModelU]
  • add type annotations for InlineModelAdmin.has_{add,change,delete}_permission as follows (per Django docs and implementation):
def has_add_permission(self, request: HttpRequest, obj: _ModelU) -> bool: ...
def has_change_permission(self, request: HttpRequest, obj: Optional[_ModelU]) -> bool: ...
def has_delete_permission(self, request: HttpRequest, obj: Optional[_ModelU]) -> bool: ...
@rolandcrosby-check rolandcrosby-check added the bug Something isn't working label Mar 15, 2022
sterliakov added a commit to sterliakov/django-stubs that referenced this issue Mar 26, 2022
sterliakov added a commit to sterliakov/django-stubs that referenced this issue Mar 26, 2022
sterliakov added a commit to sterliakov/django-stubs that referenced this issue Mar 28, 2022
sterliakov added a commit to sterliakov/django-stubs that referenced this issue Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

1 participant