Summary

Disallow users from saying "Ship It!" to their own review requests.

Review Request #14198 — Created Oct. 11, 2024 and updated Nov. 8, 2024, 6:41 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-7.1.x

Bugs

4984

Depends On

Reviewers

Groups

reviewboard

People

Description

This change implements that. The visibility of the "Ship It" action and
checkbox in the review dialog are now conditional on this flag, and
reviews will refuse to publish (either via the UI or API).

Because a big change like this may be controversial, I've added a toggle
for it in the admin settings, under a new page titled "Review Workflow".

While adding tests for the action, I discovered that several of our
existing action tests were accidentally still testing the legacy
versions of the actions (imported via the old
reviewboard.reviews.default_actions module). I've fixed it up so we
have separate tests for the legacy and modern versions of actions that
exist in the unified banner.

Testing Done


Viewed a review request as both the owner and another user, and saw

  that the visibility of the action and review dialog checkbox was

  correct.
Toggled the setting in the admin and checked again, seeing that I

  could restore previous behavior.
Ran unit tests.

Commits

Summary	ID
Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". While adding tests for the action, I discovered that several of our existing action tests were accidentally still testing the legacy versions of the actions (imported via the old `reviewboard.reviews.default_actions` module). I've fixed it up so we have separate tests for the legacy and modern versions of actions that exist in the unified banner. Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	a8920586607ce182e28345cad8b7c0069daeb460

Summary

Disallow users from saying "Ship It!" to their own review requests.

We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". While adding tests for the action, I discovered that several of our existing action tests were accidentally still testing the legacy versions of the actions (imported via the old `reviewboard.reviews.default_actions` module). I've fixed it up so we have separate tests for the legacy and modern versions of actions that exist in the unified banner. Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.

a8920586607ce182e28345cad8b7c0069daeb460

Issues

Description	From	Last Updated
continuation line missing indentation or outdented Column: 17 Error code: E122	reviewbot	Oct. 11, 2024, 7:50 a.m.
Maybe better as JSONDict?	maubin	Oct. 11, 2024, 3:26 p.m.
This can be on one line.	maubin	Oct. 11, 2024, 3:27 p.m.
This can be on one line.	maubin	Oct. 11, 2024, 3:27 p.m.
Missing Version Added.	chipx86	Oct. 24, 2024, 12:57 p.m.
Missing Version Added.	chipx86	Oct. 24, 2024, 12:57 p.m.
Can we put the ) on the next line, so it's more akin to how we do {} and []? …	chipx86	Oct. 24, 2024, 12:57 p.m.
Version Added.	chipx86	Oct. 24, 2024, 12:58 p.m.
Should we have a trailing period?	chipx86	Oct. 24, 2024, 12:58 p.m.
Missing Version Added.	chipx86	Oct. 24, 2024, 12:58 p.m.
Can we add tests checking these conditions for this action?	chipx86	Nov. 1, 2024, 8:44 a.m.
These can start on the return ( line, like the others.	chipx86	Nov. 8, 2024, 6:26 p.m.
Single quote string here.	chipx86	Nov. 1, 2024, 8:35 a.m.
Single quote string here and in the test below.	chipx86	Nov. 1, 2024, 8:35 a.m.
Can we check against the specific error message as well? We should also compare the full rsp instead of just …	chipx86	Nov. 1, 2024, 8:39 a.m.
line too long (84 > 79 characters) Column: 80 Error code: E501	reviewbot	Nov. 1, 2024, 10:48 a.m.
continuation line unaligned for hanging indent Column: 33 Error code: E131	reviewbot	Nov. 1, 2024, 11:01 a.m.
We seem to be setting one request and then creating a whole separate one for the context. Is this right? …	chipx86	Nov. 8, 2024, 6:28 p.m.
We're fetching the user twice. Let's do it only once.	chipx86	Nov. 8, 2024, 6:33 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/accounts/tests/test_template_tags.py (Diff revision 1)
The issue has been dropped. Show all issues
```
continuation line missing indentation or outdented

Column: 17
Error code: E122
```

Commits:

	Summary	ID
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	e48bbd7267d500f1f887fba94c7175206262bccf
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	ce192f063a2a7fdad383122f073119c038e04826

Diff:

Revision 2 (+474 -60)

Show changes

	reviewboard/accounts/templatetags/accounts.py
	reviewboard/accounts/tests/test_template_tags.py
	reviewboard/actions/base.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/admin/siteconfig.py
	reviewboard/admin/urls.py
	reviewboard/admin/forms/review_settings.py
	reviewboard/reviews/actions.py
	5 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

reviewboard/actions/base.py (Diff revision 2)

The issue has been dropped. Show all issues

Maybe better as JSONDict?

david Oct. 11, 2024, 3:28 p.m.

This is django template context, not JSON.

We probably should create a definition for that in typelets.

reviewboard/webapi/tests/test_review.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This can be on one line.
```
reviewboard/webapi/tests/test_review.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This can be on one line.
```

Commits:

	Summary	ID
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	ce192f063a2a7fdad383122f073119c038e04826
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	a12cfc0820aaf92e1d6657c684756826e68fc7e3

Diff:

Revision 3 (+468 -58)

Show changes

	reviewboard/accounts/templatetags/accounts.py
	reviewboard/accounts/tests/test_template_tags.py
	reviewboard/actions/base.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/admin/siteconfig.py
	reviewboard/admin/urls.py
	reviewboard/admin/forms/review_settings.py
	reviewboard/reviews/actions.py
	5 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/admin/forms/review_settings.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Missing Version Added.
```
reviewboard/admin/forms/review_settings.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Missing Version Added.
```

reviewboard/admin/forms/review_settings.py (Diff revision 3)

The issue has been resolved. Show all issues

Can we put the ) on the next line, so it's more akin to how we do {} and []? Makes it easier to edit the text without shifting that around.

