Summary

Add new API for working with default reviewers.

Review Request #3879 — Created Feb. 17, 2013 and submitted Feb. 18, 2013, 4:29 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-1.7.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

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.

Testing Done

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

Issues

Description	From	Last Updated
should this be "group names" instead of usernames?	SM smacleod	Feb. 18, 2013, 1:38 a.m.
Do these need to be explicitly provided? Aren't they going to default to those values anyway?	SM smacleod	Feb. 18, 2013, 1:40 a.m.
Col: 21 E126 continuation line over-indented for hanging indent	reviewbot	Feb. 18, 2013, 1:37 a.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Feb. 18, 2013, 1:37 a.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Feb. 18, 2013, 1:37 a.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 18, 2013, 1:37 a.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 18, 2013, 1:37 a.m.
Col: 80 E501 line too long (86 > 79 characters)	reviewbot	Feb. 18, 2013, 1:37 a.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Feb. 18, 2013, 1:37 a.m.
Col: 80 E501 line too long (103 > 79 characters)	reviewbot	Feb. 18, 2013, 1:37 a.m.
Col: 13 E122 continuation line missing indentation or outdented	reviewbot	Feb. 18, 2013, 1:37 a.m.
Col: 9 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 13 E122 continuation line missing indentation or outdented	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 9 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 80 E501 line too long (90 > 79 characters)	reviewbot	Feb. 18, 2013, 1:38 a.m.
I think this test was copied and pasted identically from above?	SM smacleod	Feb. 18, 2013, 1:39 a.m.
Col: 80 E501 line too long (98 > 79 characters)	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 80 E501 line too long (103 > 79 characters)	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 80 E501 line too long (106 > 79 characters)	reviewbot	Feb. 18, 2013, 1:38 a.m.
Col: 21 E126 continuation line over-indented for hanging indent	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (86 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (103 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 13 E122 continuation line missing indentation or outdented	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 9 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 13 E122 continuation line missing indentation or outdented	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 9 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (90 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (98 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (103 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.
Col: 80 E501 line too long (106 > 79 characters)	reviewbot	Feb. 18, 2013, 2:27 a.m.

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:

reviewboard/webapi/resources.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 21
 E126 continuation line over-indented for hanging indent
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (99 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (86 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (103 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 13
 E122 continuation line missing indentation or outdented
```

reviewboard/webapi/tests.py (Diff revision 1)

The issue has been dropped. Show all issues

Col: 9
 E123 closing bracket does not match indentation of opening bracket's line

reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 13
 E122 continuation line missing indentation or outdented
```

reviewboard/webapi/tests.py (Diff revision 1)

The issue has been dropped. Show all issues

Col: 9
 E123 closing bracket does not match indentation of opening bracket's line

reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (90 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (98 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (103 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (89 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (106 > 79 characters)
```

Other than a few small things, looks good to me!

reviewboard/webapi/resources.py (Diff revision 1)
The issue has been resolved. Show all issues
```
should this be "group names" instead of usernames?
```

reviewboard/webapi/resources.py (Diff revision 1)

The issue has been resolved. Show all issues

Do these need to be explicitly provided? Aren't they going to default to those values anyway?

reviewboard/webapi/resources.py (Diff revision 1)

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.

reviewboard/webapi/resources.py (Diff revision 1)
```
"transmogrify"... haha nice.
```

reviewboard/webapi/tests.py (Diff revision 1)

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

reviewboard/webapi/tests.py (Diff revision 1)
The issue has been resolved. Show all issues
```
I think this test was copied and pasted identically from above?
```

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.

Diff:

Revision 2 (+1086 -7)

Show changes

	reviewboard/reviews/forms.py
	reviewboard/reviews/managers.py
	reviewboard/reviews/models.py
	reviewboard/webapi/resources.py
	reviewboard/webapi/tests.py

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/webapi/tests.py
    reviewboard/reviews/forms.py
    reviewboard/reviews/managers.py
    reviewboard/webapi/resources.py
    reviewboard/reviews/models.py
  Ignored Files:

reviewboard/webapi/resources.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 21
 E126 continuation line over-indented for hanging indent
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (99 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (86 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (103 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 13
 E122 continuation line missing indentation or outdented
```

reviewboard/webapi/tests.py (Diff revision 2)

The issue has been dropped. Show all issues

Col: 9
 E123 closing bracket does not match indentation of opening bracket's line

reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 13
 E122 continuation line missing indentation or outdented
```

reviewboard/webapi/tests.py (Diff revision 2)

The issue has been dropped. Show all issues

Col: 9
 E123 closing bracket does not match indentation of opening bracket's line

reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (90 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (98 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (103 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (89 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/webapi/tests.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (106 > 79 characters)
```

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-1.6.x (05a6c2939f06b9abf94b529016d1c8f3dd57ce19)