Avoid to submit a non-public draft
Review Request #6856 — Created Jan. 31, 2015 and submitted
Fix not dealing with submit action with review_request.py
Add validation to the ReviewRequest model to prevent state transitions to SUBMITTED if the review request is not public.
Then get rid of the "Submit" button in a review request if it's not currently public on the UI level.
Changes have been made on following tests because they just creat a non-public empty draft to submit. This should not be allowed.
./reviewboard/manage.py test -- reviewboard.reviews.tests:ReviewRequestCounterTests.test_closing_closed_requests
I publish the review request first before discard it.
./reviewboard/manage.py test -- reviewboard.reviews.tests:ReviewRequestCounterTests.test_reopen_submitted_draft_requests
I use test_closing_requests instead of test_closing_draft_requests because test_closing_requests can create a public draft instead.
./reviewboard/manage.py test -- reviewboard.reviews.tests:ReviewRequestTests.test_public_with_discard_reopen_submitted
I publish the review request after reopen a discarded request before submit it.
./reviewboard/manage.py test -- reviewboard.notifications.tests:WebHookSignalDispatchTests.test_review_request_reopened
I create a published request instead.
./reviewboard/manage.py test -- reviewboard.notifications.tests:WebHookSignalDispatchTests.test_review_request_closed_submitted
I create a published request instead.
| Description | From | Last Updated |
|---|---|---|
|
You should tie this in to the import for PermissioError, like this: from reviewboard.reviews.errors import (PermissionError, PublishError) |
 mike_conley
|
|
|
I think we still want to make it possible to Delete Permanently and Discard a review request that is not … |
mike_conley
|
|
|
I suspect we're going to want to add a new test too to ensure that we cannot transition from PENDING_REVIEW … |
mike_conley
|
|
|
TBH, I don't think it's necessary to test the string representation of the exception. See test_publish_changedesc_none, for example. |
mike_conley
|
|
|
There should be two newlines between root-script level items. |
mike_conley
|
|
|
local variable 'pe' is assigned to but never used |
 reviewbot
|
-
Just a few issues, please a reminder to please read this document on writing good change descriptions / testing done documentation.
-
You should tie this in to the import for PermissioError, like this:
from reviewboard.reviews.errors import (PermissionError, PublishError) -
I think we still want to make it possible to Delete Permanently and Discard a review request that is not public, so we should just wrap the Submitted option here.
CH
- Summary:
-
[WIP] avoid to submit a non-public draftAvoid to submit a non-public draft
- Description:
-
~ avoid to submit a non-public draft
~ Fix not dealing with submit action with review_request.py
+ + Add validation to the ReviewRequest model to prevent state transitions to SUBMITTED if the review request is not public.
+ + + + Then get rid of the "Submit" button in a review request if it's not currently public on the UI level.
- Testing Done:
-
~ following tests failed because they just creat a non-public empty draft to submit. This should not be allowed .
~ ./reviewboard/manage.py test -- reviewboard.reviews.tests:ReviewRequestCounterTests.test_closing_closed_requests ~ Changes have been made on following tests because they just creat a non-public empty draft to submit. This should not be allowed.
~ + ./reviewboard/manage.py test -- reviewboard.reviews.tests:ReviewRequestCounterTests.test_closing_closed_requests
+ + I publish the review request first before discard it.
./reviewboard/manage.py test -- reviewboard.reviews.tests:ReviewRequestCounterTests.test_reopen_submitted_draft_requests
~ ./reviewboard/manage.py test -- reviewboard.reviews.tests:ReviewRequestCounterTests.test_public_with_discard_reopen_submitted
~ I use test_closing_requests instead of test_closing_draft_requests because test_closing_requests can create a public draft instead.
+ + ./reviewboard/manage.py test -- reviewboard.reviews.tests:ReviewRequestTests.test_public_with_discard_reopen_submitted
+ + I publish the review request after reopen a discarded request before submit it.
./reviewboard/manage.py test -- reviewboard.notifications.tests:WebHookSignalDispatchTests.test_review_request_reopened
+ I create a published request instead.
+ ./reviewboard/manage.py test -- reviewboard.notifications.tests:WebHookSignalDispatchTests.test_review_request_closed_submitted
+ + I create a published request instead.
- Commit:
-
96bfc286e2c015421b02c24789f4f9cd795355a1ae4e99dffc295fdc345bfdcb5416d5329a23659a
-
Tool: Pyflakes Processed Files: reviewboard/reviews/tests.py reviewboard/notifications/tests.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/templates/reviews/review_request_actions_secondary.html Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/tests.py reviewboard/notifications/tests.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/templates/reviews/review_request_actions_secondary.html
-
This looks really good, Chenxi. I think you're very close - my only suggestion is to add a new test to exercise the restriction you've put in.
-
I suspect we're going to want to add a new test too to ensure that we cannot transition from PENDING_REVIEW to SUBMITTED if the review request is not public.
CH
- Change Summary:
-
add a new test to ensure that we cannot transition from PENDING_REVIEW to SUBMITTED if the review request is not public
- Commit:
-
ae4e99dffc295fdc345bfdcb5416d5329a23659ad162e80dad67eb7cb124658049b48f0e49f65a15
-
Tool: Pyflakes Processed Files: reviewboard/reviews/tests.py reviewboard/notifications/tests.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/templates/reviews/review_request_actions_secondary.html Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/tests.py reviewboard/notifications/tests.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/templates/reviews/review_request_actions_secondary.html
CH
- Change Summary:
-
remove unnecessary part
- Commit:
-
d162e80dad67eb7cb124658049b48f0e49f65a159627a092b9cbfcf9247f05baa1af002ae8342801
-
Tool: Pyflakes Processed Files: reviewboard/reviews/tests.py reviewboard/notifications/tests.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/templates/reviews/review_request_actions_secondary.html Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/tests.py reviewboard/notifications/tests.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/templates/reviews/review_request_actions_secondary.html -
-
Tool: Pyflakes Processed Files: reviewboard/reviews/tests.py reviewboard/notifications/tests.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/templates/reviews/review_request_actions_secondary.html Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/tests.py reviewboard/notifications/tests.py reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/templates/reviews/review_request_actions_secondary.html
Tool: Pyflakes Processed Files: reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/templates/reviews/review_request_actions_secondary.html Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/models/review_request.py Ignored Files: reviewboard/templates/reviews/review_request_actions_secondary.html