reviewboard/reviews/actions.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Version Added.
```
reviewboard/reviews/models/review.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Should we have a trailing period?
```
reviewboard/static/rb/js/common/models/userSessionModel.ts (Diff revision 3)
The issue has been resolved. Show all issues
```
Missing Version Added.
```

Commits:

	Summary	ID
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	a12cfc0820aaf92e1d6657c684756826e68fc7e3
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	cb8f7f5c526e3652773685f6abae8f84bc4cece1

Branch:

release-7.x

release-7.1.x

Diff:

Revision 4 (+468 -58)

Show changes

	reviewboard/accounts/templatetags/accounts.py
	reviewboard/accounts/tests/test_template_tags.py
	reviewboard/actions/base.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/admin/siteconfig.py
	reviewboard/admin/urls.py
	reviewboard/admin/forms/review_settings.py
	reviewboard/reviews/actions.py
	5 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commits:

	Summary	ID
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	cb8f7f5c526e3652773685f6abae8f84bc4cece1
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	38761bb4829261a53be2d52d8584874b355e0db7

Diff:

Revision 5 (+498 -58)

Show changes

	reviewboard/accounts/templatetags/accounts.py
	reviewboard/accounts/tests/test_template_tags.py
	reviewboard/actions/base.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/admin/siteconfig.py
	reviewboard/admin/urls.py
	reviewboard/admin/forms/review_settings.py
	reviewboard/reviews/actions.py
	5 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/reviews/actions.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Can we add tests checking these conditions for this action?
