Summary

Add the foundations for user-customizable condition rules.

Review Request #8268 — Created July 6, 2016 and submitted July 11, 2016, 3:45 p.m.

Information

Owner

chipx86

Repository

Djblets

Branch

release-0.10.x

Bugs

Depends On

Commit

6f2c4c1...

Reviewers

Groups

djblets

People

Description

This change introduces the core support for condition rules. These are a
way to allow applications to give users a degree of flexibility for
choosing when certain actions should take place. Users can define one or
more "conditions," which consist of a "condition choice" (a property or
an object, generally, that the condition is matched upon), an "operator"
(which applies to a choice -- something like "is", "starts with",
"greater than", etc.), and a value (depending on the type of choice and
operator). Along with these, they can decide whether all conditions must
match, or any of them.

For example, if an application wants to post notifications to a chat
service, it could allow the user to customize under what conditions a
message would be sent, making it easy to have a range of per-channel
configurations.

Conditions can be serialized to a JSON-compatible form (useful for
storage in JSONFields or elsewhere), and deserialized from them as well.
This makes them usable just about anywhere, including in API requests.

Consumers would create a set of condition choices they want to expose
for some part of their application. Each choice has an ID (unique within
that batch of choices), a human-readable name, a list of one or more
operators, and an optional default value field (which handles
representing input fields on a web UI and handling
serialization/deserialization).

Operators, similarly, have an ID and a name, and can optionally override
the value field (allowing, for instance, some operators for a date-based
choice to show calendar entries or numeric text fields, whichever is
more appropriate). It also contains the logic for matching a stored
condition value with the lookup value from the caller.

Both the list of choices and list of operators are registries, which
allows other code (such as extensions) to add additional
choices/operators to any existing list. Through this, authors could add
even more flexibility for conditions beyond what the application author
intended, helping to match on specific types of metadata or state in the
database or anything else the author wants.

This is just the beginning of this support. Upcoming changes will add
standard sets of choices, operators, and values, and will introduce
standard UI for configuration conditions.

Testing Done

Unit tests pass.

Tested this along with the upcoming changes, adding working condition
support in an extension.

Issues

Description	From	Last Updated
"instance" is superfluous. The most pythonic way would be to have a property called operators but get_operators would be OK …	david	July 6, 2016, 8:04 p.m.
Same here: a choices property would be best but get_choices would be OK too.	david	July 6, 2016, 8:04 p.m.
I think it would be clearer if this said it's used internally by the registry framework. As it was I …	david	July 6, 2016, 7:56 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	July 6, 2016, 7:53 p.m.
Perhaps set the condition_index on the existing exception and re-raise?	david	July 6, 2016, 7:57 p.m.
Hmm. I don't know if you designed it that way but it looks like you could add a ConditionSet as …	david	July 6, 2016, 8:05 p.m.
I think it would be clearer if this said it's used internally by the registry framework. As it was I …	david	July 6, 2016, 7:58 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	July 6, 2016, 7:53 p.m.

Tool: Pyflakes
Processed Files:
    djblets/conditions/tests/test_values.py
    djblets/conditions/conditions.py
    djblets/conditions/choices.py
    djblets/conditions/tests/test_choices.py
    djblets/conditions/operators.py
    djblets/conditions/values.py
    djblets/conditions/errors.py
    djblets/conditions/__init__.py
    djblets/conditions/tests/test_conditions.py
    djblets/conditions/tests/test_operators.py

Ignored Files:
    docs/djblets/guides/registries/index.rst
    djblets/conditions/tests/__init__.py
    docs/djblets/coderef/index.rst



Tool: PEP8 Style Checker
Processed Files:
    djblets/conditions/tests/test_values.py
    djblets/conditions/conditions.py
    djblets/conditions/choices.py
    djblets/conditions/tests/test_choices.py
    djblets/conditions/operators.py
    djblets/conditions/values.py
    djblets/conditions/errors.py
    djblets/conditions/__init__.py
    djblets/conditions/tests/test_conditions.py
    djblets/conditions/tests/test_operators.py

Ignored Files:
    docs/djblets/guides/registries/index.rst
    djblets/conditions/tests/__init__.py
    docs/djblets/coderef/index.rst

