Added automated flag when closing or editing review requests
Review Request #12051 — Created Feb. 10, 2022 and updated
- Added
automated
flag to multiple methods related to hostingsvcs close
hook,ReviewRequestResourceAPI Update()
, andreview_request.close()
to store the value inChangeDescription
model to track automated
closing of review requests
- Added
automated
flag to
ReviewRequestResourceDraftAPI Update(), Create()
,
review_request.publish()
,draft.publish()
to store the value in
ChangeDescription
model to track automated upddating/publishing
of review requests
- Added
automated
flag to closed/published signals and email-related methods to render the
review_request_email.txt
differently whenautomated=True
- Added codes to
change.html
template to render RB logo in the
review request change entry whenautomated=True
- Added unit tests to
test_email_sending.py
- Added unit tests to
test_review_request.py
- Added unit tests to
test_review_request_draft.py
- Added unit tests to
test_entries.py
- Added unit tests to
webapi/tests/test_review_request.py
- Added unit tests to
webapi/tests/test_review_request_draft.py
- Added assertion to
test_github.py
,test_rbgateway.py
,
test_bitbucket.py
All tests passed.
- Attached screenshots to show changes in close email .txt and
frontend changeEntry.
Summary | ID | Author |
---|---|---|
ae08fb83f5c61aee389a3cbdcc9ffc8bf86b1c2d | sng06 | |
379f610ef9fd8101e95f0e3e1b4f6dac08306e22 | sng06 | |
e0fb128a7626a118246c6e5719ba188d810ba775 | sng06 | |
1c3661f9ea9e0ba1fd4364ac3bc273569db33d37 | sng06 | |
672e3f66657b5ab3fc7dc722b5fbdea25a234989 | sng06 | |
730f9b695f6f425e6a51ff68eb9093e0b75dc240 | sng06 | |
b51528501757f139255fecd65b1ef1d5a580e728 | sng06 | |
e4d4befa081b860ae9778c85eab621a013b663f9 | sng06 | |
7f060eec71a71aafc213cf695f5c01a6e58d970a | sng06 | |
fedbc56e20bc730ac7d7c22488b272250d334e1a | sng06 | |
a76d6541b60e60860319bec47a668a1534824524 | sng06 | |
4c41ae5c1a003d287b178960b873f8c2b188e73e | sng06 | |
b9eee8455a0b69fb818ee37643a83cabeb4d5ed6 | sng06 | |
de8085943180d5a6a1c9580f9a3cc27d75f8bcc2 | sng06 | |
8c8a72124aef4a479f15a8da8c413f42f929ba5f | sng06 | |
77d8a9ac58eb1c1fe331080b25d4407baf822a0f | sng06 | |
cfaf73b88ed49e03a7895530f1dc1d05d0ef1ebc | sng06 | |
5ca764de4cc3f8e43301b3f20e61393350922742 | sng06 | |
0a6178d4e8bb5b1211c8ff518bb01a2b730f13d8 | sng06 | |
7bc2c4d53dc7715e2e5f81c46a3942585efa61dd | sng06 | |
7a0dd7e5491153465a7c79f5557b3be8fd40d346 | sng06 | |
7083cb3ff5115b9173faa39e7076da009d0c52af | sng06 | |
c8cb69aa733682714d6c7dc3f5e926523cdeee0a | sng06 | |
fc9641493757893dc659591ed86786263fbe00bc | sng06 | |
68e47456666ed949ed78317f8231957a67f0afd7 | sng06 | |
97e2167d50167344f5dfc41b77651293bbc2edb2 | sng06 | |
f95ea9155018cce8da51c570cdf037d1c44c3317 | sng06 | |
c34ba1ccffee84b560a925ac2bc90205455ac38b | sng06 | |
e8f58bbc3333ba3434d31374586344eebc297563 | sng06 |
Description | From | Last Updated |
---|---|---|
Please wrap your description and testing done fields to 70 columns. |
david | |
While we're here, can we rewrite this docstring to follow our modern standards? We probably also want to add a … |
david | |
Instead of assertEqual(..., True), use assertTrue(...). |
david | |
Instead of assertEqual(..., True), use assertTrue(...). |
david | |
Instead of assertEqual(..., True), use assertTrue(...). |
david | |
Please add ", optional" to the type. |
david | |
These can be combined: if not (user or automate): |
david | |
E128 continuation line under-indented for visual indent |
reviewbot | |
Should be marked as optional (and probably the other things in here should be fixed for that as well) |
david | |
As Christian mentioned in our meeting, these should be updated to use kgb's spy assertion methods. |
david | |
E501 line too long (80 > 79 characters) |
reviewbot | |
This should be in the imperative mood ("Close" instead of "Closes") |
david | |
Instead of defining a variable for this, let's just pass automated=True to close_review_request. |
david | |
This is getting so long, can we reformat it to list one parameter per line? |
david | |
This can be simplified: extra_context.update({ 'automated': changedesc.extra_data.get('automated', False), }) |
david | |
Test docstrings should always start with "Testing ..." |
david | |
"Testing ..." |
david | |
There's an extra space at the end of this docstring. |
david | |
There's an extra space at the end of this docstring. |
david | |
Please put the """ on the next line. |
david | |
This needs a trailing comma after it. |
david | |
Please put the """ on the next line. |
david | |
Please put the """ on the next line. |
david | |
This needs a trailing comma after it. |
david | |
Please put the """ on the next line. |
david | |
Please put the """ on the next line. |
david | |
This needs a trailing comma after it. |
david |
Summary: |
|
||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
||||||||||||||||||||||
Commits: |
|
||||||||||||||||||||||
Diff: |
Revision 2 (+140 -40) |
Checks run (2 succeeded)
Commits: |
|
|||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+534 -70) |
Checks run (1 failed, 1 succeeded)
flake8
-
reviewboard/notifications/tests/test_email_sending.py (Diff revision 3)
-
Looking good!
-
Regarding the question in your testing done field, editing the existing hook tests is fine to add that one new assertion.
-
-
reviewboard/hostingsvcs/hook_utils.py (Diff revision 3) While we're here, can we rewrite this docstring to follow our modern standards?
We probably also want to add a default value for the
automate
field in case any extensions are using this API. -
reviewboard/hostingsvcs/tests/test_bitbucket.py (Diff revision 3) Instead of
assertEqual(..., True)
, useassertTrue(...)
. -
reviewboard/hostingsvcs/tests/test_github.py (Diff revision 3) Instead of
assertEqual(..., True)
, useassertTrue(...)
. -
reviewboard/hostingsvcs/tests/test_rbgateway.py (Diff revision 3) Instead of
assertEqual(..., True)
, useassertTrue(...)
. -
-
reviewboard/notifications/email/message.py (Diff revision 3) These can be combined:
if not (user or automate):
-
reviewboard/reviews/models/review_request.py (Diff revision 3) Should be marked as optional (and probably the other things in here should be fixed for that as well)
-
reviewboard/reviews/tests/test_review_request.py (Diff revision 3) As Christian mentioned in our meeting, these should be updated to use kgb's spy assertion methods.
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 4 (+571 -133) |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Added Files: |
Checks run (2 succeeded)
Testing Done: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 5 (+839 -145) |
Checks run (2 succeeded)
Change Summary:
Added automate field and codes to draft publish operations
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+929 -169) |
Checks run (1 failed, 1 succeeded)
flake8
-
reviewboard/reviews/models/review_request.py (Diff revision 6) E501 line too long (80 > 79 characters)
Change Summary:
Added unit tests to test draft operations-related methods
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 7 (+1717 -215) |
Checks run (2 succeeded)
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 8 (+1887 -373) |
Checks run (2 succeeded)
Summary: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 9 (+1907 -391) |
Checks run (2 succeeded)
-
-
reviewboard/hostingsvcs/hook_utils.py (Diff revision 9) This should be in the imperative mood ("Close" instead of "Closes")
-
reviewboard/hostingsvcs/hook_utils.py (Diff revision 9) Instead of defining a variable for this, let's just pass
automated=True
toclose_review_request
. -
reviewboard/notifications/email/message.py (Diff revision 9) This is getting so long, can we reformat it to list one parameter per line?
-
reviewboard/notifications/email/message.py (Diff revision 9) This can be simplified:
extra_context.update({ 'automated': changedesc.extra_data.get('automated', False), })
-
reviewboard/notifications/tests/test_email_sending.py (Diff revision 9) Test docstrings should always start with "Testing ..."
-
-
reviewboard/reviews/tests/test_entries.py (Diff revision 9) There's an extra space at the end of this docstring.
-
reviewboard/reviews/tests/test_entries.py (Diff revision 9) There's an extra space at the end of this docstring.
-
reviewboard/webapi/tests/test_review_request.py (Diff revision 9) Please put the """ on the next line.
-
reviewboard/webapi/tests/test_review_request.py (Diff revision 9) This needs a trailing comma after it.
-
reviewboard/webapi/tests/test_review_request.py (Diff revision 9) Please put the """ on the next line.
-
reviewboard/webapi/tests/test_review_request_draft.py (Diff revision 9) Please put the """ on the next line.
-
reviewboard/webapi/tests/test_review_request_draft.py (Diff revision 9) This needs a trailing comma after it.
-
reviewboard/webapi/tests/test_review_request_draft.py (Diff revision 9) Please put the """ on the next line.
-
reviewboard/webapi/tests/test_review_request_draft.py (Diff revision 9) Please put the """ on the next line.
-
reviewboard/webapi/tests/test_review_request_draft.py (Diff revision 9) This needs a trailing comma after it.
Change Summary:
Addressed code review feedback
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 10 (+1949 -433) |