Add new API for working with default reviewers.
Review Request #3879 — Created Feb. 17, 2013 and submitted
Add new API for working with default reviewers. We never had API for default reviewers before, meaning that it was impossible for scripts to query or manipulate them. This introduces POST, PUT, DELETE and GET support for default reviewers. The functionality is pretty standard. Querying the lists, though, has some nice filtering to narrow down by repositories, users, and groups. To get the errors to propagate correctly, DefaultReviewerForm has been updated to tie any user/group/repository validation errors to the appropriate fields. 33 new unit tests were added.
All new unit tests pass. Testing the DELETE default-reviewers/<id>/ API ... ok Testing the DELETE default-reviewers/<id>/ API with Permission Denied error ... ok Testing the DELETE default-reviewers/<id>/ API with a local site ... ok Testing the DELETE default-reviewers/<id>/ API with a local site and Permission Denied error ... ok Testing the GET default-reviewers/<id>/ API ... ok Testing the GET default-reviewers/<id>/ API with Not Modified response ... ok Testing the GET default-reviewers/<id>/ API with a local site ... ok Testing the GET default-reviewers/<id>/ API with a local site and Permission Denied error ... ok Testing the GET default-reviewers/ API ... ok Testing the GET default-reviewers/?groups= API ... ok Testing the GET default-reviewers/?repositories= API ... ok Testing the GET default-reviewers/ API with a local site ... ok Testing the GET default-reviewers/ API with a local site and Permission Denied error ... ok Testing the GET default-reviewers/?users= API ... ok Testing the POST default-reviewers/ API ... ok Testing the POST default-reviewers/ API with field defaults ... ok Testing the POST default-reviewers/ API with group and invalid site ... ok Testing the POST default-reviewers/ API with invalid group ... ok Testing the POST default-reviewers/ API with invalid repository ... ok Testing the POST default-reviewers/ API with invalid username ... ok Testing the POST default-reviewers/ API with a local site and Permission Denied error ... ok Testing the POST default-reviewers/ API with repository and invalid site ... ok Testing the POST default-reviewers/ API with a local site ... ok Testing the POST default-reviewers/ API with user and invalid site ... ok Testing the PUT default-reviewers/<id>/ API ... ok Testing the PUT default-reviewers/<id>/ API with group and invalid site ... ok Testing the PUT default-reviewers/<id>/ API with invalid group ... ok Testing the PUT default-reviewers/<id>/ API with invalid repository ... ok Testing the PUT default-reviewers/<id>/ API with invalid username ... ok Testing the PUT default-reviewers/<id>/ API with a local site and Permission Denied error ... ok Testing the PUT default-reviewers/<id>/ API with repository and invalid site ... ok Testing the PUT default-reviewers/<id>/ API with a local site ... ok Testing the PUT default-reviewers/<id>/ API with user and invalid site ... ok ---------------------------------------------------------------------- Ran 33 tests in 7.308s OK
| Description | From | Last Updated |
|---|---|---|
|
should this be "group names" instead of usernames? |
SM smacleod | |
|
Do these need to be explicitly provided? Aren't they going to default to those values anyway? |
SM smacleod | |
|
Col: 21 E126 continuation line over-indented for hanging indent |
 reviewbot
|
|
|
Col: 80 E501 line too long (82 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (99 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (80 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (86 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (82 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (103 > 79 characters) |
reviewbot
|
|
|
Col: 13 E122 continuation line missing indentation or outdented |
reviewbot
|
|
|
Col: 9 E123 closing bracket does not match indentation of opening bracket's line |
reviewbot
|
|
|
Col: 80 E501 line too long (84 > 79 characters) |
reviewbot
|
|
|
Col: 13 E122 continuation line missing indentation or outdented |
reviewbot
|
|
|
Col: 9 E123 closing bracket does not match indentation of opening bracket's line |
reviewbot
|
|
|
Col: 80 E501 line too long (85 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (90 > 79 characters) |
reviewbot
|
|
|
I think this test was copied and pasted identically from above? |
SM smacleod | |
|
Col: 80 E501 line too long (98 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (103 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (84 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (89 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (106 > 79 characters) |
reviewbot
|
|
|
Col: 21 E126 continuation line over-indented for hanging indent |
reviewbot
|
|
|
Col: 80 E501 line too long (82 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (99 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (80 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (86 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (82 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (103 > 79 characters) |
reviewbot
|
|
|
Col: 13 E122 continuation line missing indentation or outdented |
reviewbot
|
|
|
Col: 9 E123 closing bracket does not match indentation of opening bracket's line |
reviewbot
|
|
|
Col: 80 E501 line too long (84 > 79 characters) |
reviewbot
|
|
|
Col: 13 E122 continuation line missing indentation or outdented |
reviewbot
|
|
|
Col: 9 E123 closing bracket does not match indentation of opening bracket's line |
reviewbot
|
|
|
Col: 80 E501 line too long (85 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (90 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (98 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (103 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (84 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (89 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (106 > 79 characters) |
reviewbot
|
SM
-
Other than a few small things, looks good to me!
-
-
-
I'm glad you added the filtering. I've come across a couple of resources which lack this sort of thing which makes it a pain.
-
-
Test coverage ftw :D Do we want to add a test which checks the behaviour of POSTing to a request which already has groups / users, and what happens when we provide those arguemnts (i.e. Test overwriting the previous groups / users).
-
- Change Summary:
-
* Fixed documentation that referenced usernames instead of group names. * Updated the PUT test to override all fields, not just the name and file_regex. * Removed an extra testcase (copy/paste error). * Removed the mimetype settings, since these are auto-computed.
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/webapi/tests.py reviewboard/reviews/managers.py reviewboard/reviews/models.py reviewboard/webapi/resources.py reviewboard/reviews/forms.py Ignored Files: