This change has been marked as completed.

Describe the completed change (optional):

Pushed to release-7.1.x (11a4f1f)

Summary

Add a central registry and hooks for custom condition choices.

Review Request #14280 — Created Jan. 1, 2025 and submitted 3 weeks, 3 days ago

Information

Owner

chipx86*

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

Blocks

14281

Reviewers

Groups

reviewboard

People

Description*

This change introduces a new central review_request_condition_choices
registry, along with a new ReviewRequestConditionChoicesHook that
wraps it.

A central ReviewRequestConditionsField can be used whenever a
ConditionsField is meant to be used with review_request_condition_choices,
enabling custom condition choices from any extension. rbintegrations 4.1
will be updated to use this.

Documentation has been written to help extension authors make use of the
new hook. It only touches upon the basics of creating new choices, and
doesn't touch upon custom operators, but it should be enough to get
people going.

Testing Done

Unit tests passed.

Tested the new central registry with in-progress code in rbintegrations
and Power Pack.

Built the docs. Checked the example code against the unit tests, and
checked for bad links and other errors.

Commits

Summary	ID
Add a central registry and hooks for custom condition choices. When we first wrote the support for condition choices for integrations, we intended for the framework to be extensible, allowing extensions to produce new condition choices that users could select from. However, this didn't work in practice, since we lacked any kind of central registry for condition choices that extensions could hook into. This change introduces a new central `review_request_condition_choices` registry, along with a new `ReviewRequestConditionChoicesHook` that wraps it. With this, any extension can easily introduce one or more new condition choices, which will be picked up automatically when presenting a `ConditionsField` (when used along with the upcoming rbintegrations 4.1 release). Documentation has been written to help extension authors make use of the new hook. It only touches upon the basics of creating new choices, and doesn't touch upon custom operators, but it should be enough to get people going.	34336df74418e4338c3a8735fa4cd518dec3a4f4

Summary

Add a central registry and hooks for custom condition choices.

When we first wrote the support for condition choices for integrations, we intended for the framework to be extensible, allowing extensions to produce new condition choices that users could select from. However, this didn't work in practice, since we lacked any kind of central registry for condition choices that extensions could hook into. This change introduces a new central `review_request_condition_choices` registry, along with a new `ReviewRequestConditionChoicesHook` that wraps it. With this, any extension can easily introduce one or more new condition choices, which will be picked up automatically when presenting a `ConditionsField` (when used along with the upcoming rbintegrations 4.1 release). Documentation has been written to help extension authors make use of the new hook. It only touches upon the basics of creating new choices, and doesn't touch upon custom operators, but it should be enough to get people going.

34336df74418e4338c3a8735fa4cd518dec3a4f4

Issues

Description	From	Last Updated
'reviewboard.extensions.base.Extension' imported but unused Column: 1 Error code: F401	reviewbot	3 months, 2 weeks ago
Should we include type hints in this example?	david	3 months ago
In order to make some linters happier about the lack of docstrings, can we preface this with _?	david	3 months ago
Same here.	david	3 months ago
This needs to be alphabetized.	david	3 months, 2 weeks ago
Doesn't this need a matching _integrations definition?	david	1 month ago
For this and the other choice_id below, I think it'd be good to prefix it like we recommend to do. …	maubin	4 weeks, 1 day ago
There are no open issues

flake8 failed.

JSHint passed.

flake8

reviewboard/extensions/tests/test_review_request_condition_choices_hook.py (Diff revision 1)
The issue has been resolved. Show all issues
```
'reviewboard.extensions.base.Extension' imported but unused

Column: 1
Error code: F401
```

Change Summary:

Removed an unused import.

Commits:

	Summary	ID
	Add a central registry and hooks for custom condition choices. When we first wrote the support for condition choices for integrations, we intended for the framework to be extensible, allowing extensions to produce new condition choices that users could select from. However, this didn't work in practice, since we lacked any kind of central registry for condition choices that extensions could hook into. This change introduces a new central `review_request_condition_choices` registry, along with a new `ReviewRequestConditionChoicesHook` that wraps it. With this, any extension can easily introduce one or more new condition choices, which will be picked up automatically when presenting a `ConditionsField` (when used along with the upcoming rbintegrations 4.1 release). Documentation has been written to help extension authors make use of the new hook. It only touches upon the basics of creating new choices, and doesn't touch upon custom operators, but it should be enough to get people going.	edcffe03efc09593aafe2ecd22c157d25fbd40d7
	Add a central registry and hooks for custom condition choices. When we first wrote the support for condition choices for integrations, we intended for the framework to be extensible, allowing extensions to produce new condition choices that users could select from. However, this didn't work in practice, since we lacked any kind of central registry for condition choices that extensions could hook into. This change introduces a new central `review_request_condition_choices` registry, along with a new `ReviewRequestConditionChoicesHook` that wraps it. With this, any extension can easily introduce one or more new condition choices, which will be picked up automatically when presenting a `ConditionsField` (when used along with the upcoming rbintegrations 4.1 release). Documentation has been written to help extension authors make use of the new hook. It only touches upon the basics of creating new choices, and doesn't touch upon custom operators, but it should be enough to get people going.	9451fe1c2a3b17921c97ee4cb3d2d58754680aa3

