Using `AND` (`&`) with a predicate that returns `None` incorrectly returns `True` about django-rules HOT 5 OPEN

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 to the old behaviour as it is a breaking change however I'm willing to be convinced otherwise if others feel the change makes sense.

from django-rules.

Ah, I see you have this note in the README, which I apparently missed:

You may skip evaluation by returning None from your predicate

So I guess this is by design! Conditionals like the one I described above return obj and obj.boolean_field are dangerous then (though this wasn't known/obvious to me), since they're falsey on their own but skipped in combined predicates. (And None is falsey in other Python contexts, of course, so I wouldn't have assumed this behavior.) For instance, setting the following will result in "read" returning "no permission" if the sometimes_returns_none predicate returns None, but "add" returning "has permission" if the user is staff (even if sometimes_returns_none has returned None):

        rules_permissions = {
            "read": sometimes_returns_none,
            "add": rules.is_staff & sometimes_returns_none,
        }

I'll leave this open for now for any discussion. Thanks again.

from django-rules.

It may be worth at least moving "Skipping predicates" up to the "Combining predicates" portion of the doc so it's more discoverable, since it's important in that context (rather than it being under "Advanced features").

from django-rules.

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

from django-rules.

Yeah, that could be useful. I also noticed in the "Upgrading from 1.x" section of the docs, it mentions that skipping used to be done with raising a SkipPredicate exception. That more explicit approach seems significantly more preferable/safer to me, particularly for a security-oriented library, where a mistake in allowing access can be rather severe. (And then all truthy/falsey conditions can behave more in line with what's typical in python.)

from django-rules.