FJNR-inc / dry-rest-permissions

Rules based permissions for the Django Rest Framework
ISC License
75 stars 11 forks source link

feat: Fallback to django permissions if no method #14

Closed zumoshi closed 2 years ago

zumoshi commented 3 years ago

Django's django.contrib.auth has a perfectly functional permission system for global checks. I don't see any reason not to use it if we don't need more complex logic. (For an example of reinventing the wheel for something achievable by Django's auth see this StackOverflow question)

This can be achieved using the current version of this library by defining methods as shown below:

@staticmethod
def has_create_permission(request):
    return request.user.has_perm('app.add_model')

@staticmethod
def has_read_permission(request):
    return request.user.has_perm('app.view_model')

@staticmethod
def has_update_permission(request):
    return request.user.has_perm('app.change_model')

@staticmethod
def has_destroy_permission(request):
    return request.user.has_perm('app.delete_model')

In the spirit of keeping it DRY, I propose adding another class DRYOrDjangoPermissions which automatically falls back to Django permissions if custom methods weren't present, removing the need for duplicate code for each model.

Since this is a separate class it won't change the behavior of DRYPermissions base class and thus is backward compatible.

Feel free to suggest changes or a better approach.

zumoshi commented 3 years ago

I will add tests/docs if you approve the idea. This is more of a request for comment at the moment.

RignonNoel commented 3 years ago

Hi @zumoshi !

Thanks a lot for the Pull-Request, i think it's a great idea and it could be a good enhancement for the package to be more easy to integrate on existing project that already use basic django permissions.

I do not see any reason to not let you go ahead with that, feel free to add documentation and testing on this feature and we will be able to integrate it :)

zumoshi commented 3 years ago

Any idea why the Travis check failed? It says all the tests passed but had some warnings, which if I understand correctly are because I imported the Permissions model? But the ContentType model was imported the same way.

RignonNoel commented 3 years ago

It's a Flake8 error (linting)

dry_rest_permissions/generics.py:145:41: F999 f-string is missing placeholders
dry_rest_permissions/generics.py:145:100: F999 f-string is missing placeholders
Running flake8 code linting
flake8 failed

You just need to use place holder around your two f-string like in this example

RomainFayolle commented 2 years ago

This PR has been open for a year and a half. If someone wants to take the comments and finish it feel free to do so, but since it's it's not a requested feature and nobody has asked to finish it I will close it.