Summary

API method to get the diffsets of a review request

Review Request #1013 — Created Sept. 1, 2009 and submitted

Information

Owner

mak

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Currently there is no support for this. But it's needed for clients who are interested in the diffs of a review request.

Testing Done

Includes a test.

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

I think I'd prefer that this just be /diff/.

mak Sept. 17, 2009, 7:31 a.m.

OK, that was actually my original choice...

Diff:

Revision 2 (+27 -1)

Show changes

	reviewboard/webapi/json.py
	reviewboard/webapi/tests.py
	reviewboard/webapi/urls.py

Ship it!

Looks good. Let us know your github username and we'll set you up with commit access.

mknittig Nov. 3, 2009, 1:08 a.m.

Sorry, I didn't see this earlier but for some reason I use the account "mak" for this request instead of "mknittig", so I didn't get a mail. My GitHub Account is "mknittig".

chipx86 Nov. 4, 2009, 2:57 p.m.

You're added. Make sure to reference this review request by appending the following to your commit message:

Reviewed at http://reviews.reviewboard.org/r/1013/

MK

mknittig Nov. 4, 2009, 6:39 p.m.
```
Thanks. Submitted.
```

mknittig Nov. 4, 2009, 7:05 p.m.

Can you give me write access to the wiki at Google Code so I can document to API method? My Google account username is "markus@myd0.de".

chipx86 Nov. 4, 2009, 7:12 p.m.
```
Should work now.
```

mknittig Nov. 4, 2009, 7:56 p.m.

Thanks, but I'll just noticed a bug. :-/ In urls.py I use "diff" and in tests.py I use "diffs". While "diff" is more consistent with the current API, logically "diffs" would be the better name...

chipx86 Nov. 5, 2009, 4:41 a.m.

Yeah, the naming is perhaps confusing, but I'd prefer consistency in this case. I don't think it's so bad, though I agree that diffs may be more correct if we were doing this from scratch.

reviewboard/webapi/json.py (Diff revision 2)
```
This is from Kamal
```

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

Nice patch; I just did the same thing myself and was getting ready to submit it for review!

I have seen http 500 errors if you call "review_request.diffset_history.diffsets.all()" or ".latest()", and there are no diffsets.

The existing code typically does the following:

  public_diffsets = review_request.diffset_history.diffsets
  if public_diffsets.count() > 0:
    ...

My original code didn't have that code and was throwing 500 on some review requests.
I added that "count() > 0" check and haven't seen one since.

Christian, can you comment on whether this should be added?

Thanks all!

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

I'd say this should definitely be changed to "diffs".