Added automated flag when closing or editing review requests

Review Request #12051 — Created Feb. 10, 2022 and updated

Information

Review Board
master

Reviewers

  • Added automated flag to multiple methods related to hostingsvcs close
    hook, ReviewRequestResourceAPI Update(), and review_request.close()
    to store the value in ChangeDescription 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 when automated=True

  • Added codes to change.html template to render RB logo in the
    review request change entry when automated=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 Author
Added isAutomatedClose flag through close hook flow to ChangeDescription's extra_data field and added unit tests
sng06
Deleted print statements in unit tests
sng06
Changed variable name to automate
sng06
Added automate field to ReviewRequestResource API update method and passed it to close method
sng06
Added automate fields and made changes to  email-related methods based on automate flag value
sng06
Made changes to close email txt template based on automate flag value
sng06
Added unit tests to review_request and test_email_sending test suites
sng06
Added default value to automate argument in close_review_request method
sng06
Changed assertEqual to assertTrue for test_post_commit_hook unit test
sng06
Added optional for automate field in docstrings
sng06
Fixed review_request_close unit test spy assertion
sng06
Added codes to render Review Board logo in review request change box when it is closed automatically
sng06
Added or edited docstrings and fixed formatting issues
sng06
Added class tag to RB logo avatar rendering in change.html template
sng06
Added unit tests to test changeEntry
sng06
Added automate field to webapi-request-fields decorator for both create and update methods
sng06
added unit tests to test PUT api for submitted and discarded actions
sng06
Deleted print statements
sng06
Edited automate field's description
sng06
Added automate field to draft resource PUT endpoint, review request publish-related email methods, draft and review request models
sng06
Fixed formatting and added docstring
sng06
Added unit tests to test draft-related methods
sng06
Changed review_request close unit test name
sng06
Changed automate to automated and edited unit tests docstrings
sng06
Fixed review request draft unit tests docstrings
sng06
Changed the conditional block to save automated value in extra context using extra_data field
sng06
Fixed formatting issues in review_request and review_request_draft unit tests
sng06
Fixed missing words and periods in docstrings
sng06
Addressed code review feedback received
sng06

Description From Last Updated

Please wrap your description and testing done fields to 70 columns.

daviddavid

While we're here, can we rewrite this docstring to follow our modern standards? We probably also want to add a …

daviddavid

Instead of assertEqual(..., True), use assertTrue(...).

daviddavid

Instead of assertEqual(..., True), use assertTrue(...).

daviddavid

Instead of assertEqual(..., True), use assertTrue(...).

daviddavid

Please add ", optional" to the type.

daviddavid

These can be combined: if not (user or automate):

daviddavid

E128 continuation line under-indented for visual indent

reviewbotreviewbot

Should be marked as optional (and probably the other things in here should be fixed for that as well)

daviddavid

As Christian mentioned in our meeting, these should be updated to use kgb's spy assertion methods.

daviddavid

E501 line too long (80 > 79 characters)

reviewbotreviewbot

This should be in the imperative mood ("Close" instead of "Closes")

daviddavid

Instead of defining a variable for this, let's just pass automated=True to close_review_request.

daviddavid

This is getting so long, can we reformat it to list one parameter per line?

daviddavid

This can be simplified: extra_context.update({ 'automated': changedesc.extra_data.get('automated', False), })

daviddavid

Test docstrings should always start with "Testing ..."

daviddavid

"Testing ..."

daviddavid

There's an extra space at the end of this docstring.

daviddavid

There's an extra space at the end of this docstring.

daviddavid

Please put the """ on the next line.

daviddavid

This needs a trailing comma after it.

daviddavid

Please put the """ on the next line.

daviddavid

Please put the """ on the next line.

daviddavid

This needs a trailing comma after it.

daviddavid

Please put the """ on the next line.

daviddavid

