Summary:

Added automated_service_info field when closing or editing review requests automatically

Review Request #12223 — Created March 31, 2022 and updated April 7, 2022, 11:31 p.m.

Information
Owner:	sheenaNg
Repository:	Review Board
Branch:	master
Bugs:
Depends On:	12051
Reviewers
Groups:	reviewboard
People:

Description

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.

Testing Done

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.

Commits

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

Screenshots

Issues

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	April 7, 2022, 11:31 p.m.
Instead of defining a variable, let's pass this inline as automated_service_info={'service_name': hosting_service_id}	david	April 6, 2022, 6:34 p.m.
This could potentially be shortened to remove the if statement by utilizing a new variable and the fact that the …	gdehal	April 6, 2022, 6:34 p.m.
This needs a trailing comma.	david	April 6, 2022, 6:34 p.m.

flake8 passed.

JSHint passed.

Summary:

-	[WIP] Added automated_service_info field
+	Added automated_service_info field when closing or editing review requests automatically

reviewboard/notifications/email/message.py (Diff revision 1)

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 
 })

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}

reviewboard/webapi/resources/review_request_draft.py (Diff revision 1)
Show all issues
```
This needs a trailing comma.
```

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

	reviewboard/hostingsvcs/hook_utils.py
	reviewboard/hostingsvcs/tests/test_bitbucket.py
	reviewboard/hostingsvcs/tests/test_github.py
	reviewboard/hostingsvcs/tests/test_rbgateway.py
	reviewboard/notifications/email/message.py
	reviewboard/notifications/tests/test_email_sending.py
	reviewboard/reviews/signals.py
	reviewboard/reviews/models/review_request.py
	7 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

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.

sheenaNg April 7, 2022, 11:31 p.m.

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.