Summary

Allow review publishing to submitter only

Review Request #7537 — Created July 23, 2015 and submitted July 29, 2015, 6:50 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-2.6.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Reviews can now be published to the review request submitter only. When
published this way, only the submitter and reviewer will receive
e-mails about the review (if they both have e-mail acitivity enabled).
All other users in the target review groups or reviewers will not
receieve e-mails about the review. This is particularly useful when the
review consists of pointing out a small typo, etc.

As such, the review_published signal now expect another argument,
to_submitter_only, which indicates if the review is only for the
review request submitter.

Unit tests have been added to test the front end and backend behaviour.

The new RB.SplitButtonView has been added to facilitate the UI
changes for this patch. It consists of a primary button and a drop down
control which, when hovered, will display a list of alternative
actions.

Testing Done

Ran unit test.
Ran JS tests.

Published a review to a review request that was assigned to more than
one reviewer. Only the review request submitter and review publisher
received e-mails.

Files

Issues

Description	From	Last Updated
Whether you have one or two blank lines between top-level sections is inconsistent in this file.	david	July 24, 2015, 8:21 p.m.
You can remove the $() from this (so you're just calling append with a string, rather than first constructing a …	david	July 24, 2015, 8:22 p.m.
Space before the />	david	July 24, 2015, 8:23 p.m.
Can you do this with the events object?	david	July 24, 2015, 8:23 p.m.
No space between function and ()	david	July 27, 2015, 3:40 p.m.
Probably slightly more efficient to do: this._$dropDownBtn .removeClass([this._dropDownShownClass, 'hover']);	david	July 29, 2015, 6:48 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/notifications/tests.py
    reviewboard/webapi/resources/base_review.py
    reviewboard/reviews/models/review.py
    reviewboard/staticbundles.py
    reviewboard/notifications/email.py
    reviewboard/reviews/signals.py

Ignored Files:
    reviewboard/static/rb/js/views/draftReviewBannerView.js
    reviewboard/static/rb/js/views/tests/draftReviewBannerViewTests.js
    reviewboard/static/rb/js/ui/views/splitButtonView.js
    reviewboard/static/rb/js/resources/models/draftReviewModel.js
    reviewboard/static/rb/js/views/reviewDialogView.js
    reviewboard/templates/reviews/reviewable_base.html
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/css/ui/buttons.less



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/notifications/tests.py
    reviewboard/webapi/resources/base_review.py
    reviewboard/reviews/models/review.py
    reviewboard/staticbundles.py
    reviewboard/notifications/email.py
    reviewboard/reviews/signals.py

Ignored Files:
    reviewboard/static/rb/js/views/draftReviewBannerView.js
    reviewboard/static/rb/js/views/tests/draftReviewBannerViewTests.js
    reviewboard/static/rb/js/ui/views/splitButtonView.js
    reviewboard/static/rb/js/resources/models/draftReviewModel.js
    reviewboard/static/rb/js/views/reviewDialogView.js
    reviewboard/templates/reviews/reviewable_base.html
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/css/ui/buttons.less

reviewboard/static/rb/css/ui/buttons.less (Diff revision 1)

Show all issues

Whether you have one or two blank lines between top-level sections is inconsistent in this file.

reviewboard/static/rb/js/ui/views/splitButtonView.js (Diff revision 1)

Show all issues

You can remove the $() from this (so you're just calling append with a string, rather than first constructing a jquery).

reviewboard/static/rb/js/ui/views/splitButtonView.js (Diff revision 1)
Show all issues
```
Space before the />
```

reviewboard/static/rb/js/ui/views/splitButtonView.js (Diff revision 1)

Show all issues

Can you do this with the events object?

brennie July 24, 2015, 8:13 p.m.

The _$alternatives element is not a child of SplitButtonView.$el so that won't work.

reviewboard/static/rb/js/ui/views/splitButtonView.js (Diff revision 1)
Show all issues
```
No space between function and ()
```

Description:

		Reviews can now be published to the review request submitter only. When
		published this way, only the submitter and reviewer will receive
		e-mails about the review (if they both have e-mail acitivity enabled).
		All other users in the target review groups or reviewers will not
		receieve e-mails about the review. This is particularly useful when the
		review consists of pointing out a small typo, etc.

	+	As such, the `review_published` signal now expect another argument,
	+	`to_submitter_only`, which indicates if the review is only for the
	+	review request submitter.
	+
		Unit tests have been added to test the front end and backend behaviour.

		The new `RB.SplitButtonView` has been added to facilitate the UI
		changes for this patch. It consists of a primary button and a drop down
		control which, when hovered, will display a list of alternative
		actions.

Diff:

Revision 2 (+472 -25)

Show changes

	reviewboard/staticbundles.py
	reviewboard/notifications/email.py
	reviewboard/notifications/tests.py
	reviewboard/reviews/signals.py
	reviewboard/reviews/models/review.py
	reviewboard/static/rb/css/defs.less
	reviewboard/static/rb/css/ui/buttons.less
	reviewboard/static/rb/js/pages/views/reviewablePageView.js
	8 more

Added Files:

Tool: Pyflakes
Processed Files:
    reviewboard/notifications/tests.py
    reviewboard/webapi/resources/base_review.py
    reviewboard/reviews/models/review.py
    reviewboard/staticbundles.py
    reviewboard/notifications/email.py
    reviewboard/reviews/signals.py

Ignored Files:
    reviewboard/static/rb/js/views/draftReviewBannerView.js
    reviewboard/static/rb/js/views/tests/draftReviewBannerViewTests.js
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/ui/views/splitButtonView.js
    reviewboard/static/rb/js/resources/models/draftReviewModel.js
    reviewboard/static/rb/js/views/reviewDialogView.js
    reviewboard/templates/reviews/reviewable_base.html
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/css/ui/buttons.less
    reviewboard/static/rb/js/pages/views/reviewablePageView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/notifications/tests.py
    reviewboard/webapi/resources/base_review.py
    reviewboard/reviews/models/review.py
    reviewboard/staticbundles.py
    reviewboard/notifications/email.py
    reviewboard/reviews/signals.py

Ignored Files:
    reviewboard/static/rb/js/views/draftReviewBannerView.js
    reviewboard/static/rb/js/views/tests/draftReviewBannerViewTests.js
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/ui/views/splitButtonView.js
    reviewboard/static/rb/js/resources/models/draftReviewModel.js
    reviewboard/static/rb/js/views/reviewDialogView.js
    reviewboard/templates/reviews/reviewable_base.html
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/css/ui/buttons.less
    reviewboard/static/rb/js/pages/views/reviewablePageView.js

Diff:

Revision 3 (+464 -25)

Show changes

	reviewboard/staticbundles.py
	reviewboard/notifications/email.py
	reviewboard/notifications/tests.py
	reviewboard/reviews/signals.py
	reviewboard/reviews/models/review.py
	reviewboard/static/rb/css/defs.less
	reviewboard/static/rb/css/ui/buttons.less
	reviewboard/static/rb/js/pages/views/reviewablePageView.js
	8 more

Tool: Pyflakes
Processed Files:
    reviewboard/notifications/tests.py
    reviewboard/webapi/resources/base_review.py
    reviewboard/reviews/models/review.py
    reviewboard/staticbundles.py
    reviewboard/notifications/email.py
    reviewboard/reviews/signals.py

Ignored Files:
    reviewboard/static/rb/js/views/draftReviewBannerView.js
    reviewboard/static/rb/js/views/tests/draftReviewBannerViewTests.js
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/ui/views/splitButtonView.js
    reviewboard/static/rb/js/resources/models/draftReviewModel.js
    reviewboard/static/rb/js/views/reviewDialogView.js
    reviewboard/templates/reviews/reviewable_base.html
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/css/ui/buttons.less
    reviewboard/static/rb/js/pages/views/reviewablePageView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/notifications/tests.py
    reviewboard/webapi/resources/base_review.py
    reviewboard/reviews/models/review.py
    reviewboard/staticbundles.py
    reviewboard/notifications/email.py
    reviewboard/reviews/signals.py

Ignored Files:
    reviewboard/static/rb/js/views/draftReviewBannerView.js
    reviewboard/static/rb/js/views/tests/draftReviewBannerViewTests.js
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/ui/views/splitButtonView.js
    reviewboard/static/rb/js/resources/models/draftReviewModel.js
    reviewboard/static/rb/js/views/reviewDialogView.js
    reviewboard/templates/reviews/reviewable_base.html
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/css/ui/buttons.less
    reviewboard/static/rb/js/pages/views/reviewablePageView.js

Ship it!

reviewboard/static/rb/js/ui/views/splitButtonView.js (Diff revision 3)

Show all issues

Probably slightly more efficient to do:

this._$dropDownBtn
    .removeClass([this._dropDownShownClass, 'hover']);

brennie July 29, 2015, 6:48 p.m.

removeClass only takes a string of space-separated classes.

Status: Closed (submitted)

Change Summary:

Pushed to release-2.6.x (f3f65d0)