Please put the """ on the next line.

daviddavid

This needs a trailing comma after it.

daviddavid
sheenaNg
sheenaNg
Review request changed

Commits:

Summary Author
-
Added isAutomatedClose flag through close hook flow to ChangeDescription's extra_data field and added unit tests
sng06
-
Deleted print statements in unit tests
sng06
-
Changed variable name to automate
sng06
-
Added automate field to ReviewRequestResource API update method and passed it to close method
sng06
+
Added isAutomatedClose flag through close hook flow to ChangeDescription's extra_data field and added unit tests
sng06
+
Deleted print statements in unit tests
sng06
+
Changed variable name to automate
sng06
+
Added automate field to ReviewRequestResource API update method and passed it to close method
sng06
+
Added automate fields and made changes to  email-related methods based on automate flag value
sng06
+
Made changes to close email txt template based on automate flag value
sng06
+
Added unit tests to review_request and test_email_sending test suites
sng06

Diff:

Revision 3 (+534 -70)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
  1. Looking good!

  2. Regarding the question in your testing done field, editing the existing hook tests is fine to add that one new assertion.

  3. Please wrap your description and testing done fields to 70 columns.

  4. 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.

  5. Instead of assertEqual(..., True), use assertTrue(...).

  6. Instead of assertEqual(..., True), use assertTrue(...).

  7. Instead of assertEqual(..., True), use assertTrue(...).

  8. Please add ", optional" to the type.

  9. reviewboard/notifications/email/message.py (Diff revision 3)
     
     
     

    These can be combined:

    if not (user or automate):

  10. Should be marked as optional (and probably the other things in here should be fixed for that as well)

  11. As Christian mentioned in our meeting, these should be updated to use kgb's spy assertion methods.

  12. 
      
sheenaNg
sheenaNg
sheenaNg
Review request changed

Change Summary:

Added automate field and codes to draft publish operations

Commits:

Summary Author
-
Added isAutomatedClose flag through close hook flow to ChangeDescription's extra_data field and added unit tests
sng06
-
Deleted print statements in unit tests
sng06
-
Changed variable name to automate
sng06
-
Added automate field to ReviewRequestResource API update method and passed it to close method
sng06
-
Added automate fields and made changes to  email-related methods based on automate flag value
sng06
-
Made changes to close email txt template based on automate flag value
sng06
-
Added unit tests to review_request and test_email_sending test suites
sng06
-
Added default value to automate argument in close_review_request method
sng06
-
Changed assertEqual to assertTrue for test_post_commit_hook unit test
sng06
-
Added optional for automate field in docstrings
sng06
-
Fixed review_request_close unit test spy assertion
sng06
-
Added codes to render Review Board logo in review request change box when it is closed automatically
sng06
-
Added or edited docstrings and fixed formatting issues
sng06
-
Added class tag to RB logo avatar rendering in change.html template
sng06
-
Added unit tests to test changeEntry
sng06
-
Added automate field to webapi-request-fields decorator for both create and update methods
sng06
-
added unit tests to test PUT api for submitted and discarded actions
sng06
-
Deleted print statements
sng06
+
Added isAutomatedClose flag through close hook flow to ChangeDescription's extra_data field and added unit tests
sng06
+
Deleted print statements in unit tests
sng06
+
Changed variable name to automate
sng06
+
Added automate field to ReviewRequestResource API update method and passed it to close method
sng06
+
Added automate fields and made changes to  email-related methods based on automate flag value
sng06
+
Made changes to close email txt template based on automate flag value
sng06
+
Added unit tests to review_request and test_email_sending test suites
sng06
+
Added default value to automate argument in close_review_request method
sng06
+
Changed assertEqual to assertTrue for test_post_commit_hook unit test
sng06
+
Added optional for automate field in docstrings
sng06
+
Fixed review_request_close unit test spy assertion
sng06
+
Added codes to render Review Board logo in review request change box when it is closed automatically
sng06
+
Added or edited docstrings and fixed formatting issues
sng06
+
Added class tag to RB logo avatar rendering in change.html template
sng06
+
Added unit tests to test changeEntry
sng06
+
Added automate field to webapi-request-fields decorator for both create and update methods
sng06
+
added unit tests to test PUT api for submitted and discarded actions
sng06
+
Deleted print statements
sng06
+
Edited automate field's description
sng06
+
Added automate field to draft resource PUT endpoint, review request publish-related email methods, draft and review request models
sng06

Diff:

Revision 6 (+929 -169)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

sheenaNg
sheenaNg
sheenaNg
david
  1. 
      
  2. reviewboard/hostingsvcs/hook_utils.py (Diff revision 9)
     
     

    This should be in the imperative mood ("Close" instead of "Closes")

  3. reviewboard/hostingsvcs/hook_utils.py (Diff revision 9)
     
     
     
     

    Instead of defining a variable for this, let's just pass automated=True to close_review_request.

  4. reviewboard/notifications/email/message.py (Diff revision 9)
     
     
     
     
     

    This is getting so long, can we reformat it to list one parameter per line?

  5. reviewboard/notifications/email/message.py (Diff revision 9)
     
     
     
     
     
     
     
     
     

    This can be simplified:

    extra_context.update({
        'automated': changedesc.extra_data.get('automated', False),
    })
    
  6. Test docstrings should always start with "Testing ..."

  7. There's an extra space at the end of this docstring.

  8. There's an extra space at the end of this docstring.

  9. Please put the """ on the next line.

  10. This needs a trailing comma after it.

  11. Please put the """ on the next line.

  12. Please put the """ on the next line.

  13. This needs a trailing comma after it.

  14. Please put the """ on the next line.

  15. Please put the """ on the next line.

  16. This needs a trailing comma after it.

  17. 
      
