Summary

Add initial web API for commit histories.

Review Request #6816 — Created Jan. 20, 2015 and submitted Feb. 27, 2015, 7:40 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

dvcs

Bugs

Depends On

~~6815~~

Blocks

6933, 6787, 6931

Commit

617622a...

Reviewers

Groups

reviewboard

People

chipx86, mike_conley, smacleod

Description

Add the DiffCommitResource and DraftDiffCommitResource resources.
The DiffCommitResource is a resource for retrieving the information
about the DiffCommits under a particular DiffSet. The
DraftDiffCommitResource inherits from the DiffCommitResource and
modifies it to allow for the creation of DiffCommits and associated
FileDiffs. The DraftDiffCommitResource handles the uploading of
FileDiffs for DiffCommits instead of the DiffSetResource.

The DiffSetResource POST method now takes an extra optional field,
create_empty, that determines if the DiffSet that is created is
created without uploading a .diff file. This is used in the creation
of review requests with commit histories.

The FileDiffResource and DraftFileDiffResource classes now filter
their results based on the commit query parameter. If no commit query
parameter is given, only those FileDiffs unattached to a
DiffCommit will be return. This filtering is only applied to the
list API, not the item API.

Add a new form, the UploadCommitForm, for handling the processing of
data for creating new DiffCommit instances.

Refactor behaviour for uploading FileDiffs from the DiffSetManager
to a new class, the DiffProcessorMixin. This mix-in handles the
initial processing of .diff files, their comparison (for header vs.
source files), generating file lists, and creating actual FileDiffs
from upload (it calls the child class's implementation of
create_from_data).

The DiffSetManager now has a create_empty method for creating
DiffSets without FileDiffs for usage with DiffCommits.

Add the DiffCommitManager model. It inherits from the
DiffProcessorMixin.

Add a WebAPI capability to show that the server supports review
requests with commit histories. This allows new versions of rbt that
support commit histories to create review requests with history by
default if the tool supports it.

Add new unit tests for the web API. The tests include:
- test_filediff, which tests FileDiff filtering of a DiffCommit
- test_draft_diff_commit, which does the base API tests for the
DraftDiffCommitResource
- test_diff_commit, which does the base API tests for the
DiffCommitResource

Testing Done

Unit tests pass.
Did the following manual testing:

Created a review request via rbt post. It was automatically

   created with history. (There was one commit under the draft).
Able to read the commit list and single commit via the API.
Able to filter the FileDiff list resource.
The FileDiff item resource was not filtered.
Able to publish, discard, and delete the review request.
Able to use all the public API endoint functionality of the

   published RR anonymously

Issues