```
reviewboard/webapi/tests/test_review.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Single quote string here.
```
reviewboard/webapi/tests/test_review.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Single quote string here and in the test below.
```

reviewboard/webapi/tests/test_review.py (Diff revision 5)

The issue has been resolved. Show all issues

Can we check against the specific error message as well?

We should also compare the full rsp instead of just parts going forward. I found some previously-missed problems before when transitioning code to that approach.

Description:

		We've always taken a pretty hands-off approach, preferring to encourage
		conversation over policy. That said, one of the most requested bits of
		policy is to make it so users can't mark their own review requests as
		"Ship It". This is not only a reasonable policy ask (especially if
		people are using approval workflows that can block pushes), but should
		probably be the default.

		This change implements that. The visibility of the "Ship It" action and
		checkbox in the review dialog are now conditional on this flag, and
		reviews will refuse to publish (either via the UI or API).

		Because a big change like this may be controversial, I've added a toggle
		for it in the admin settings, under a new page titled "Review Workflow".
	+
	+	While adding tests for the action, I discovered that several of our
	+	existing action tests were accidentally still testing the legacy
	+	versions of the actions (imported via the old
	+	`reviewboard.reviews.default_actions` module). I've fixed it up so we
	+	have separate tests for the legacy and modern versions of actions that
	+	exist in the unified banner.

Commits:

	Summary	ID
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	38761bb4829261a53be2d52d8584874b355e0db7
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". While adding tests for the action, I discovered that several of our existing action tests were accidentally still testing the legacy versions of the actions (imported via the old `reviewboard.reviews.default_actions` module). I've fixed it up so we have separate tests for the legacy and modern versions of actions that exist in the unified banner. Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	c5f6bf5545f12d7b84ca4d653c5ce899c80d07bf

Diff:

Revision 6 (+1130 -150)

Show changes

	reviewboard/accounts/templatetags/accounts.py
	reviewboard/accounts/tests/test_template_tags.py
	reviewboard/actions/base.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/admin/siteconfig.py
	reviewboard/admin/urls.py
	reviewboard/admin/forms/review_settings.py
	reviewboard/reviews/actions.py
	6 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/tests/test_actions.py (Diff revision 6)
The issue has been resolved. Show all issues
```
line too long (84 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/webapi/tests/test_review.py (Diff revision 6)
The issue has been resolved. Show all issues
```
continuation line unaligned for hanging indent

Column: 33
Error code: E131
```

Commits:

	Summary	ID
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". While adding tests for the action, I discovered that several of our existing action tests were accidentally still testing the legacy versions of the actions (imported via the old `reviewboard.reviews.default_actions` module). I've fixed it up so we have separate tests for the legacy and modern versions of actions that exist in the unified banner. Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	c5f6bf5545f12d7b84ca4d653c5ce899c80d07bf
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". While adding tests for the action, I discovered that several of our existing action tests were accidentally still testing the legacy versions of the actions (imported via the old `reviewboard.reviews.default_actions` module). I've fixed it up so we have separate tests for the legacy and modern versions of actions that exist in the unified banner. Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	1a3159ccb35b752b865b81b632e769d42beda40c

Diff:

Revision 7 (+1132 -150)

Show changes

	reviewboard/accounts/templatetags/accounts.py
	reviewboard/accounts/tests/test_template_tags.py
	reviewboard/actions/base.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/admin/siteconfig.py
	reviewboard/admin/urls.py
	reviewboard/admin/forms/review_settings.py
	reviewboard/reviews/actions.py
	6 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/reviews/actions.py (Diff revisions 5 - 7)
The issue has been resolved. Show all issues
```
These can start on the return ( line, like the others.
```

reviewboard/reviews/tests/test_actions.py (Diff revision 7)

The issue has been resolved. Show all issues

We seem to be setting one request and then creating a whole separate one for the context. Is this right? If so, we should document why we're doing this.
If it's not right, and we want to create_http_request, that will take care of creating the ResolverMatch automatically. In fact, if we want these two separate requests, we could just let it do that and then assign the result into self.request to avoid duplicating the weird ResolverMatch magic.

reviewboard/reviews/tests/test_actions.py (Diff revision 7)
The issue has been resolved. Show all issues
```
We're fetching the user twice. Let's do it only once.
```

Commits:

	Summary	ID
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". While adding tests for the action, I discovered that several of our existing action tests were accidentally still testing the legacy versions of the actions (imported via the old `reviewboard.reviews.default_actions` module). I've fixed it up so we have separate tests for the legacy and modern versions of actions that exist in the unified banner. Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	1a3159ccb35b752b865b81b632e769d42beda40c
	Disallow users from saying "Ship It!" to their own review requests. We've always taken a pretty hands-off approach, preferring to encourage conversation over policy. That said, one of the most requested bits of policy is to make it so users can't mark their own review requests as "Ship It". This is not only a reasonable policy ask (especially if people are using approval workflows that can block pushes), but should probably be the default. This change implements that. The visibility of the "Ship It" action and checkbox in the review dialog are now conditional on this flag, and reviews will refuse to publish (either via the UI or API). Because a big change like this may be controversial, I've added a toggle for it in the admin settings, under a new page titled "Review Workflow". While adding tests for the action, I discovered that several of our existing action tests were accidentally still testing the legacy versions of the actions (imported via the old `reviewboard.reviews.default_actions` module). I've fixed it up so we have separate tests for the legacy and modern versions of actions that exist in the unified banner. Testing Done: - Viewed a review request as both the owner and another user, and saw that the visibility of the action and review dialog checkbox was correct. - Toggled the setting in the admin and checked again, seeing that I could restore previous behavior. - Ran unit tests. Fixes bug 4984.	a8920586607ce182e28345cad8b7c0069daeb460

Diff:

Revision 8 (+1132 -166)

Show changes

	reviewboard/accounts/templatetags/accounts.py
	reviewboard/accounts/tests/test_template_tags.py
	reviewboard/actions/base.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/admin/siteconfig.py
	reviewboard/admin/urls.py
	reviewboard/admin/forms/review_settings.py
	reviewboard/reviews/actions.py
	6 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.