FJNR-inc / dry-rest-permissions

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

Custom message/code support #30

Closed mjaquiery closed 8 months ago

mjaquiery commented 9 months ago
Q R
Type of contribution ? (fix, enhancement, ..) Enhancement
Tickets (issues) concerned #27

What have you changed ?

Added support for returning non-boolean values from permission methods.

How did you change it ? (architectural consideration)

Return values for DRYPermissions.has_permission() and DRYPermissions.has_object_permission() are now run through DRYPermissions._process_permission_result() where non-boolean values are scraped for message/code information. That information is used to update self.message and self.code, and boolean False is returned. This means that Django's generic APIView.permission_denied() method recieves the message and code for further processing.

Is there a special consideration?

None that I'm aware of - should integrate with Django's preferred approach.

Verification :

mjaquiery commented 9 months ago

I recommend squashing if/when we merge because the commits don't really make sense separately.

FYI I had to update pytest to get the tests to run, so I hope they also run in the travis CI workflow.

mjaquiery commented 9 months ago

@RignonNoel are you able to review these changes? It's a very helpful library and it'd be good to be able to use the updated version cleanly in my project if the changes are suitable for general release.

mjaquiery commented 8 months ago

@RignonNoel @MelanieFJNR is anyone from FJNR watching this repository? I'm grateful you're officially supporting this project but I would appreciate a response to this PR, please.

MelanieFJNR commented 8 months ago

Hi @mjaquiery , Thank you for your patience and your contribution. I will ask my colleague @RomainFayolle to review your PR during the week.

mjaquiery commented 8 months ago

Thanks @RomainFayolle, @MelanieFJNR, and thanks again for adopting this useful project!

RomainFayolle commented 8 months ago

@mjaquiery thanks for the good work, and sorry for the delay. It is a useful PR, I like having the possibilities to add more details when possible. I Approved the changed but I would like to double check with @RignonNoel before merging, should be done today if it's all good.

RignonNoel commented 8 months ago

Thanks @mjaquiery! We will prepare a new release and make that available on Pypi asap.