Added automated_service_info field when closing or editing review requests automatically

Review Request #12223 — Created March 31, 2022 and updated

sheenaNg
Review Board
master
12051
reviewboard

Added automated_service_info field alongside automated flag to
provide details on the service that updated/closed the review
request automatically.

  • Added automated_service_info field to hook_utils,
    ReviewRequestResourceAPI Update(), and
    review_request.close() to store the value in
    ChangeDescription model.

  • Added automated_service_info field to
    ReviewRequestResourceDraftAPI Update(), Create(),
    review_request.publish(), draft.publish() to store the value
    in ChangeDescription model.

  • Added automated_service_info field to closed/published
    signals and email-related methods.

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

Summary Author
Resolved rebase conflict
sng06
Resolved rebase conflicts in unit tests
sng06
Deleted automated_service_info from signal handlers and prepare_review_request_mail method and addressed code review feedback
sng06
Fixed docstring typo for webapi/tests/test_review_request unit tests
sng06
Description From Last Updated

It would be nice if all of the docstrings for functions with automated_service_info arguments described the keys that can be …

KY kylemclean

Instead of defining a variable, let's pass this inline as automated_service_info={'service_name': hosting_service_id}

daviddavid

This could potentially be shortened to remove the if statement by utilizing a new variable and the fact that the …

gdehalgdehal

This needs a trailing comma.

daviddavid
sheenaNg
gdehal
  1. 
      
  2. This could potentially be shortened to remove the if statement by utilizing a new variable and the fact that the get function defaults to None if nothing is found.

    automated_service_info = extra_data.get('automated_service_info') extra_context.update({ 'automated_service_info': automated_service_info })

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

    Instead of defining a variable, let's pass this inline as automated_service_info={'service_name': hosting_service_id}

  3. This needs a trailing comma.

  4. 
      
sheenaNg
Review request changed

Change Summary:

Addressed code review feedback.

Commits:

Summary Author
-
Added automated_service_info field
sng06
-
Resolved rebase merge conflicts
sng06
+
Resolved rebase conflict
sng06
+
Resolved rebase conflicts in unit tests
sng06
+
Deleted automated_service_info from signal handlers and prepare_review_request_mail method and addressed code review feedback
sng06
+
Fixed docstring typo for webapi/tests/test_review_request unit tests
sng06

Diff:

Revision 2 (+561 -187)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
KY
  1. 
      
  2. It would be nice if all of the docstrings for functions with automated_service_info arguments described the keys that can be in the dictionary.

    1. Hi Kyle, thank you for the suggestion! For now, this is a dictionary field that can take any arbitrary data passed in by the user. I think the goal is to define what the client can pass in separately from the implmentation.

  3. 
      
Loading...