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

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

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.