sheenaNg
Review request changed

Change Summary:

Addressed code review feedback

Commits:

Summary Author
-
Added isAutomatedClose flag through close hook flow to ChangeDescription's extra_data field and added unit tests
sng06
-
Deleted print statements in unit tests
sng06
-
Changed variable name to automate
sng06
-
Added automate field to ReviewRequestResource API update method and passed it to close method
sng06
-
Added automate fields and made changes to  email-related methods based on automate flag value
sng06
-
Made changes to close email txt template based on automate flag value
sng06
-
Added unit tests to review_request and test_email_sending test suites
sng06
-
Added default value to automate argument in close_review_request method
sng06
-
Changed assertEqual to assertTrue for test_post_commit_hook unit test
sng06
-
Added optional for automate field in docstrings
sng06
-
Fixed review_request_close unit test spy assertion
sng06
-
Added codes to render Review Board logo in review request change box when it is closed automatically
sng06
-
Added or edited docstrings and fixed formatting issues
sng06
-
Added class tag to RB logo avatar rendering in change.html template
sng06
-
Added unit tests to test changeEntry
sng06
-
Added automate field to webapi-request-fields decorator for both create and update methods
sng06
-
added unit tests to test PUT api for submitted and discarded actions
sng06
-
Deleted print statements
sng06
-
Edited automate field's description
sng06
-
Added automate field to draft resource PUT endpoint, review request publish-related email methods, draft and review request models
sng06
-
Fixed formatting and added docstring
sng06
-
Added unit tests to test draft-related methods
sng06
-
Changed review_request close unit test name
sng06
-
Changed automate to automated and edited unit tests docstrings
sng06
-
Fixed review request draft unit tests docstrings
sng06
-
Changed the conditional block to save automated value in extra context using extra_data field
sng06
-
Fixed formatting issues in review_request and review_request_draft unit tests
sng06
-
Fixed missing words and periods in docstrings
sng06
+
Added isAutomatedClose flag through close hook flow to ChangeDescription's extra_data field and added unit tests
sng06
+
Deleted print statements in unit tests
sng06
+
Changed variable name to automate
sng06
+
Added automate field to ReviewRequestResource API update method and passed it to close method
sng06
+
Added automate fields and made changes to  email-related methods based on automate flag value
sng06
+
Made changes to close email txt template based on automate flag value
sng06
+
Added unit tests to review_request and test_email_sending test suites
sng06
+
Added default value to automate argument in close_review_request method
sng06
+
Changed assertEqual to assertTrue for test_post_commit_hook unit test
sng06
+
Added optional for automate field in docstrings
sng06
+
Fixed review_request_close unit test spy assertion
sng06
+
Added codes to render Review Board logo in review request change box when it is closed automatically
sng06
+
Added or edited docstrings and fixed formatting issues
sng06
+
Added class tag to RB logo avatar rendering in change.html template
sng06
+
Added unit tests to test changeEntry
sng06
+
Added automate field to webapi-request-fields decorator for both create and update methods
sng06
+
added unit tests to test PUT api for submitted and discarded actions
sng06
+
Deleted print statements
sng06
+
Edited automate field's description
sng06
+
Added automate field to draft resource PUT endpoint, review request publish-related email methods, draft and review request models
sng06
+
Fixed formatting and added docstring
sng06
+
Added unit tests to test draft-related methods
sng06
+
Changed review_request close unit test name
sng06
+
Changed automate to automated and edited unit tests docstrings
sng06
+
Fixed review request draft unit tests docstrings
sng06
+
Changed the conditional block to save automated value in extra context using extra_data field
sng06
+
Fixed formatting issues in review_request and review_request_draft unit tests
sng06
+
Fixed missing words and periods in docstrings
sng06
+
Addressed code review feedback received
sng06

Diff:

Revision 10 (+1949 -433)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...