Description	From	Last Updated
No blank line here.	chipx86	Jan. 22, 2015, 12:18 p.m.
Nit - we like to put these in alphabetical order. from reviewboard.diffviewer.forms... should go before from reviewboard.diffviewer.models...	mike_conley	Jan. 22, 2015, 10:04 a.m.
We also like trailing commas! :D	mike_conley	Jan. 22, 2015, 10:04 a.m.
Is this necessary? Like, if we provide no file for the path parameter, doesn't that essentially mean "Create an empty …	mike_conley	Jan. 22, 2015, 1:34 p.m.
Maybe specifically say "commit history," so that people looking at the docs don't think this is required for standard diff …	chipx86	Jan. 29, 2015, 7:21 a.m.
You should be able to put this in the argument list for the function.	chipx86	Jan. 29, 2015, 7:21 a.m.
What are we using this for in the form? Any way we can allow DiffSet.save to deal with this? No …	chipx86	Jan. 22, 2015, 12:46 p.m.
Can you flesh this out more? This directly turns into documentation on the website for the API, so we should …	chipx86	Jan. 28, 2015, 1:13 p.m.
We should probably say that this is a list of IDs.	chipx86	Jan. 22, 2015, 12:20 p.m.
Instead of describing this in the docs, you can do: 'commit_type': { 'type': ('M', 'C'), ... } It will then …	chipx86	Jan. 22, 2015, 12:22 p.m.
Missing period.	chipx86	Jan. 29, 2015, 7:23 a.m.
If all we need are the commit_id values, we can reduce the query and deserialization overhead by doing: return obj.merge_parent_ids.values_list('commit_id', …	chipx86	Jan. 22, 2015, 12:26 p.m.
Missing period.	chipx86	Jan. 29, 2015, 7:23 a.m.
As above, docstrings for this and the other handler methods will all be turned into public documentation on the website. …	chipx86	Jan. 28, 2015, 1:13 p.m.
I wouldn't expect this to be needed. It should be handled by the parent resource.	chipx86	Jan. 22, 2015, 12:52 p.m.
Same as above.	chipx86	Jan. 22, 2015, 12:52 p.m.
This can be one line.	chipx86	Jan. 22, 2015, 12:52 p.m.
Maybe ValueError? I would also log this, so that it's easier for admins to see the issue. Use exc_info=1 with …	chipx86	Feb. 5, 2015, 5:07 p.m.
No blank line.	chipx86	Jan. 22, 2015, 12:52 p.m.
""" on the next line.	chipx86	Jan. 29, 2015, 7:25 a.m.
If accessing this with a query parameter, you can end up with something like <url>/?existing-param=123?commit_id=.... We need to check if …	chipx86	Jan. 22, 2015, 12:12 p.m.
This will be used in the public documentation on the site, so the internal-only extra_fields doesn't mean much. I'd say …	chipx86	Jan. 29, 2015, 7:29 a.m.
Could simplify just a tad with: if '?' in url: url += '&' else: url += '?' return '%scommit-id=%s' % …	chipx86	Jan. 29, 2015, 7:31 a.m.
These should be listed in alphabetical order.	chipx86	Jan. 22, 2015, 12:13 p.m.
These are probably copy/pasted from the other file, but these should also be in alphabetical order.	chipx86	Jan. 22, 2015, 12:13 p.m.
This one too.	chipx86	Jan. 22, 2015, 12:14 p.m.
Same comments on docs will apply here and elsewhere.	chipx86	Jan. 28, 2015, 1:13 p.m.
This is one of those optimization tricks you learn over time, but this would be easier on the database if …	chipx86	Jan. 22, 2015, 2:24 p.m.
As this is the DraftDiffCommitResource, Do we not need to ensure that the user associated with the request can view …	mike_conley	Jan. 21, 2015, 9:27 a.m.
This should go above the HTTP handlers.	chipx86	Jan. 22, 2015, 12:34 p.m.
As above, you can use a tuple of allowed values. And also, we should probably use "merge" or "change".	chipx86	Jan. 22, 2015, 12:33 p.m.
Nit - please add a trailing comma	mike_conley	Jan. 22, 2015, 10:05 a.m.
Nit - please add a trailing comma	mike_conley	Jan. 22, 2015, 10:06 a.m.
list?	chipx86	Jan. 22, 2015, 1:03 p.m.
I might be missing it, but what's expecting this to be {}? The usage below checking if it's valid only …	chipx86	Jan. 22, 2015, 12:32 p.m.
No blank line between try/excepts.	chipx86	Jan. 22, 2015, 12:32 p.m.
Same here.	chipx86	Jan. 22, 2015, 12:32 p.m.
This is an internal detail, but not one that's useful to consumers of the API. We should avoid talking about …	chipx86	Feb. 5, 2015, 5:19 p.m.
'commit-id'	chipx86	Jan. 22, 2015, 12:30 p.m.
This looks the same as the addition in DraftFileDiffResource. Any way to DRY it up?	mike_conley	Jan. 22, 2015, 10:16 a.m.
This might be better suited to diffs above.	chipx86	Jan. 22, 2015, 12:58 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Jan. 22, 2015, 10:03 a.m.
You probably didn't mean to leave this here.	mike_conley	Jan. 22, 2015, 10:06 a.m.
Blank line here.	chipx86	Jan. 22, 2015, 12:31 p.m.
Missing trailing slash.	chipx86	Jan. 22, 2015, 12:31 p.m.
'DiffCommit' imported but unused	reviewbot	Jan. 22, 2015, 10:03 a.m.
'resources' imported but unused	reviewbot	Jan. 22, 2015, 10:03 a.m.
Col: 5 E265 block comment should start with '# '	reviewbot	Jan. 22, 2015, 10:04 a.m.
Left over from something?	chipx86	Jan. 22, 2015, 12:31 p.m.
No blank line needed here	chipx86	Jan. 22, 2015, 12:31 p.m.
Col: 1 E303 too many blank lines (3)	reviewbot	Jan. 23, 2015, 1:19 p.m.
'DiffCommit' imported but unused	reviewbot	Jan. 23, 2015, 1:19 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Jan. 28, 2015, 12:18 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Jan. 28, 2015, 12:18 p.m.
'DiffSet' imported but unused	reviewbot	Jan. 28, 2015, 1:15 p.m.
This will be directly rendered as ReST into the docs, so we should place double backticks around with_history and path, …	chipx86	Feb. 5, 2015, 5:08 p.m.
No blank line here.	chipx86	Feb. 5, 2015, 5:09 p.m.
Maybe "Each diff commit resource contains ..."	chipx86	Feb. 5, 2015, 5:09 p.m.
"to reproduce"	chipx86	Feb. 5, 2015, 5:09 p.m.
This might be mistaken for a hash. Maybe "The numeric ID of the commit resource."	chipx86	Feb. 5, 2015, 5:11 p.m.
"The e-mail address of ..." Other docs use "e-mail address," so we should be consistent there.	chipx86	Feb. 5, 2015, 5:11 p.m.
I wonder if we want to say "commit message" instead?	chipx86	Feb. 5, 2015, 5:11 p.m.
"DiffSets" are internal. Let's just say "on a particular commit."	chipx86	Feb. 5, 2015, 5:12 p.m.
Technically, it also has things like likes, extra_data, etc. Maybe we don't want to say "currently only," but instead something …	chipx86	Feb. 5, 2015, 5:13 p.m.
Same note about DiffSets.	chipx86	Feb. 5, 2015, 5:13 p.m.
"DiffCommit" is also internal, so let's say "Updates information on the commit."	chipx86	Feb. 5, 2015, 5:14 p.m.
Too many blank lines.	chipx86	Feb. 5, 2015, 5:14 p.m.
"resource"	chipx86	Feb. 5, 2015, 5:14 p.m.
Same comment regarding internal names. I don't think, from the consumer's perspective, that we even need this last sentence, since …	chipx86	Feb. 5, 2015, 5:14 p.m.
I'd remove "... in a DiffSet."	chipx86	Feb. 5, 2015, 5:14 p.m.
Same comment here as in the other resource.	chipx86	Feb. 5, 2015, 5:14 p.m.
"... pertaining to the diff revision." ?	chipx86	Feb. 5, 2015, 5:15 p.m.
"e-mail address"	chipx86	Feb. 5, 2015, 5:15 p.m.
"e-mail address"	chipx86	Feb. 5, 2015, 5:15 p.m.
"DiffCommit" is an internal name.	chipx86	Feb. 5, 2015, 5:16 p.m.
"DiffSet" is an internal name. Here and elsewhere in this docstring.	chipx86	Feb. 5, 2015, 5:17 p.m.
No blank line.	chipx86	Feb. 5, 2015, 5:17 p.m.
This line should no longer be needed.	chipx86	Feb. 5, 2015, 5:18 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 5, 2015, 6:42 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 5, 2015, 6:42 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Feb. 6, 2015, 3:14 p.m.
Col: 5 E303 too many blank lines (2)	reviewbot	Feb. 6, 2015, 3:14 p.m.
Col: 5 E303 too many blank lines (2)	reviewbot	Feb. 6, 2015, 3:13 p.m.
'os' imported but unused	reviewbot	Feb. 12, 2015, 1:27 p.m.
'scmtools' imported but unused	reviewbot	Feb. 12, 2015, 1:27 p.m.
local variable 'diff_filename' is assigned to but never used	reviewbot	Feb. 12, 2015, 1:28 p.m.
'os' imported but unused	reviewbot	Feb. 12, 2015, 1:28 p.m.
'scmtools' imported but unused	reviewbot	Feb. 12, 2015, 1:28 p.m.
This could just be if has_path == with_history	david	Feb. 24, 2015, 8:29 a.m.
This should end in a period.	david	Feb. 24, 2015, 8:33 a.m.
Missing period.	chipx86	Feb. 27, 2015, 8:47 a.m.
"commits"	chipx86	Feb. 27, 2015, 8:47 a.m.
"so that it's created empty"	chipx86	Feb. 27, 2015, 8:48 a.m.
Maybe verify it is indeed one of the valid values?	chipx86	Feb. 27, 2015, 8:50 a.m.
Missing a trailing slash.	chipx86	Feb. 27, 2015, 8:50 a.m.

Tool: Pyflakes
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/webapi/resources/draft_diff.py
    reviewboard/webapi/resources/__init__.py
    reviewboard/webapi/resources/draft_diff_commit.py
    reviewboard/webapi/server_info.py
    reviewboard/webapi/resources/filediff.py
    reviewboard/webapi/tests/test_draft_diff_commit.py
    reviewboard/webapi/tests/test_diff_commit.py
    reviewboard/webapi/tests/urls.py
    reviewboard/webapi/tests/test_filediff.py
    reviewboard/webapi/tests/mimetypes.py
    reviewboard/webapi/resources/diff.py
    reviewboard/webapi/resources/diff_commit.py
    reviewboard/webapi/resources/validate_diff.py
    reviewboard/webapi/resources/draft_filediff.py

reviewboard/webapi/tests/test_filediff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'DiffCommit' imported but unused
```
reviewboard/webapi/tests/test_filediff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'resources' imported but unused
```

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/webapi/resources/draft_diff.py
    reviewboard/webapi/resources/__init__.py
    reviewboard/webapi/resources/draft_diff_commit.py
    reviewboard/webapi/server_info.py
    reviewboard/webapi/resources/filediff.py
    reviewboard/webapi/tests/test_draft_diff_commit.py
    reviewboard/webapi/tests/test_diff_commit.py
    reviewboard/webapi/tests/urls.py
    reviewboard/webapi/tests/test_filediff.py
    reviewboard/webapi/tests/mimetypes.py
    reviewboard/webapi/resources/diff.py
    reviewboard/webapi/resources/diff_commit.py
    reviewboard/webapi/resources/validate_diff.py
    reviewboard/webapi/resources/draft_filediff.py

reviewboard/webapi/tests/test_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
reviewboard/webapi/tests/test_filediff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 5
 E265 block comment should start with '# '
```

Summary:: Update the diffviewer package.
Add initial web API for commit histories.

People:: mike_conley

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

The issue has been resolved. Show all issues

Nit - we like to put these in alphabetical order.

from reviewboard.diffviewer.forms...

should go before

from reviewboard.diffviewer.models...

reviewboard/webapi/resources/diff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We also like trailing commas! :D
```

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

The issue has been dropped. Show all issues

Is this necessary? Like, if we provide no file for the path parameter, doesn't that essentially mean "Create an empty DiffResource"?

brennie Jan. 21, 2015, 9:27 a.m.

Oops. That wasn't supposed to be empty.

The idea with the create_empty parameter is that we want the client to KNOW it is creating a review request involving commit history. It could be possible for incompatible tools to go screw up uploading a file diff and create empty diffsets accidentally.

chipx86 Jan. 21, 2015, 3:04 p.m.

Hmm, I can see both sides here.

How about a third object: Some sort of "type" field that specifies if it's for a single diff or a commit set, defaulting to a single diff? We could then do the appropriate validation. Also gives us room for future expansion, if needed.

brennie Jan. 22, 2015, 1:34 p.m.

I think that in the future, we can refactor this into a new type field if necessary. If we add a type field, then one of the types is old (or similar) and the other type is new (or similar). This must default to the old type, but we can't require its existence if the version of RBTools is old. So it basically is identical to the create_empty field.

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

The issue has been dropped. Show all issues

As this is the DraftDiffCommitResource, Do we not need to ensure that the user associated with the request can view this information (ie, be at least staff level, or be the submitter)?

brennie Jan. 21, 2015, 9:26 a.m.

Thats what the has_access_permissions and has_list_access_permissions functions are for.

reviewboard/webapi/resources/draft_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Nit - please add a trailing comma
```
reviewboard/webapi/resources/draft_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Nit - please add a trailing comma
```

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

The issue has been resolved. Show all issues

This looks the same as the addition in DraftFileDiffResource. Any way to DRY it up?

brennie Jan. 22, 2015, 10:16 a.m.

Refactored the common code into a new method :)

reviewboard/webapi/tests/test_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
You probably didn't mean to leave this here.
```

reviewboard/testing/testcase.py (Diff revision 1)
The issue has been resolved. Show all issues
```
No blank line here.
```

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

The issue has been resolved. Show all issues

What are we using this for in the form? Any way we can allow DiffSet.save to deal with this? No big deal if not (there's unlikely to be a race condition in this particular case), but worth asking.
Also, since this is very close to the logic in DiffSet.save, maybe we can just extract that to a function on DiffSetManager and use that in both places?

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

The issue has been resolved. Show all issues

Can you flesh this out more? This directly turns into documentation on the website for the API, so we should be as clear and explicit about the purposes of the resource as we can.

reviewboard/webapi/resources/diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We should probably say that this is a list of IDs.
```

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

The issue has been resolved. Show all issues

Instead of describing this in the docs, you can do:

'commit_type': {
    'type': ('M', 'C'),
    ...
}

It will then automatically be validated when the user posts, without you having to do anything.

I'd prefer we use "merge" or "change" though, instead of "M" or "C".

brennie Jan. 22, 2015, 12:21 p.m.

The posting only happens on the DraftDiffCommit.create method so should I put that on there instead of here?

chipx86 Jan. 22, 2015, 1:03 p.m.

Both places. The one for create will be shown on the list of possible fields for a POST in the docs, and the one in fields will be shown for the resource payload fields docs.

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

The issue has been resolved. Show all issues

If all we need are the commit_id values, we can reduce the query and deserialization overhead by doing:

return obj.merge_parent_ids.values_list('commit_id', flat=True)

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

The issue has been resolved. Show all issues

As above, docstrings for this and the other handler methods will all be turned into public documentation on the website. They should contain more detailed information. You can see examples in other resources.

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

The issue has been resolved. Show all issues

I wouldn't expect this to be needed. It should be handled by the parent resource.

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

The issue has been resolved. Show all issues

Same as above.

reviewboard/webapi/resources/diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This can be one line.
```
reviewboard/webapi/resources/diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
No blank line.
```

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

The issue has been resolved. Show all issues

If accessing this with a query parameter, you can end up with something like <url>/?existing-param=123?commit_id=.... We need to check if ? is already in there, and then choose between ? and &.

Also, this should be commit-id=. We don't use _ (except in a couple legacy cases) in query strings.

reviewboard/webapi/resources/draft_diff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
These should be listed in alphabetical order.
```

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

The issue has been resolved. Show all issues

These are probably copy/pasted from the other file, but these should also be in alphabetical order.

reviewboard/webapi/resources/draft_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This one too.
```
reviewboard/webapi/resources/draft_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same comments on docs will apply here and elsewhere.
```

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

The issue has been resolved. Show all issues

This is one of those optimization tricks you learn over time, but this would be easier on the database if you do:

draft = ReviewRequestDraft.objects.get(diffset__commits=commit)
return draft.is_accessible_by(request.user)

brennie Jan. 22, 2015, 1:57 p.m.

This method won't actually work because diff_commits is a related field generated by the ORM.

brennie Jan. 22, 2015, 2:24 p.m.
```
Never mind me :)
```

reviewboard/webapi/resources/draft_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This should go above the HTTP handlers.
```

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

The issue has been resolved. Show all issues

As above, you can use a tuple of allowed values. And also, we should probably use "merge" or "change".

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

The issue has been dropped. Show all issues

list?

brennie Jan. 22, 2015, 12:35 p.m.

When you post to the resource, we are sending a comma-seperated string. Should this be list or text type?

chipx86 Jan. 22, 2015, 1:03 p.m.

Sorry, I had this wrong. For posting, it'll need to be a comma-separated string, not a list. Looking at webapi_request_fields, list actually implies a set of specific choices, same as the tuple I mentioned above.

(We may want to extend Djblets's support at some point to have some type indicating comma-separated lists...)

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

The issue has been resolved. Show all issues

I might be missing it, but what's expecting this to be {}? The usage below checking if it's valid only checks for truthiness.

reviewboard/webapi/resources/draft_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
No blank line between try/excepts.
```
reviewboard/webapi/resources/draft_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/webapi/resources/draft_filediff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
'commit-id'
```
reviewboard/webapi/server_info.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This might be better suited to diffs above.
```
reviewboard/webapi/tests/test_draft_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line here.
```
reviewboard/webapi/tests/test_draft_diff_commit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing trailing slash.
```
reviewboard/webapi/tests/test_filediff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Left over from something?
```
reviewboard/webapi/tests/test_filediff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
No blank line needed here
```

Change Summary:

Address issues.

Diff:

Revision 2 (+1688 -203)

	reviewboard/diffviewer/admin.py
	reviewboard/diffviewer/forms.py
	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/models.py
	reviewboard/diffviewer/tests.py
	reviewboard/diffviewer/evolutions/__init__.py
	reviewboard/diffviewer/evolutions/add_commit_history_fields.py
	reviewboard/testing/testcase.py
	14 more

	reviewboard/testing/testcase.py
	reviewboard/webapi/server_info.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/diff.py
	reviewboard/webapi/resources/diff_commit.py
	reviewboard/webapi/resources/draft_diff.py
	reviewboard/webapi/resources/draft_diff_commit.py
	reviewboard/webapi/resources/draft_filediff.py
	7 more

	reviewboard/diffviewer/forms.py
	reviewboard/reviews/forms.py
	reviewboard/testing/testcase.py
	reviewboard/webapi/server_info.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/diff.py
	reviewboard/webapi/resources/diff_commit.py
	reviewboard/webapi/resources/draft_diff.py
	9 more

		Add the `DiffCommitResource` and `DraftDiffCommitResource` resources.
		The `DiffCommitResource` is a resource for retrieving the information
		about the `DiffCommits` under a particular `DiffSet`. The
		`DraftDiffCommitResource` inherits from the `DiffCommitResource` and
		modifies it to allow for the creation of `DiffCommits` and associated
		`FileDiffs`. The `DraftDiffCommitResource` handles the uploading of
		`FileDiffs` for `DiffCommits` instead of the `DiffSetResource`.

		The `DiffSetResource` POST method now takes an extra optional field,
		`create_empty`, that determines if the DiffSet that is created is
		created without uploading a `.diff file`. This is used in the creation
		of review requests with commit histories.

		The `FileDiffResource` and `DraftFileDiffResource` classes now filter
		their results based on the commit query parameter. If no commit query
		parameter is given, only those `FileDiffs` unattached to a
		`DiffCommit` will be return. This filtering is only applied to the
		list API, not the item API.

		Add a new form, the `UploadCommitForm`, for handling the processing of
		data for creating new `DiffCommit` instances.

		Refactor behaviour for uploading `FileDiffs` from the `DiffSetManager`
		to a new class, the `DiffProcessorMixin`. This mix-in handles the
		initial processing of `.diff` files, their comparison (for header vs.
		source files), generating file lists, and creating actual `FileDiffs`
		from upload (it calls the child class's implementation of
		`create_from_data`).

		The `DiffSetManager` now has a `create_empty` method for creating
		`DiffSets` without `FileDiffs` for usage with `DiffCommits`.

		Add the `DiffCommitManager` model. It inherits from the
		`DiffProcessorMixin`.

-		Update the `validate_diff` endpoint to use named parameters for
-		`DiffSetManager.create_from_upload` because of the refactoring.
-
		Add a WebAPI capability to show that the server supports review
		requests with commit histories. This allows new versions of `rbt` that
		support commit histories to create review requests with history by
		default if the tool supports it.

		Add new unit tests for the web API. The tests include:
		- `test_filediff`, which tests `FileDiff` filtering of a `DiffCommit`
		- `test_draft_diff_commit`, which does the base API tests for the
		`DraftDiffCommitResource`
		- `test_diff_commit`, which does the base API tests for the
		`DiffCommitResource`

		Unit tests pass.

~		Did the following manual testing:
~		- Created a review request via `rbt post`. It was automatically
~		created with history. (There was one commit under the draft).
~		- Able to read the commit list and single commit via the API.
~		- Able to filter the `FileDiff` list resource.
~		- The `FileDiff` item resource was not filtered.
~		- Able to publish, discard, and delete the review request.
~		- Able to use all the public API endoint functionality of the
	~	Did the following manual testing:
	~
	~	Created a review request via `rbt post`. It was automatically created with history. (There was one commit under the draft).
	~	Able to read the commit list and single commit via the API.
	~	Able to filter the `FileDiff` list resource.
	~	The `FileDiff` item resource was not filtered.
	~	Able to publish, discard, and delete the review request.
	~	Able to use all the public API endoint functionality of the published RR anonymously
-		published RR anonymously