Add support for showing diffs of file attachments in the diff viewer.
Review Request #4481 — Created Aug. 23, 2013 and submitted
Add support for showing diffs of file attachments in the diff viewer. Review UIs can now opt in to providing diffing functionality for file attachments. A ReviewUI subclass that has supports_diffing=True will have the whole width of the diff table to render in. It will be given both the new file's fileAttachmentID, and the original's (as diffAgainstFileAttachmentID). FileAttachmentComment can now specify a FileAttachment that's being diffed against. The ID for this can be set when creating a new comment in the webapi. FileAttachmentReviewUI will properly filter the comments when passing down to the JavaScript to ensure only the comments matching that diff range (or, if not using a diff, comments that are on a single FileAttachment) are passed down. It's up to the review UI to decide how to render the diff.
Tested with the image diff review UI change in /r/4493/.
| Description | From | Last Updated |
|---|---|---|
|
It seems a little weird that we ask if orig_review_ui.supports_diffing but then use the modified_review_ui to do the diff. I … |
 david
|
|
|
I think it would make more sense for this to be an else block from the previous conditional. |
david
|
|
|
Col: 80 E501 line too long (116 > 79 characters) |
 reviewbot
|
|
|
Col: 80 E501 line too long (80 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (116 > 79 characters) |
reviewbot
|
-
This is a review from Review Bot. Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/webapi/resources.py reviewboard/reviews/models.py reviewboard/reviews/evolutions/__init__.py reviewboard/webapi/tests.py reviewboard/reviews/evolutions/file_attachment_comment_diff_id.py reviewboard/reviews/ui/base.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/templates/diffviewer/diff_file_fragment.html reviewboard/static/rb/js/models/fileAttachmentCommentBlockModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
-
-
reviewboard/reviews/views.py (Diff revision 1) It seems a little weird that we ask if orig_review_ui.supports_diffing but then use the modified_review_ui to do the diff. I know they should be the same class, but perhaps change the check to be modified_review_ui.supports_diffing ?
-
reviewboard/reviews/views.py (Diff revision 1) I think it would make more sense for this to be an else block from the previous conditional.
Change Summary:
* Fixed an issue posting comments on file attachments without a diff UI. * Added a "diff-review-ui" class to the container element for review UIs in the diff viewer. * Code structure cleanups requested during review.
Testing Done: |
|
||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+265 -55) |
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/webapi/resources.py reviewboard/reviews/models.py reviewboard/reviews/evolutions/__init__.py reviewboard/webapi/tests.py reviewboard/reviews/evolutions/file_attachment_comment_diff_id.py reviewboard/reviews/ui/base.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/templates/diffviewer/diff_file_fragment.html reviewboard/static/rb/js/models/fileAttachmentCommentBlockModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js -
-
This is a review from Review Bot. Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/webapi/resources.py reviewboard/reviews/models.py reviewboard/reviews/evolutions/__init__.py reviewboard/webapi/tests.py reviewboard/reviews/evolutions/file_attachment_comment_diff_id.py reviewboard/reviews/ui/base.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/templates/diffviewer/diff_file_fragment.html reviewboard/static/rb/js/models/fileAttachmentCommentBlockModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/webapi/resources.py reviewboard/reviews/models.py reviewboard/reviews/evolutions/__init__.py reviewboard/webapi/tests.py reviewboard/reviews/evolutions/file_attachment_comment_diff_id.py reviewboard/reviews/ui/base.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/templates/diffviewer/diff_file_fragment.html reviewboard/static/rb/js/models/fileAttachmentCommentBlockModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js