Added automated_service_info field when closing or editing review requests automatically

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

Information

Review Board
master

Reviewers

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 ID Author
Resolved rebase conflict
1a7a9f92fb96a621c4fd3b261557f9fd77dba659 sng06
Resolved rebase conflicts in unit tests
89a4fd6603ab502d442b3e41f2c7fbc71bd36151 sng06
Deleted automated_service_info from signal handlers and prepare_review_request_mail method and addressed code review feedback
2c2bb0e84ffa70cfa56a6d9de47f96e6f1aed39a sng06
Fixed docstring typo for webapi/tests/test_review_request unit tests
65e1fdbe179ddf4febd361e142db10529296ee43 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. Show all issues

    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)
     
     
     
     
    Show all issues

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

  3. Show all issues

    This needs a trailing comma.

  4. 
      
sheenaNg
Review request changed
Change Summary:

Addressed code review feedback.

Commits:
Summary ID Author
Added automated_service_info field
4a2802f006b8e1b70575af7040d591707e0523a1 sng06
Resolved rebase merge conflicts
da0a9497e63a328ad78f8b23057947497bf089c3 sng06
Resolved rebase conflict
1a7a9f92fb96a621c4fd3b261557f9fd77dba659 sng06
Resolved rebase conflicts in unit tests
89a4fd6603ab502d442b3e41f2c7fbc71bd36151 sng06
Deleted automated_service_info from signal handlers and prepare_review_request_mail method and addressed code review feedback
2c2bb0e84ffa70cfa56a6d9de47f96e6f1aed39a sng06
Fixed docstring typo for webapi/tests/test_review_request unit tests
65e1fdbe179ddf4febd361e142db10529296ee43 sng06

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
KY
  1. 
      
  2. Show all issues

    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.