dfunckt comments

Results 99 comments of


                                            dfunckt

Return rule denial reasons (and improved integration with rest framework)

I think providing messages at predicate-level a bit too fine-grained and feels like overloading the predicates. I'm not strictly opposed to your suggestion, but I'd have to see a real...

Return rule denial reasons (and improved integration with rest framework)

I think a good approach would be to allow a predicate to fail by raising a specific type of exception instead of returning `false` -- `rules` could catch that, turn...

Using `AND` (`&`) with a predicate that returns `None` incorrectly returns `True`

I think a truth table for all possible combinations of operations and return values close to the "Combining predicates" section would be very useful.

Using `AND` (`&`) with a predicate that returns `None` incorrectly returns `True`

I agree it's more "Pythonic" to raise an exception and I have unfortunately no recollection why I made the switch to None. I don't think it's sensible to revert back...

Add `verbose_name` for rules and permissions

Great stuff! 👍 My only concern is we're changing the structure of the items stored in rule sets -- RuleSets used to be a map of strings to predicates, whereas...

Enhancement - Human readable name for rules/permissions

This sounds great to me and it's definitely desirable given Django's native perms also have this ability. I also like the API. Only question is what `verbose_name` should be if...

Having trouble adding rule model mixin. -- TypeError: 'class Meta' got invalid attribute(s): rules_permissions

You need to specify the custom metaclass, see that part of the README again.

django.contrib.sites integration

Passing the request sounds much more useful, but I wouldn't want to make it a required argument. The request could be passed as a context argument however. For plain rules,...

added two DRF generic views to the check in get_permission_object()

This looks good, I guess, but it can’t really be importing “rest_framework” like that as it’ll break things for everyone else that only needs to use rules with Django. What’s...

added two DRF generic views to the check in get_permission_object()

On the other hand, we don’t declare a dependency on Django as well, so the second option might not be too much of a stretch to include.