djangorestframework-stubs
djangorestframework-stubs copied to clipboard
change get_permissions to return list
I have made things!
Proposal to change the signature of get_permissions to list[BasePermission]. This allows one to customize the permissions of a view as suggested by the documentation: https://www.django-rest-framework.org/api-guide/permissions/#overview-of-access-restriction-methods
The source code also suggests that is is a list:
https://github.com/encode/django-rest-framework/blob/f56b85b7dd7e4f786e0769bba6b7609d4507da83/rest_framework/views.py#L278
It was originally changed in #320 to Sequence[_SupportsHasPermission]. ~I am not sure if the element type has to be _SupportsHasPermission instead of BasePermission. All the other method signatures use the base type.~
Update: In the end I reverted back for the same reason why the return type was changed to Sequence in #320 and added a few more test cases.
Related issues
Refs #319 (PR: #320)
I found a case that supports _SupportsHasPermission instead of BasePermission:
def get_permissions(self) -> Sequence[_SupportsHasPermission]:
if self.action == 'list':
return [OR(IsAdminUser(), SomeOtherPermission())]
return super().get_permissions()
I had to revert back to Sequence. Given that in some scenarios (see added test cases) _SupportsHasPermission has to be exposed I think it would be best to make it part of the public API. Thoughts?
Thanks. I'll have a look in the evening. (feel free to ping if I forget)
I'm not using the DRF permission system myself, so I'm not very familiar with it.
Is it worth investigating support for boolean operators as well, e.g. IsAuthenticated & IsAdminUser, or are they deprecated? I think the | operator is particularly problematic because it conflicts with PEP 604 type union operator.
The test / stubtest (3.12) failure is unrelated to your PR, it's due to upstream PR https://github.com/typeddjango/django-stubs/pull/1771