Skip to content

Add InlineModelAdmin.has_add_permission definition #470

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

JoaRiski
Copy link

Add a proper definition for the InlineModelAdmin's has_add_permission method.

Fixes #469

NOTE: This change is entirely untested, so take extra care when reviewing. I'm unsure if the type check works in stub inheritance the same way it does in actual code, but if yes it could be this change will hit the same "incompatible with supertype" error as mentioned in #469

Add a proper definition for the InlineModelAdmin's has_add_permission method.

Fixes typeddjango#469
Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sobolevn sobolevn closed this Nov 25, 2021
@sobolevn sobolevn reopened this Nov 25, 2021
@JoaRiski
Copy link
Author

Looks like this needs to entirely remove the function from BaseModelAdmin and implement the ModelAdmin specific variant directly to ModelAdmin, as signature changes in subclasses don't seem to be allowed.

Would that sound reasonable?

Improve how `has_add_permission` is declared by removing it from `BaseModelAdmin` and adding it directly to `ModelAdmin`. This is required as `InlineModelAdmin` has a different signature, see typeddjango#469
@JoaRiski
Copy link
Author

@blueyed made the change proposed above, still untested (just used GitHub's editor), but I don't think there should be an issue unless BaseModelAdmin is used in unexpected places.

Care to re-trigger the workflow?

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe we can use # type: ignore on Inline's version of this method.

I think, that it will be more accurate. In this case, subclasses of BaseModelAdmin would still have has_add_permission method.

@JoaRiski
Copy link
Author

I haven't checked how inheritance works with # type: ignore in the base class. Does that approach allow us to still provide proper type hints for inheritance, or would it make essentially just untyped?

@sobolevn
Copy link
Member

Mypy copies the method definition from parent. It should be typed as we define it. But, we just silence the error there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

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