Summary

Add API for fetching and setting diff file attachments.

Review Request #4647 — Created Sept. 25, 2013 and discarded Oct. 31, 2013, 12:06 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Add API for fetching and setting diff file attachments.

There's now a /api/repositories/<id>/diff-file-attachments/ resource
that contains the known, registered diff file attachments tied to a
repository. Nothing on this tree can be modified, only retrieved.

In the future, some file attachments will be automatically associated.
Those that cannot can be modified by issuing a PUT on a draft filediff
resource and passing a file for the file attachment representing the
destination/modified file. Right now, this is allowed for all filediffs
for binary files, but later it will be restricted.

Testing Done

Unit tests pass.

Issues

Description	From	Last Updated
testing	VE venkuvenkius	Oct. 11, 2013, 4:18 a.m.
'FileDiff' imported but unused	reviewbot	Sept. 25, 2013, 12:47 a.m.
Using .exists() will be faster and less code.	david	Oct. 11, 2013, 5:06 p.m.
local variable 'attachment' is assigned to but never used	reviewbot	Sept. 25, 2013, 12:47 a.m.
'INVALID_FORM_DATA' imported but unused	reviewbot	Sept. 25, 2013, 12:47 a.m.
'scmtools' imported but unused	reviewbot	Sept. 25, 2013, 12:47 a.m.
'FileAttachment' imported but unused	reviewbot	Sept. 25, 2013, 12:47 a.m.
'DiffSet' imported but unused	reviewbot	Sept. 25, 2013, 12:47 a.m.
'FileDiff' imported but unused	reviewbot	Sept. 25, 2013, 12:47 a.m.
'FileDiff' imported but unused	reviewbot	Sept. 25, 2013, 12:47 a.m.
undefined name 'filediff'	reviewbot	Sept. 25, 2013, 12:47 a.m.

This is a review from Review Bot.
Tool: PEP8 Style Checker
Processed Files:
reviewboard/testing/testcase.py
reviewboard/webapi/tests/urls.py
reviewboard/webapi/resources/init.py
reviewboard/attachments/forms.py
reviewboard/attachments/managers.py
reviewboard/webapi/resources/filediff.py
reviewboard/webapi/tests/base.py
reviewboard/webapi/tests/test_diff_file_attachment.py
reviewboard/webapi/tests/test_draft_filediff.py
reviewboard/webapi/resources/diff_file_attachment.py
reviewboard/webapi/tests/mimetypes.py
reviewboard/webapi/resources/draft_filediff.py
reviewboard/webapi/resources/repository.py
Ignored Files:
docs/manual/webapi/2.0/resources/index.txt
docs/manual/webapi/2.0/resources/diff-file-attachment.txt
docs/manual/fixtures/initial_data.json
docs/manual/webapi/2.0/resources/diff-file-attachment-list.txt

Fix it!

This is a review from Review Bot.
Tool: Pyflakes
Processed Files:
reviewboard/testing/testcase.py
reviewboard/webapi/tests/urls.py
reviewboard/webapi/resources/init.py
reviewboard/attachments/forms.py
reviewboard/attachments/managers.py
reviewboard/webapi/resources/filediff.py
reviewboard/webapi/tests/base.py
reviewboard/webapi/tests/test_diff_file_attachment.py
reviewboard/webapi/tests/test_draft_filediff.py
reviewboard/webapi/resources/diff_file_attachment.py
reviewboard/webapi/tests/mimetypes.py
reviewboard/webapi/resources/draft_filediff.py
reviewboard/webapi/resources/repository.py
Ignored Files:
docs/manual/webapi/2.0/resources/index.txt
docs/manual/webapi/2.0/resources/diff-file-attachment.txt
docs/manual/fixtures/initial_data.json
docs/manual/webapi/2.0/resources/diff-file-attachment-list.txt

reviewboard/webapi/resources/diff_file_attachment.py (Diff revision 1)
An issue was opened. Show all issues
```
'FileDiff' imported but unused
```
reviewboard/webapi/resources/draft_filediff.py (Diff revision 1)
An issue was opened. Show all issues
```
local variable 'attachment' is assigned to but never used
```
reviewboard/webapi/tests/test_diff_file_attachment.py (Diff revision 1)
An issue was opened. Show all issues
```
'INVALID_FORM_DATA' imported but unused
```
reviewboard/webapi/tests/test_diff_file_attachment.py (Diff revision 1)
An issue was opened. Show all issues
```
'scmtools' imported but unused
```
reviewboard/webapi/tests/test_diff_file_attachment.py (Diff revision 1)
An issue was opened. Show all issues
```
'FileAttachment' imported but unused
```
reviewboard/webapi/tests/test_diff_file_attachment.py (Diff revision 1)
An issue was opened. Show all issues
```
'DiffSet' imported but unused
```
reviewboard/webapi/tests/test_diff_file_attachment.py (Diff revision 1)
An issue was opened. Show all issues
```
'FileDiff' imported but unused
```
reviewboard/webapi/tests/test_draft_filediff.py (Diff revision 1)
An issue was opened. Show all issues
```
'FileDiff' imported but unused
```
reviewboard/webapi/tests/urls.py (Diff revision 1)
An issue was opened. Show all issues
```
undefined name 'filediff'
```

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

The issue has been dropped. Show all issues

Using .exists() will be faster and less code.

chipx86 Sept. 26, 2013, 4:46 p.m.

get_for_filediff doesn't return a QuerySet. It's a get() call, so I can't take advantage of exists() without creating another method just for checking.

david Sept. 27, 2013, 1:49 a.m.
```
OK.
```

Fix it!

reviewboard/attachments/managers.py (Diff revision 1)

An issue was opened. Show all issues

testing

david Oct. 11, 2013, 12:04 p.m.

This is a production server, and by doing your test here, you just spammed a bunch of people.

If you want to test, you can use demo.reviewboard.org (username 'guest' password 'guest')

Status: Discarded

Change Summary:

Replaced by /r/4860/