djblets/conditions/conditions.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
djblets/conditions/tests/test_values.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```

djblets/conditions/choices.py (Diff revision 1)

The issue has been resolved. Show all issues

"instance" is superfluous. The most pythonic way would be to have a property called operators but get_operators would be OK too.

chipx86 July 6, 2016, 8:05 p.m.

The choice owns an operators registry, which stores a list of classes, not instances. Instances are created on demand in several places, but we only do that when we need to work with the operator(s) in question. Previously, I had callers pull from the registry and instantiate, but then it had to handle grabbing the class, creating an instance, and passing in the choice. get_operator_instances is designed as a convenience around that.

I can rename it to get_operators, though. More consistent with ConditionOperators.get_operator.

djblets/conditions/choices.py (Diff revision 1)

The issue has been resolved. Show all issues

Same here: a choices property would be best but get_choices would be OK too.

chipx86 July 6, 2016, 8:05 p.m.

ConditionChoices is what would normally be the result of a choices property (and is in some upcoming code). Like above, it stores the classes, and that's often the only thing a caller needs to deal with. This method is about getting the instances for it. However, I can rename to get_choices().

djblets/conditions/choices.py (Diff revision 1)

The issue has been resolved. Show all issues

I think it would be clearer if this said it's used internally by the registry framework. As it was I was looking through the code to figure out what it was used for.

djblets/conditions/conditions.py (Diff revision 1)

The issue has been resolved. Show all issues

Perhaps set the condition_index on the existing exception and re-raise?

djblets/conditions/conditions.py (Diff revision 1)

The issue has been dropped. Show all issues

Hmm. I don't know if you designed it that way but it looks like you could add a ConditionSet as one of the conditions...

chipx86 July 6, 2016, 8:05 p.m.

While you could technically try to make it work by forcing a ConditionSet into the list of conditions, other things will fail when serializing/deserializing due to lack of nested deserialization support and missing attributes. It's meant to be consistent in terms of method naming, though, so that you can run a match on a whole ConditionSet or on individual Conditions.

djblets/conditions/operators.py (Diff revision 1)

The issue has been resolved. Show all issues

I think it would be clearer if this said it's used internally by the registry framework. As it was I was looking through the code to figure out what it was used for.

Change Summary:


Renamed BaseConditionChoice.get_operator_instances to get_operators.
Renamed BaseConditionChoice.get_instances to get_choices.
Clarified docs for get_defaults for registry subclasses.
Updated an InvalidConditionValueError to set a property on an exception and re-raise, instead of raising a new exception.
Fixed long lines.

Commit:

7e7977d1ddc963068e92e1b13b09eb1e50753bad

6f2c4c116756818f2b3f6a5489ad819efe31d7c8

Diff:

Revision 2 (+1844)

Show changes

	djblets/conditions/__init__.py
	djblets/conditions/choices.py
	djblets/conditions/conditions.py
	djblets/conditions/errors.py
	djblets/conditions/operators.py
	djblets/conditions/values.py
	djblets/conditions/tests/__init__.py
	djblets/conditions/tests/test_choices.py
	5 more

Tool: PEP8 Style Checker
Processed Files:
    djblets/conditions/tests/test_values.py
    djblets/conditions/conditions.py
    djblets/conditions/choices.py
    djblets/conditions/tests/test_choices.py
    djblets/conditions/operators.py
    djblets/conditions/values.py
    djblets/conditions/errors.py
    djblets/conditions/__init__.py
    djblets/conditions/tests/test_conditions.py
    djblets/conditions/tests/test_operators.py

Ignored Files:
    docs/djblets/guides/registries/index.rst
    djblets/conditions/tests/__init__.py
    docs/djblets/coderef/index.rst



Tool: Pyflakes
Processed Files:
    djblets/conditions/tests/test_values.py
    djblets/conditions/conditions.py
    djblets/conditions/choices.py
    djblets/conditions/tests/test_choices.py
    djblets/conditions/operators.py
    djblets/conditions/values.py
    djblets/conditions/errors.py
    djblets/conditions/__init__.py
    djblets/conditions/tests/test_conditions.py
    djblets/conditions/tests/test_operators.py

Ignored Files:
    docs/djblets/guides/registries/index.rst
    djblets/conditions/tests/__init__.py
    docs/djblets/coderef/index.rst

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-0.10.x (ae24ad0)