Diff:

Revision 2 (+740 -6)

Show changes

	docs/manual/extending/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/review-request-condition-choices-hook.rst
	reviewboard/extensions/hooks/__init__.py
	reviewboard/extensions/hooks/conditions.py
	reviewboard/extensions/tests/test_review_request_condition_choices_hook.py
	reviewboard/reviews/conditions.py
	reviewboard/reviews/forms.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

docs/manual/extending/extensions/hooks/review-request-condition-choices-hook.rst (Diff revision 2)

The issue has been dropped. Show all issues

Should we include type hints in this example?

chipx86 3 months, 2 weeks ago

I've been leaving them out just to keep the examples simple. I figure most people who are doing development probably aren't going to be writing type hints (largely IT and such), and they should still be inheriting the types from the parent methods anyway.

reviewboard/extensions/tests/test_review_request_condition_choices_hook.py (Diff revision 2)

The issue has been resolved. Show all issues

In order to make some linters happier about the lack of docstrings, can we preface this with _?

chipx86 3 months, 2 weeks ago

Which linters have this rule? I haven't seen this. We should standardize any linting tools we're using.

david 3 months, 1 week ago

I don't think this is something we should turn on globally (yet) but these trigger D100-D103 in pydocstyle/ruff

reviewboard/extensions/tests/test_review_request_condition_choices_hook.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/reviews/forms.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This needs to be alphabetized.
```

Change Summary:


Added a _ prefix to internal testing classes.
Fixed import ordering.

Commits:

	Summary	ID
	Add a central registry and hooks for custom condition choices. When we first wrote the support for condition choices for integrations, we intended for the framework to be extensible, allowing extensions to produce new condition choices that users could select from. However, this didn't work in practice, since we lacked any kind of central registry for condition choices that extensions could hook into. This change introduces a new central `review_request_condition_choices` registry, along with a new `ReviewRequestConditionChoicesHook` that wraps it. With this, any extension can easily introduce one or more new condition choices, which will be picked up automatically when presenting a `ConditionsField` (when used along with the upcoming rbintegrations 4.1 release). Documentation has been written to help extension authors make use of the new hook. It only touches upon the basics of creating new choices, and doesn't touch upon custom operators, but it should be enough to get people going.	9451fe1c2a3b17921c97ee4cb3d2d58754680aa3
	Add a central registry and hooks for custom condition choices. When we first wrote the support for condition choices for integrations, we intended for the framework to be extensible, allowing extensions to produce new condition choices that users could select from. However, this didn't work in practice, since we lacked any kind of central registry for condition choices that extensions could hook into. This change introduces a new central `review_request_condition_choices` registry, along with a new `ReviewRequestConditionChoicesHook` that wraps it. With this, any extension can easily introduce one or more new condition choices, which will be picked up automatically when presenting a `ConditionsField` (when used along with the upcoming rbintegrations 4.1 release). Documentation has been written to help extension authors make use of the new hook. It only touches upon the basics of creating new choices, and doesn't touch upon custom operators, but it should be enough to get people going.	7bb67dbaacbfea7ac34ca4b2de8f9e6ddd3ebf9e

Diff:

Revision 3 (+740 -6)

Show changes

	docs/manual/extending/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/review-request-condition-choices-hook.rst
	reviewboard/extensions/hooks/__init__.py
	reviewboard/extensions/hooks/conditions.py
	reviewboard/extensions/tests/test_review_request_condition_choices_hook.py
	reviewboard/reviews/conditions.py
	reviewboard/reviews/forms.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

docs/manual/extending/extensions/hooks/review-request-condition-choices-hook.rst (Diff revision 3)

The issue has been resolved. Show all issues

Doesn't this need a matching _integrations definition?

Change Summary:

Fixed a reference in the docs to the Integrations page.

Commits:

	Summary	ID
	Add a central registry and hooks for custom condition choices. When we first wrote the support for condition choices for integrations, we intended for the framework to be extensible, allowing extensions to produce new condition choices that users could select from. However, this didn't work in practice, since we lacked any kind of central registry for condition choices that extensions could hook into. This change introduces a new central `review_request_condition_choices` registry, along with a new `ReviewRequestConditionChoicesHook` that wraps it. With this, any extension can easily introduce one or more new condition choices, which will be picked up automatically when presenting a `ConditionsField` (when used along with the upcoming rbintegrations 4.1 release). Documentation has been written to help extension authors make use of the new hook. It only touches upon the basics of creating new choices, and doesn't touch upon custom operators, but it should be enough to get people going.	7bb67dbaacbfea7ac34ca4b2de8f9e6ddd3ebf9e
	Add a central registry and hooks for custom condition choices. When we first wrote the support for condition choices for integrations, we intended for the framework to be extensible, allowing extensions to produce new condition choices that users could select from. However, this didn't work in practice, since we lacked any kind of central registry for condition choices that extensions could hook into. This change introduces a new central `review_request_condition_choices` registry, along with a new `ReviewRequestConditionChoicesHook` that wraps it. With this, any extension can easily introduce one or more new condition choices, which will be picked up automatically when presenting a `ConditionsField` (when used along with the upcoming rbintegrations 4.1 release). Documentation has been written to help extension authors make use of the new hook. It only touches upon the basics of creating new choices, and doesn't touch upon custom operators, but it should be enough to get people going.	70d9cb2aaae7088b39753ce3bcc3265f84fc4e5a

Diff:

Revision 4 (+740 -6)

Show changes

	docs/manual/extending/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/review-request-condition-choices-hook.rst
	reviewboard/extensions/hooks/__init__.py
	reviewboard/extensions/hooks/conditions.py
	reviewboard/extensions/tests/test_review_request_condition_choices_hook.py
	reviewboard/reviews/conditions.py
	reviewboard/reviews/forms.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

docs/manual/extending/extensions/hooks/review-request-condition-choices-hook.rst (Diff revision 4)

The issue has been resolved. Show all issues

For this and the other choice_id below, I think it'd be good to prefix it like we recommend to do. Maybe sample-extension_my-category. That would help if someone for example copies and pastes this code to get started on their condition choice, and forgets to think about adding a prefix.

Change Summary:

Added vendor prefixes to the example condition choices.

Commits:

	Summary	ID
	Add a central registry and hooks for custom condition choices. When we first wrote the support for condition choices for integrations, we intended for the framework to be extensible, allowing extensions to produce new condition choices that users could select from. However, this didn't work in practice, since we lacked any kind of central registry for condition choices that extensions could hook into. This change introduces a new central `review_request_condition_choices` registry, along with a new `ReviewRequestConditionChoicesHook` that wraps it. With this, any extension can easily introduce one or more new condition choices, which will be picked up automatically when presenting a `ConditionsField` (when used along with the upcoming rbintegrations 4.1 release). Documentation has been written to help extension authors make use of the new hook. It only touches upon the basics of creating new choices, and doesn't touch upon custom operators, but it should be enough to get people going.	70d9cb2aaae7088b39753ce3bcc3265f84fc4e5a
	Add a central registry and hooks for custom condition choices. When we first wrote the support for condition choices for integrations, we intended for the framework to be extensible, allowing extensions to produce new condition choices that users could select from. However, this didn't work in practice, since we lacked any kind of central registry for condition choices that extensions could hook into. This change introduces a new central `review_request_condition_choices` registry, along with a new `ReviewRequestConditionChoicesHook` that wraps it. With this, any extension can easily introduce one or more new condition choices, which will be picked up automatically when presenting a `ConditionsField` (when used along with the upcoming rbintegrations 4.1 release). Documentation has been written to help extension authors make use of the new hook. It only touches upon the basics of creating new choices, and doesn't touch upon custom operators, but it should be enough to get people going.	34336df74418e4338c3a8735fa4cd518dec3a4f4

Diff:

Revision 5 (+740 -6)

Show changes

	docs/manual/extending/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/review-request-condition-choices-hook.rst
	reviewboard/extensions/hooks/__init__.py
	reviewboard/extensions/hooks/conditions.py
	reviewboard/extensions/tests/test_review_request_condition_choices_hook.py
	reviewboard/reviews/conditions.py
	reviewboard/reviews/forms.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-7.1.x (11a4f1f)