Summary

Merge branch 'ucosp/dkus/dnd-inline-images' into release-3.0.x

Review Request #8176 — Created May 23, 2016 and submitted July 12, 2016, 4:20 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This review request is just to sanity-check the merge of the new DnD features into release-3.0.x

Testing Done

Issues

Description	From	Last Updated
Should be in standard one-line summary, multi-line description format.	chipx86	July 8, 2016, 3:06 p.m.
Missing args and *kwargs docs.	chipx86	July 8, 2016, 3:07 p.m.
Should have , optional.	chipx86	July 8, 2016, 3:07 p.m.
, optional	chipx86	July 8, 2016, 3:07 p.m.
Three of these are common between this and the dictionary above. We can build these before the file_obj condition, and …	chipx86	July 8, 2016, 3:08 p.m.
We're saving twice in the case of file_obj, since we're doing file.save(..., save=True) above, and save() here. We probably just …	chipx86	July 8, 2016, 3:13 p.m.
The mimetype setting can be one line. Probably the filename too.	chipx86	July 8, 2016, 3:14 p.m.
unicode.	chipx86	July 8, 2016, 3:14 p.m.
unicode	chipx86	July 8, 2016, 3:14 p.m.
Let's use :mimetype: for these.	chipx86	July 8, 2016, 3:45 p.m.
unicode.	chipx86	July 8, 2016, 3:14 p.m.
unicode	chipx86	July 8, 2016, 3:14 p.m.
, optional	chipx86	July 8, 2016, 3:16 p.m.
We can skip the first lookup by doing user__username=username in the second.	chipx86	July 8, 2016, 3:16 p.m.
Can we say "Drop to add an image" ?	chipx86	July 8, 2016, 3:17 p.m.
I think the type is File.	chipx86	July 8, 2016, 3:17 p.m.
File	chipx86	July 8, 2016, 3:17 p.m.
This is listed as callback in the defaults.	chipx86	July 8, 2016, 3:17 p.m.
Should use /** and Args	chipx86	July 8, 2016, 3:18 p.m.
Missing Args and Returns.	chipx86	July 8, 2016, 3:21 p.m.
Let's use :py:class: for the classes.	chipx86	July 8, 2016, 3:22 p.m.
Missing period.	chipx86	July 8, 2016, 3:22 p.m.
Same comments as above.	chipx86	July 8, 2016, 3:24 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/file_attachment.py
    reviewboard/staticbundles.py
    reviewboard/webapi/tests/test_user_file_attachment.py
    reviewboard/webapi/resources/base_file_attachment.py
    reviewboard/attachments/models.py
    reviewboard/attachments/evolutions/file_attachment_uuid.py
    reviewboard/webapi/resources/base_review_request_file_attachment.py
    reviewboard/attachments/forms.py
    reviewboard/webapi/resources/user_file_attachment.py
    reviewboard/testing/testcase.py
    reviewboard/webapi/resources/draft_file_attachment.py
    reviewboard/webapi/resources/diff_file_attachment.py
    reviewboard/attachments/mimetypes.py
    reviewboard/urls.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/tests/urls.py
    reviewboard/attachments/evolutions/__init__.py
    reviewboard/attachments/evolutions/file_attachment_ownership.py
    reviewboard/webapi/tests/mimetypes.py
    reviewboard/reviews/context.py
    reviewboard/attachments/tests.py
    reviewboard/attachments/views.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/tests/userFileAttachmentModelTests.js
    reviewboard/static/rb/css/ui/dnd-uploader.less
    reviewboard/static/rb/js/resources/models/userFileAttachmentModel.es6.js
    reviewboard/static/rb/js/ui/views/textEditorView.es6.js
    reviewboard/static/rb/js/views/dndUploaderView.es6.js
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/views/reviewReplyEditorView.js
    reviewboard/templates/reviews/review_request_box.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/ui/views/tests/textEditorViewTests.js
    reviewboard/static/rb/css/common.less
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/js/pages/views/reviewablePageView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/file_attachment.py
    reviewboard/staticbundles.py
    reviewboard/webapi/tests/test_user_file_attachment.py
    reviewboard/webapi/resources/base_file_attachment.py
    reviewboard/attachments/models.py
    reviewboard/attachments/evolutions/file_attachment_uuid.py
    reviewboard/webapi/resources/base_review_request_file_attachment.py
    reviewboard/attachments/forms.py
    reviewboard/webapi/resources/user_file_attachment.py
    reviewboard/testing/testcase.py
    reviewboard/webapi/resources/draft_file_attachment.py
    reviewboard/webapi/resources/diff_file_attachment.py
    reviewboard/attachments/mimetypes.py
    reviewboard/urls.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/tests/urls.py
    reviewboard/attachments/evolutions/__init__.py
    reviewboard/attachments/evolutions/file_attachment_ownership.py
    reviewboard/webapi/tests/mimetypes.py
    reviewboard/reviews/context.py
    reviewboard/attachments/tests.py
    reviewboard/attachments/views.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/tests/userFileAttachmentModelTests.js
    reviewboard/static/rb/css/ui/dnd-uploader.less
    reviewboard/static/rb/js/resources/models/userFileAttachmentModel.es6.js
    reviewboard/static/rb/js/ui/views/textEditorView.es6.js
    reviewboard/static/rb/js/views/dndUploaderView.es6.js
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/views/reviewReplyEditorView.js
    reviewboard/templates/reviews/review_request_box.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/ui/views/tests/textEditorViewTests.js
    reviewboard/static/rb/css/common.less
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/js/pages/views/reviewablePageView.js

Some things I noticed in this pass. Most of it documentation-related, and a couple very small code cleanups.

reviewboard/attachments/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should be in standard one-line summary, multi-line description format.
```
reviewboard/attachments/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing *args and **kwargs docs.
```
reviewboard/attachments/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should have , optional.
```
reviewboard/attachments/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
, optional
```

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

The issue has been resolved. Show all issues

Three of these are common between this and the dictionary above. We can build these before the file_obj condition, and update them in here.

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

The issue has been resolved. Show all issues

We're saving twice in the case of file_obj, since we're doing file.save(..., save=True) above, and save() here. We probably just want to remove file_attachment.save() and use FileAttachment.objects.create() in the else.

reviewboard/attachments/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
The mimetype setting can be one line. Probably the filename too.
```
reviewboard/attachments/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
unicode.
```
reviewboard/attachments/mimetypes.py (Diff revision 1)
The issue has been resolved. Show all issues
```
unicode
```

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

The issue has been resolved. Show all issues

Let's use :mimetype: for these.

david July 8, 2016, 3:24 p.m.

I'm hesitant to use :mimetype: here because "text/text/application/pdf" is very much not a mimetype, which is exactly the problem.

chipx86 July 8, 2016, 3:28 p.m.

Sure. How about for application/octet-stream, and just double-backtick the text/text/application/pdf.

reviewboard/attachments/mimetypes.py (Diff revision 1)
The issue has been resolved. Show all issues
```
unicode.
```
reviewboard/attachments/views.py (Diff revision 1)
The issue has been resolved. Show all issues
```
unicode
```
reviewboard/attachments/views.py (Diff revision 1)
The issue has been resolved. Show all issues
```
, optional
```

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

The issue has been resolved. Show all issues

We can skip the first lookup by doing user__username=username in the second.

reviewboard/static/rb/js/ui/views/textEditorView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we say "Drop to add an image" ?
```
reviewboard/static/rb/js/ui/views/textEditorView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
I think the type is File.
```
reviewboard/static/rb/js/ui/views/textEditorView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
File
```
reviewboard/static/rb/js/views/dndUploaderView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
This is listed as callback in the defaults.
```

reviewboard/static/rb/js/views/reviewRequestEditorView.js (Diff revision 1)

The issue has been resolved. Show all issues

Should use /** and Args

reviewboard/testing/testcase.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing Args and Returns.
```
reviewboard/testing/testcase.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Let's use :py:class: for the classes.
```
reviewboard/testing/testcase.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing period.
```
reviewboard/testing/testcase.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same comments as above.
```

Diff:

Revision 2 (+2067 -514)

Show changes

	reviewboard/staticbundles.py
	reviewboard/urls.py
	reviewboard/attachments/forms.py
	reviewboard/attachments/mimetypes.py
	reviewboard/attachments/models.py
	reviewboard/attachments/tests.py
	reviewboard/attachments/views.py
	reviewboard/attachments/evolutions/__init__.py
	28 more

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/file_attachment.py
    reviewboard/staticbundles.py
    reviewboard/webapi/tests/test_user_file_attachment.py
    reviewboard/webapi/resources/base_file_attachment.py
    reviewboard/attachments/models.py
    reviewboard/attachments/evolutions/file_attachment_uuid.py
    reviewboard/webapi/resources/base_review_request_file_attachment.py
    reviewboard/attachments/forms.py
    reviewboard/webapi/resources/user_file_attachment.py
    reviewboard/testing/testcase.py
    reviewboard/webapi/resources/draft_file_attachment.py
    reviewboard/webapi/resources/diff_file_attachment.py
    reviewboard/attachments/mimetypes.py
    reviewboard/urls.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/tests/urls.py
    reviewboard/attachments/evolutions/__init__.py
    reviewboard/attachments/evolutions/file_attachment_ownership.py
    reviewboard/webapi/tests/mimetypes.py
    reviewboard/reviews/context.py
    reviewboard/attachments/tests.py
    reviewboard/attachments/views.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/tests/userFileAttachmentModelTests.js
    reviewboard/static/rb/css/ui/dnd-uploader.less
    reviewboard/static/rb/js/resources/models/userFileAttachmentModel.es6.js
    reviewboard/static/rb/js/ui/views/textEditorView.es6.js
    reviewboard/static/rb/js/views/dndUploaderView.es6.js
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/views/reviewReplyEditorView.js
    reviewboard/templates/reviews/review_request_box.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/ui/views/tests/textEditorViewTests.js
    reviewboard/static/rb/css/common.less
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/js/pages/views/reviewablePageView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/file_attachment.py
    reviewboard/staticbundles.py
    reviewboard/webapi/tests/test_user_file_attachment.py
    reviewboard/webapi/resources/base_file_attachment.py
    reviewboard/attachments/models.py
    reviewboard/attachments/evolutions/file_attachment_uuid.py
    reviewboard/webapi/resources/base_review_request_file_attachment.py
    reviewboard/attachments/forms.py
    reviewboard/webapi/resources/user_file_attachment.py
    reviewboard/testing/testcase.py
    reviewboard/webapi/resources/draft_file_attachment.py
    reviewboard/webapi/resources/diff_file_attachment.py
    reviewboard/attachments/mimetypes.py
    reviewboard/urls.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/tests/urls.py
    reviewboard/attachments/evolutions/__init__.py
    reviewboard/attachments/evolutions/file_attachment_ownership.py
    reviewboard/webapi/tests/mimetypes.py
    reviewboard/reviews/context.py
    reviewboard/attachments/tests.py
    reviewboard/attachments/views.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/tests/userFileAttachmentModelTests.js
    reviewboard/static/rb/css/ui/dnd-uploader.less
    reviewboard/static/rb/js/resources/models/userFileAttachmentModel.es6.js
    reviewboard/static/rb/js/ui/views/textEditorView.es6.js
    reviewboard/static/rb/js/views/dndUploaderView.es6.js
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/views/reviewReplyEditorView.js
    reviewboard/templates/reviews/review_request_box.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/ui/views/tests/textEditorViewTests.js
    reviewboard/static/rb/css/common.less
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/js/pages/views/reviewablePageView.js

Diff:

Revision 3 (+2067 -514)

Show changes

	reviewboard/staticbundles.py
	reviewboard/urls.py
	reviewboard/attachments/forms.py
	reviewboard/attachments/mimetypes.py
	reviewboard/attachments/models.py
	reviewboard/attachments/tests.py
	reviewboard/attachments/views.py
	reviewboard/attachments/evolutions/__init__.py
	28 more

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/file_attachment.py
    reviewboard/staticbundles.py
    reviewboard/webapi/tests/test_user_file_attachment.py
    reviewboard/webapi/resources/base_file_attachment.py
    reviewboard/attachments/models.py
    reviewboard/attachments/evolutions/file_attachment_uuid.py
    reviewboard/webapi/resources/base_review_request_file_attachment.py
    reviewboard/attachments/forms.py
    reviewboard/webapi/resources/user_file_attachment.py
    reviewboard/testing/testcase.py
    reviewboard/webapi/resources/draft_file_attachment.py
    reviewboard/webapi/resources/diff_file_attachment.py
    reviewboard/attachments/mimetypes.py
    reviewboard/urls.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/tests/urls.py
    reviewboard/attachments/evolutions/__init__.py
    reviewboard/attachments/evolutions/file_attachment_ownership.py
    reviewboard/webapi/tests/mimetypes.py
    reviewboard/reviews/context.py
    reviewboard/attachments/tests.py
    reviewboard/attachments/views.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/tests/userFileAttachmentModelTests.js
    reviewboard/static/rb/css/ui/dnd-uploader.less
    reviewboard/static/rb/js/resources/models/userFileAttachmentModel.es6.js
    reviewboard/static/rb/js/ui/views/textEditorView.es6.js
    reviewboard/static/rb/js/views/dndUploaderView.es6.js
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/views/reviewReplyEditorView.js
    reviewboard/templates/reviews/review_request_box.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/ui/views/tests/textEditorViewTests.js
    reviewboard/static/rb/css/common.less
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/js/pages/views/reviewablePageView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/file_attachment.py
    reviewboard/staticbundles.py
    reviewboard/webapi/tests/test_user_file_attachment.py
    reviewboard/webapi/resources/base_file_attachment.py
    reviewboard/attachments/models.py
    reviewboard/attachments/evolutions/file_attachment_uuid.py
    reviewboard/webapi/resources/base_review_request_file_attachment.py
    reviewboard/attachments/forms.py
    reviewboard/webapi/resources/user_file_attachment.py
    reviewboard/testing/testcase.py
    reviewboard/webapi/resources/draft_file_attachment.py
    reviewboard/webapi/resources/diff_file_attachment.py
    reviewboard/attachments/mimetypes.py
    reviewboard/urls.py
    reviewboard/webapi/resources/user.py
    reviewboard/webapi/tests/urls.py
    reviewboard/attachments/evolutions/__init__.py
    reviewboard/attachments/evolutions/file_attachment_ownership.py
    reviewboard/webapi/tests/mimetypes.py
    reviewboard/reviews/context.py
    reviewboard/attachments/tests.py
    reviewboard/attachments/views.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/tests/userFileAttachmentModelTests.js
    reviewboard/static/rb/css/ui/dnd-uploader.less
    reviewboard/static/rb/js/resources/models/userFileAttachmentModel.es6.js
    reviewboard/static/rb/js/ui/views/textEditorView.es6.js
    reviewboard/static/rb/js/views/dndUploaderView.es6.js
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/views/reviewReplyEditorView.js
    reviewboard/templates/reviews/review_request_box.html
    reviewboard/static/rb/css/pages/reviews.less
    reviewboard/static/rb/js/ui/views/tests/textEditorViewTests.js
    reviewboard/static/rb/css/common.less
    reviewboard/static/rb/css/defs.less
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/js/pages/views/reviewablePageView.js

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to ucosp/dkus/dnd-inline-images (91be638)