diff --git a/reviewboard/admin/urls.py b/reviewboard/admin/urls.py
index 8e956b95cda674bd5caa7808f4c26d63532d08f0..e3a865c0e0f667209633c47dc54958c2e52c6b33 100644
--- a/reviewboard/admin/urls.py
+++ b/reviewboard/admin/urls.py
@@ -36,6 +36,7 @@ from reviewboard.admin import forms, views
NEWS_FEED = 'https://www.reviewboard.org/news/feed/'
+
urlpatterns = [
url(r'^$', views.dashboard, name='admin-dashboard'),
diff --git a/reviewboard/dependencies.py b/reviewboard/dependencies.py
index cfff6d04037b9cda86e72002d2e70244888830dc..404089deb05b4d0c2cb4c79c1fdb27fa187d7d54 100644
--- a/reviewboard/dependencies.py
+++ b/reviewboard/dependencies.py
@@ -106,6 +106,7 @@ package_dependencies = {
'python-memcached': '',
'pytz': '>=2015.2',
'Whoosh': '>=2.6',
+ 'lxml': '>=4.4.1'
}
#: Dependencies only specified during the packaging process.
diff --git a/reviewboard/deprecation.py b/reviewboard/deprecation.py
index 13f554ce60763409c93077cec247ab71726fa525..1f69347b1fc0c63423423737fdff78c2a84dd5c4 100644
--- a/reviewboard/deprecation.py
+++ b/reviewboard/deprecation.py
@@ -39,5 +39,16 @@ class RemovedInReviewBoard50Warning(BaseRemovedInReviewBoardVersionWarning):
"""
+class RemovedInReviewBoard60Warning(BaseRemovedInReviewBoardVersionWarning):
+ """Deprecations for features removed in Review Board 6.0.
+
+ Note that this class will itself be removed in Review Board 6.0. If you
+ need to check against Review Board deprecation warnings, please see
+ :py:class:`BaseRemovedInReviewBoardVersionWarning`. Alternatively, you
+ can use the alias for this class,
+ :py:data:`RemovedInNextReviewBoardVersionWarning`.
+ """
+
+
#: An alias for the next release of Djblets where features would be removed.
RemovedInNextReviewBoardVersionWarning = RemovedInReviewBoard50Warning
diff --git a/reviewboard/reviews/tests/test_file_attachment_review_ui.py b/reviewboard/reviews/tests/test_file_attachment_review_ui.py
index ebdf44083bee4ff0d8428da84c0462a2165c0041..51f2bfee49da2a31cce71a3e7a45bcb48f968324 100644
--- a/reviewboard/reviews/tests/test_file_attachment_review_ui.py
+++ b/reviewboard/reviews/tests/test_file_attachment_review_ui.py
@@ -1,9 +1,11 @@
"""Unit tests for reviewboard.reviews.ui.base.FileAttachmentReviewUI."""
from __future__ import unicode_literals
+from django.core.urlresolvers import reverse
+from django.conf.urls import url
+from django.utils.text import slugify
from djblets.testing.decorators import add_fixtures
from kgb import SpyAgency
-
from reviewboard.reviews.ui.base import (FileAttachmentReviewUI,
register_ui,
unregister_ui)
@@ -17,6 +19,7 @@ class MyReviewUI(FileAttachmentReviewUI):
supports_diffing = True
+# noinspection PyTypeChecker
class FileAttachmentReviewUITests(SpyAgency, TestCase):
"""Unit tests for reviewboard.reviews.ui.base.FileAttachmentReviewUI."""
@@ -458,3 +461,80 @@ class FileAttachmentReviewUITests(SpyAgency, TestCase):
'username': 'dopey',
},
})
+
+ def test_register_review_ui_id_set(self):
+ """Testing register_ui with a review_ui_id set.
+ """
+ class ReviewUIWithId(FileAttachmentReviewUI):
+ supported_mimetypes = ['application/reviewid']
+ review_ui_id = 'test'
+
+ self.spy_on(ReviewUIWithId.__init__,
+ owner=ReviewUIWithId)
+ register_ui(ReviewUIWithId)
+
+ try:
+ attachment = self.create_file_attachment(
+ self.review_request,
+ mimetype='application/reviewid',
+ )
+ review_ui = attachment.review_ui
+ self.assertIsInstance(review_ui, ReviewUIWithId)
+ finally:
+ unregister_ui(ReviewUIWithId)
+
+ def test_register_review_ui_no_id_set(self):
+ """Testing register_ui with no review_ui_id set.
+ """
+ class ReviewUIWithoutId(FileAttachmentReviewUI):
+ supported_mimetypes = ['application/noreviewid']
+
+ self.spy_on(ReviewUIWithoutId.__init__,
+ owner=ReviewUIWithoutId)
+ register_ui(ReviewUIWithoutId)
+
+ try:
+ attachment = self.create_file_attachment(
+ self.review_request,
+ mimetype='application/noreviewid',
+ )
+ review_ui = attachment.review_ui
+ expected_review_ui_id = slugify(
+ '%s.%s' % (review_ui.__module__, "ReviewUIWithoutId"))
+ self.assertEqual(review_ui.review_ui_id, expected_review_ui_id)
+ self.assertIsInstance(review_ui, ReviewUIWithoutId)
+ finally:
+ unregister_ui(ReviewUIWithoutId)
+
+ def test_register_review_ui_custom_url(self):
+ """ Testing register_ui with custom URLs.
+ """
+
+ def dummy_view():
+ pass
+
+ class URLPatternReviewUI(FileAttachmentReviewUI):
+ supported_mimetypes = ['application/urlpattern']
+ review_ui_id = 'testid'
+ url_patterns = [
+ url(r'^testpattern/$', dummy_view, name='testpattern')
+ ]
+
+ self.spy_on(URLPatternReviewUI.__init__,
+ owner=URLPatternReviewUI)
+ register_ui(URLPatternReviewUI)
+
+ attachment = self.create_file_attachment(
+ self.review_request,
+ mimetype='application/urlpattern',
+ )
+ review_ui = attachment.review_ui
+
+ test_url = reverse('testpattern', args=(self.review_request.id,
+ attachment.id))
+ expected_url = '/r/%d/file/%d/%s/testpattern/' % \
+ (self.review_request.id, attachment.id,
+ review_ui.review_ui_id)
+
+ self.assertEqual(test_url, expected_url)
+ unregister_ui(URLPatternReviewUI)
diff --git a/reviewboard/reviews/tests/test_xml_review_ui.py b/reviewboard/reviews/tests/test_xml_review_ui.py
new file mode 100644
index 0000000000000000000000000000000000000000..979dbe1ec4f08f1987027ce2fb9a7e419a3ee3c5
--- /dev/null
+++ b/reviewboard/reviews/tests/test_xml_review_ui.py
@@ -0,0 +1,454 @@
+# -*- coding: utf-8 -*-
+
+from reviewboard.reviews.ui.xmlui import (
+ get_xml_declaration,
+ parse_xml_to_tree,
+ get_attributes_str,
+ format_empty_element_tag,
+ format_element_with_text,
+ format_element_with_children,
+ prettify_xml,
+ render_xml_as_html,
+ is_comment_element,
+ get_encoding_from_declaration,
+ get_siblings_before_root,
+ get_siblings_after_root,
+ parse_text_from_element_source)
+
+from reviewboard.testing import TestCase
+
+
+def get_encoding_string(encoding):
+ return " encoding='%s'" % encoding
+
+
+class XmlUiTests(TestCase):
+ """Unit tests for reviewboard.reviews.ui.xmlui."""
+
+ def parse_tree_and_validate(self, encoding=None, content=''):
+ if encoding is None:
+ encoding_string = get_encoding_string('ASCII')
+ else:
+ encoding_string = get_encoding_string(encoding)
+
+ xml_contents = "\n%s" % (
+ encoding_string, content)
+
+ root = parse_xml_to_tree(xml_contents)
+ parsed_string = format_element_with_text(root, 0)
+ expected_string = u'\n %s\n\n' % (content)
+
+ self.assertEqual(parsed_string, expected_string)
+
+ def test_get_xml_declaration_no_declaration(self):
+ """Testing get_xml_declaration with no declaration tag in the xml"""
+ xml_contents = ''
+
+ declaration = get_xml_declaration(xml_contents)
+ self.assertEqual(declaration, '')
+
+ def test_get_xml_declaration_with_declaration(self):
+ """Testing get_xml_declaration with a declaration tag in the xml"""
+ expected_declaration = ''
+ xml_contents = expected_declaration + ''
+
+ declaration = get_xml_declaration(xml_contents)
+ self.assertEqual(declaration, expected_declaration)
+
+ def test_get_xml_declaration_with_declaration_on_separate_line(self):
+ """Testing get_xml_declaration with a declaration tag in the xml
+ that is on a different line from the actual contents
+ """
+ expected_declaration = ''
+ xml_contents = expected_declaration + '\n'
+
+ declaration = get_xml_declaration(xml_contents)
+ self.assertEqual(declaration, expected_declaration)
+
+ def test_parse_xml_to_tree_default_encoding(self):
+ """Testing parse_xml_to_tree with no encoding specified"""
+ self.parse_tree_and_validate(content='test parsing default')
+
+ def test_parse_xml_to_tree_utf8_encoding(self):
+ """Testing parse_xml_to_tree with utf8 encoding specified"""
+ self.parse_tree_and_validate('UTF-8', u'¢')
+
+ def test_parse_xml_to_tree_utf16_encoding(self):
+ """Testing parse_xml_to_tree with utf16 encoding specified"""
+ self.parse_tree_and_validate('UTF-16', u'€')
+
+ def test_get_encoding_from_declaration_no_encoding(self):
+ declaration = ''
+
+ encoding = get_encoding_from_declaration(declaration)
+
+ self.assertEqual(encoding, 'ASCII')
+
+ def test_get_encoding_from_declaration_utf16(self):
+ declaration = ''
+
+ encoding = get_encoding_from_declaration(declaration)
+
+ self.assertEqual(encoding, 'UTF-16')
+
+ def test_get_attributes_str_no_attributes(self):
+ """Testing get_attributes_str when an element has no attributes"""
+ root = parse_xml_to_tree('')
+
+ attributes = get_attributes_str(root)
+
+ self.assertEqual(attributes, '')
+
+ def test_get_attributes_str_with_one_attribute(self):
+ """Testing get_attributes_str when an element has one attribute"""
+ root = parse_xml_to_tree('')
+
+ attributes = get_attributes_str(root)
+
+ self.assertEqual(attributes, 'test_key="test_value"')
+
+ def test_get_attributes_str_with_single_quotes(self):
+ """Testing get_attributes_str when single quotes are used"""
+ root = parse_xml_to_tree('')
+
+ attributes = get_attributes_str(root)
+
+ self.assertEqual(attributes, 'test_key="test_value"')
+
+ def test_get_attributes_str_with_multiple_attributes(self):
+ """Testing get_attributes_str when an element has attributes"""
+ root = parse_xml_to_tree('')
+
+ attributes = get_attributes_str(root)
+
+ self.assertEqual(attributes, 'test_key="value" int_key="999"')
+
+ def test_format_empty_element_tag_no_attributes(self):
+ """Testing format_empty_element_tag with no attributes"""
+ root = parse_xml_to_tree('')
+
+ formatted_string = format_empty_element_tag(root, 2)
+
+ self.assertEqual(formatted_string, ' \n')
+
+ def test_format_empty_element_tag_one_attribute(self):
+ """Testing format_empty_element_tag with one attribute"""
+ root = parse_xml_to_tree('')
+
+ formatted_string = format_empty_element_tag(root, 2)
+
+ self.assertEqual(formatted_string, ' \n')
+
+ def test_format_empty_element_tag_no_indent(self):
+ """Testing format_empty_element_tag with no indentation"""
+ root = parse_xml_to_tree('')
+
+ formatted_string = format_empty_element_tag(root, 0)
+
+ self.assertEqual(formatted_string, '\n')
+
+ def test_format_element_with_text_no_attributes(self):
+ """Testing format_element_with_text with no attributes"""
+ root = parse_xml_to_tree('test text')
+
+ formatted_string = format_element_with_text(root, 2)
+
+ self.assertEqual(formatted_string, """\
+
+ test text
+
+""")
+
+ def test_format_element_with_text_same_line(self):
+ """Testing format_element_with_text with content on the same line"""
+ root = parse_xml_to_tree('test text')
+
+ formatted_string = format_element_with_text(root, 2, True)
+
+ self.assertEqual(formatted_string, ' test text\n')
+
+ def test_format_element_with_multiline_text(self):
+ """Testing format_element_with_text with multiline text"""
+ root = parse_xml_to_tree('test\ntext')
+
+ formatted_string = format_element_with_text(root, 2)
+
+ self.assertEqual(formatted_string, """\
+
+ test
+ text
+
+""")
+
+ def test_format_element_with_text_prefixed_with_whitespace(self):
+ """Testing format_element_with_text beginning with whitespace"""
+ root = parse_xml_to_tree(' test text')
+
+ formatted_string = format_element_with_text(root, 2)
+
+ self.assertEqual(formatted_string, """\
+
+ test text
+
+""")
+
+ def test_format_element_with_text_with_cdata(self):
+ """Testing format_element_with_text when text includes data"""
+ xml_contents = 'cdataTest CDATA]]>'
+ root = parse_xml_to_tree(xml_contents)
+
+ formatted_string = format_element_with_text(root, 2)
+
+ self.assertEqual(formatted_string, """\
+
+ cdataTest CDATA]]>
+
+""")
+
+ def test_format_element_with_text_one_attribute(self):
+ """Testing format_element_with_text with one attribute"""
+ root = parse_xml_to_tree('test text')
+
+ formatted_string = format_element_with_text(root, 2)
+
+ self.assertEqual(formatted_string, """\
+
+ test text
+
+""")
+
+ def test_format_element_with_children_no_attributes(self):
+ """Testing format_element_with_children with no attributes"""
+ root = parse_xml_to_tree('')
+
+ formatted_string = format_element_with_children(root, 2)
+
+ self.assertEqual(formatted_string, """\
+
+
+
+""")
+
+ def test_format_element_with_children_one_attribute(self):
+ """Testing format_element_with_children with one attribute"""
+ root = parse_xml_to_tree('')
+
+ formatted_string = format_element_with_children(root, 2)
+
+ self.assertEqual(formatted_string, """\
+
+
+
+""")
+
+ def test_get_siblings_before_root_no_siblings(self):
+ """Testing get_siblings_before_root with no nodes before the root"""
+ root = parse_xml_to_tree('')
+
+ before_root = get_siblings_before_root(root)
+
+ self.assertEqual(before_root, [])
+
+ def test_get_siblings_before_root_with_siblings(self):
+ """Testing get_siblings_before_root with nodes before the root"""
+ root = parse_xml_to_tree('')
+
+ before_root = get_siblings_before_root(root)
+
+ self.assertEqual(before_root, ['\n', '\n'])
+
+ def test_get_siblings_after_root_no_siblings(self):
+ """Testing get_siblings_after_root with no nodes after the root"""
+ root = parse_xml_to_tree('')
+
+ after_root = get_siblings_after_root(root)
+
+ self.assertEqual(after_root, [])
+
+ def test_get_siblings_after_root_with_siblings(self):
+ """Testing get_siblings_after_root with nodes after the root"""
+ root = parse_xml_to_tree('')
+
+ after_root = get_siblings_after_root(root)
+
+ self.assertEqual(after_root, ['\n', '\n'])
+
+ def test_prettify_xml_no_declaration(self):
+ """Testing prettify_xml with no xml declaration tag"""
+ xml = ''
+
+ formatted_string = prettify_xml(xml)
+
+ self.assertEqual(formatted_string, """\
+
+
+
+""")
+
+ def test_prettify_xml_with_external_dtd(self):
+ """Testing prettify_xml with a doctype tag with external entities"""
+ xml = '' \
+ + '' \
+ + '' \
+ + ''
+
+ formatted_string = prettify_xml(xml)
+
+ self.assertEqual(formatted_string, """\
+
+
+
+
+
+""")
+
+ def test_prettify_xml_with_internal_dtd(self):
+ """Testing prettify_xml with a doctype tag with internal entities"""
+ xml = '' \
+ + '' \
+ + ']>' \
+ + ''
+
+ formatted_string = prettify_xml(xml)
+
+ self.assertEqual(formatted_string, """\
+
+
+
+]>
+
+
+
+""")
+
+ def test_prettify_xml_with_declaration(self):
+ """Testing prettify_xml with an xml declaration tag"""
+ xml = ''
+
+ formatted_string = prettify_xml(xml)
+
+ self.assertEqual(formatted_string, """\
+
+
+
+
+""")
+
+ def test_prettify_xml_with_empty_string(self):
+ """Testing prettify_xml with an empty string passed in"""
+ formatted_string = prettify_xml('')
+
+ self.assertEqual(formatted_string, '')
+
+ def test_prettify_xml_with_detailed_xml(self):
+ """Testing prettify_xml with detailed xml"""
+
+ xml = """\
+
+
+
+
+
+ Not
+ Real
+ XML
+
+
test
+
test
+
+
+ Test nest
+
+
+ Example CDATA]]>
+
+
+
+"""
+ formatted_string = prettify_xml(xml)
+
+ self.assertEqual(formatted_string, """\
+
+
+
+
+
+ Not
+ Real
+ XML
+
+
+ test
+
+
+ test
+
+
+
+
+
+ Test nest
+
+
+
+ Example CDATA]]>
+
+
+
+
+""")
+
+ def test_render_xml_as_html_double_quotes(self):
+ """Testing render_xml_as_html escaping and prettifying xml"""
+ xml = ''
+
+ formatted_string = render_xml_as_html(xml)
+
+ self.assertEqual(formatted_string, u"""\
+<root \
+test_key="test value"\
+>
+ <child/>
+</root>
+""")
+
+ def test_render_xml_as_html_single_quotes(self):
+ """Testing render_xml_as_html escaping single quotes"""
+ xml = ''
+
+ formatted_string = render_xml_as_html(xml)
+
+ self.assertEqual(formatted_string, u"""\
+<root \
+test_key="test value"\
+>
+ <child/>
+</root>
+""")
+
+ def test_is_comment_element_with_comment(self):
+ """Testing is_comment_element with an element being a comment"""
+ xml_contents = ''
+ tree_root = parse_xml_to_tree(xml_contents)
+ element_node = tree_root.getchildren()[0]
+
+ comment = is_comment_element(element_node)
+ self.assertTrue(comment)
+
+ def test_is_comment_element_with_normal_tag(self):
+ """Testing is_comment_element with an non-comment element"""
+ xml_contents = ''
+ tree_root = parse_xml_to_tree(xml_contents)
+
+ comment = is_comment_element(tree_root)
+ self.assertFalse(comment)
+
+ def test_parse_text_from_element_source(self):
+ """Testing parse_text_from_element_source returning the text"""
+ xml_contents = b'Test CDATA]]>'
+
+ parsed_contents = parse_text_from_element_source('root', xml_contents)
+
+ self.assertEqual(
+ parsed_contents, 'Test CDATA]]>')
diff --git a/reviewboard/reviews/ui/__init__.py b/reviewboard/reviews/ui/__init__.py
index 64c9eac0c4bd31e78ae84e136c80c93579ca13f6..11ff2d208f9937ab84fae9bc0d2f71012d236270 100644
--- a/reviewboard/reviews/ui/__init__.py
+++ b/reviewboard/reviews/ui/__init__.py
@@ -8,10 +8,12 @@ def _register_review_uis(**kwargs):
from reviewboard.reviews.ui.base import register_ui
from reviewboard.reviews.ui.image import ImageReviewUI
from reviewboard.reviews.ui.markdownui import MarkdownReviewUI
+ from reviewboard.reviews.ui.xmlui import XMLReviewUI
from reviewboard.reviews.ui.text import TextBasedReviewUI
register_ui(ImageReviewUI)
register_ui(MarkdownReviewUI)
+ register_ui(XMLReviewUI)
register_ui(TextBasedReviewUI)
diff --git a/reviewboard/reviews/ui/base.py b/reviewboard/reviews/ui/base.py
index 69d30dfd4471f1cffc6287a2c352202905baf778..0c8d5fcaf808c105863b909135c957553cae69cf 100644
--- a/reviewboard/reviews/ui/base.py
+++ b/reviewboard/reviews/ui/base.py
@@ -3,27 +3,94 @@ from __future__ import unicode_literals
import json
import logging
import os
+import re
+import warnings
from uuid import uuid4
import mimeparse
+from django.conf.urls import include, url
from django.core.exceptions import ObjectDoesNotExist
-from django.http import HttpResponse
+from django.http import HttpResponse, Http404
from django.utils import six
-from django.utils.safestring import mark_safe
+from django.utils.text import slugify
from django.utils.translation import ugettext as _
+from django.views.generic.base import View
from djblets.util.compat.django.template.loader import render_to_string
-
+from reviewboard.accounts.mixins import UserProfileRequiredViewMixin
from reviewboard.attachments.mimetypes import MIMETYPE_EXTENSIONS, score_match
from reviewboard.attachments.models import (FileAttachment,
get_latest_file_attachments)
+from reviewboard.deprecation import RemovedInReviewBoard60Warning
from reviewboard.reviews.context import make_review_request_context
from reviewboard.reviews.markdown_utils import (markdown_render_conditional,
normalize_text_for_edit)
-from reviewboard.reviews.models import FileAttachmentComment, Review
+from reviewboard.reviews.models import (FileAttachmentComment, Review,
+ ReviewRequest)
+from reviewboard.reviews.views_mixins import ReviewFileAttachmentViewMixin
from reviewboard.site.urlresolvers import local_site_reverse
_file_attachment_review_uis = []
+_file_attachment_url_patterns = {}
+
+
+class BaseReviewUIUtilityView(ReviewFileAttachmentViewMixin,
+ UserProfileRequiredViewMixin,
+ View):
+
+ def dispatch(self, request, local_site=None, *args, **kwargs):
+ """Dispatch the view.
+
+
+ Args:
+ request (django.http.HttpRequest):
+ The current HTTP request.
+
+ local_site (reviewboard.site.models.LocalSite):
+ The LocalSite on which the UI is being requested.
+
+ *args (tuple, unused):
+ Ignored positional arguments.
+
+ **kwargs (dict, unused):
+ Ignored keyword arguments.
+
+ Returns:
+ django.http.HttpResponse:
+ The HTTP response for the search.
+ """
+ file_attachment_id = kwargs['file_attachment_id']
+
+ if 'file_attachment_diff_id' in kwargs:
+ file_attachment_diff_id = kwargs['file_attachment_diff_id']
+ else:
+ file_attachment_diff_id = None
+
+ review_request_id = kwargs['review_request_id']
+ if local_site:
+ review_request = ReviewRequest.objects.get(
+ local_site=local_site, local_id=review_request_id)
+ else:
+ review_request = ReviewRequest.objects.get(pk=review_request_id)
+
+ file_attachment, file_attachment_revision = self.get_attachments(
+ request, file_attachment_id, review_request,
+ file_attachment_diff_id)
+
+ self.review_ui = self.set_attachment_ui(file_attachment)
+ self.review_ui.review_request = review_request
+
+ if file_attachment_revision:
+ self.review_ui.set_diff_against(file_attachment_revision)
+
+ is_enabled_for = self.set_enabled_for(request, self.review_ui,
+ review_request, file_attachment)
+ if self.review_ui and is_enabled_for:
+ return super(BaseReviewUIUtilityView, self).dispatch(request,
+ *args,
+ **kwargs)
+ else:
+ raise Http404
class ReviewUI(object):
@@ -83,6 +150,13 @@ class ReviewUI(object):
#: Whether this Review UI supports diffing two objects.
supports_diffing = False
+ #: The id of the file format namespace to use for the Review UI.
+ #: This should be set on all custom Review UIs.
+ review_ui_id = None
+
+ #: URL patterns registered by the subclass. This defaults to None.
+ url_patterns = None
+
#: A list of CSS bundle names to include on the Review UI's page.
css_bundle_names = []
@@ -884,6 +958,27 @@ def register_ui(review_ui):
raise TypeError('Only FileAttachmentReviewUI subclasses can be '
'registered')
+ if not review_ui.review_ui_id:
+ review_ui.review_ui_id = slugify(
+ '%s.%s' % (review_ui.__module__, review_ui.__name__))
+ warnings.warn('%r should set review_ui_id. This will be required '
+ 'in Review Board 6.0. Defaulting the ID Defaulting '
+ 'the ID to "%s".'
+ % (review_ui, review_ui.review_ui_id),
+ RemovedInReviewBoard60Warning,
+ stacklevel=2)
+
+ if review_ui.url_patterns:
+ import reviewboard.reviews.urls as review_request_urls
+ ui_urlpatterns = [
+ url(r'^%s/'
+ % re.escape(review_ui.review_ui_id),
+ include(review_ui.url_patterns)),
+ ]
+
+ _file_attachment_url_patterns[review_ui.review_ui_id] = ui_urlpatterns
+ review_request_urls.dynamic_review_ui_urls.add_patterns(ui_urlpatterns)
+
_file_attachment_review_uis.append(review_ui)
@@ -912,6 +1007,13 @@ def unregister_ui(review_ui):
raise TypeError('Only FileAttachmentReviewUI subclasses can be '
'unregistered')
+ if review_ui.review_ui_id in _file_attachment_url_patterns:
+ import reviewboard.reviews.urls as review_request_urls
+ ui_urlpatterns = _file_attachment_url_patterns[review_ui.review_ui_id]
+ review_request_urls.dynamic_review_ui_urls\
+ .remove_patterns(ui_urlpatterns)
+ del _file_attachment_url_patterns[review_ui.review_ui_id]
+
try:
_file_attachment_review_uis.remove(review_ui)
except ValueError:
diff --git a/reviewboard/reviews/ui/text.py b/reviewboard/reviews/ui/text.py
index f231edf8700873ed02f5e48b827f78a285d2b604..d4429d9b7c49ec1a08f088753202459517e35dc9 100644
--- a/reviewboard/reviews/ui/text.py
+++ b/reviewboard/reviews/ui/text.py
@@ -2,6 +2,7 @@ from __future__ import unicode_literals
import logging
+from django.http import HttpResponse, Http404
from django.utils.encoding import force_bytes
from django.utils.safestring import mark_safe
from djblets.cache.backend import cache_memoize
@@ -9,12 +10,47 @@ from djblets.util.compat.django.template.loader import render_to_string
from pygments import highlight
from pygments.lexers import (ClassNotFound, guess_lexer_for_filename,
TextLexer)
-
+from django.conf.urls import url
from reviewboard.attachments.models import FileAttachment
from reviewboard.diffviewer.chunk_generator import (NoWrapperHtmlFormatter,
RawDiffChunkGenerator)
from reviewboard.diffviewer.diffutils import get_chunks_in_range
-from reviewboard.reviews.ui.base import FileAttachmentReviewUI
+from reviewboard.reviews.ui.base import (FileAttachmentReviewUI,
+ BaseReviewUIUtilityView)
+
+
+class TextBasedReviewUITextView(BaseReviewUIUtilityView):
+ """Displays a text file attachment with a review UI."""
+ def get(self, request, *args, **kwargs):
+ for query_param in request.GET:
+ kwargs[query_param] = request.GET[query_param]
+ render_type = kwargs.get('render_type', 'source')
+
+ if render_type not in ['rendered', 'source']:
+ raise Http404
+
+ try:
+ context = self.review_ui.build_render_context(request, inline=True)
+ context.update(self.review_ui.get_extra_context(request))
+ except Exception as e:
+ context = {}
+ logging.exception('Error when calling get_extra_context for '
+ '%r: %s', self, e)
+
+ if render_type == 'rendered':
+ template = 'reviews/ui/_text_rendered_table.html'
+ context['lines'] = [
+ mark_safe(line)
+ for line in self.review_ui.generate_render(**kwargs)
+ ]
+ context['chunks'] = context.get('rendered_chunks', None)
+ else:
+ template = 'reviews/ui/_text_table.html'
+ context['lines'] = context['text_lines']
+ context['chunks'] = context.get('source_chunks', None)
+
+ return HttpResponse(render_to_string(template_name=template,
+ context=context, request=request))
class TextBasedReviewUI(FileAttachmentReviewUI):
@@ -29,6 +65,8 @@ class TextBasedReviewUI(FileAttachmentReviewUI):
'text/*',
'application/x-javascript',
]
+ review_ui_id = 'text'
+
template_name = 'reviews/ui/text.html'
comment_thumbnail_template_name = 'reviews/ui/text_comment_thumbnail.html'
can_render_text = False
@@ -42,9 +80,19 @@ class TextBasedReviewUI(FileAttachmentReviewUI):
js_model_class = 'RB.TextBasedReviewable'
js_view_class = 'RB.TextBasedReviewableView'
+ url_patterns = [
+ url(r'^_render/(?P[\w.@+-]+)',
+ TextBasedReviewUITextView.as_view(),
+ name='render_options')
+ ]
+
+ def customize_render(self):
+ pass
+
def get_js_model_data(self):
data = super(TextBasedReviewUI, self).get_js_model_data()
data['hasRenderedView'] = self.can_render_text
+ data['reviewUIId'] = self.review_ui_id
if self.can_render_text:
data['viewMode'] = 'rendered'
@@ -56,7 +104,6 @@ class TextBasedReviewUI(FileAttachmentReviewUI):
def get_extra_context(self, request):
context = {}
diff_type_mismatch = False
-
if self.diff_against_obj:
diff_against_review_ui = self.diff_against_obj.review_ui
@@ -103,7 +150,6 @@ class TextBasedReviewUI(FileAttachmentReviewUI):
'num_revisions': num_revisions,
'diff_type_mismatch': diff_type_mismatch,
})
-
return context
def get_text(self):
@@ -177,7 +223,7 @@ class TextBasedReviewUI(FileAttachmentReviewUI):
except ClassNotFound:
return TextLexer()
- def generate_render(self):
+ def generate_render(self, **kwargs):
"""Generates a render of the text.
By default, this won't do anything. Subclasses should override it
diff --git a/reviewboard/reviews/ui/xmlui.py b/reviewboard/reviews/ui/xmlui.py
new file mode 100644
index 0000000000000000000000000000000000000000..c24964275b0c19bc9ad756187e3c3a2dfc603bb3
--- /dev/null
+++ b/reviewboard/reviews/ui/xmlui.py
@@ -0,0 +1,386 @@
+from __future__ import unicode_literals
+
+import logging
+
+from lxml import etree
+
+from django.utils.translation import ugettext as _
+from django.utils.encoding import force_text, force_bytes
+
+from pygments.lexers import XmlLexer
+from pygments import highlight
+
+from reviewboard.reviews.ui.text import TextBasedReviewUI
+from reviewboard.diffviewer.chunk_generator import NoWrapperHtmlFormatter
+
+
+def get_indent_str(indent_level):
+ """Gets the string to use for indentation with the specified indentation
+ level.
+ """
+ return ' ' * indent_level
+
+
+def is_comment_element(element):
+ """Gets whether the specified element is a comment or not."""
+ return element.tag is etree.Comment
+
+
+def parse_text_from_element_source(tag_name, element_source):
+ """Parses the inner text content of the specified element."""
+ tag_as_bytes = force_bytes(tag_name)
+
+ start_indicator = b'<' + tag_as_bytes + b'>'
+ start_index = element_source.find(start_indicator) + len(start_indicator)
+
+ end_indicator = b''
+ end_index = element_source.rfind(end_indicator)
+
+ return force_text(element_source[start_index:end_index])
+
+
+def get_element_text(element):
+ """Gets the text content that the specified element has.
+
+ If it does not have any text, a blank string will be returned.
+ If it has actual text, the text will have leading and trailing whitespace
+ stripped.
+ """
+ element_as_string = etree.tostring(element)
+ has_cdata = element_as_string.find(b' -1
+
+ if has_cdata:
+ text = parse_text_from_element_source(element.tag, element_as_string)
+ else:
+ text = element.text
+
+ return text.strip() if text else ''
+
+
+def get_attributes_str(element):
+ """Gets the attributes associated with the specified element has as a
+ space separated string of key-value pairs.
+ """
+ attributes = ''
+ for attribute in element.attrib:
+ if attributes:
+ attributes += ' '
+
+ attributes += '{}="{}"'.format(attribute, element.attrib[attribute])
+ return attributes
+
+
+def get_formatted_attributes(element):
+ """Formats the attributes an element has for display."""
+ attributes = get_attributes_str(element)
+ if attributes:
+ return ' ' + attributes
+
+ return ''
+
+
+def indent_text(text, indent_level):
+ """Indents each line in the specified text based on the given
+ indentation level.
+ """
+ indent_str = get_indent_str(indent_level)
+ lines = text.split('\n')
+
+ for i in range(0, len(lines)):
+ lines[i] = indent_str + lines[i]
+
+ return '\n'.join(lines)
+
+
+def format_empty_element_tag(element, indent_level):
+ """Formats the specified element as an empty element tag.
+ Indentation is applied according to the specified indentation level.
+ """
+ indent = get_indent_str(indent_level)
+ attributes = get_formatted_attributes(element)
+
+ return '%(indent)s<%(tag)s%(attributes)s />\n' % {
+ 'indent': indent, 'tag': element.tag, 'attributes': attributes}
+
+
+def format_element_with_text(element, indent_level, same_line=False):
+ """Formats the specified element as one with text content.
+ Indentation is applied according to the specified indentation level.
+ """
+ indent = get_indent_str(indent_level)
+ text = get_element_text(element)
+ attributes = get_formatted_attributes(element)
+
+ if same_line:
+ line_separator = ''
+ closing_indent = ''
+ else:
+ text = indent_text(text, indent_level + 1)
+ line_separator = '\n'
+ closing_indent = indent
+
+ format_options = {
+ 'indent': indent, 'tag': element.tag, 'text': text,
+ 'attr': attributes, 'separator': line_separator,
+ 'closing_indent': closing_indent
+ }
+
+ return '%(indent)s<%(tag)s%(attr)s>%(separator)s' \
+ '%(text)s%(separator)s' \
+ '%(closing_indent)s\n' % format_options
+
+
+def format_comment_element(element, indent_level):
+ """Formats the specified element as one that is a comment.
+ Indentation is applied according to the specified indentation level.
+ """
+ indent = get_indent_str(indent_level)
+
+ return '%(indent)s\n' % {
+ 'indent': indent, 'comment': element.text}
+
+
+def format_element_with_children(element, indent_level, same_line_text=False):
+ """Formats the specified element as one with children.
+ Indentation is applied according to the specified indentation level.
+ """
+ indent = get_indent_str(indent_level)
+ attributes = get_formatted_attributes(element)
+ children = element.getchildren()
+
+ contents = '%(indent)s<%(tag)s%(attributes)s>\n' % {
+ 'indent': indent, 'tag': element.tag, 'attributes': attributes}
+
+ for child in children:
+ contents += format_element(child, indent_level + 1, same_line_text)
+
+ closing_tag = '%(indent)s\n' % {
+ 'indent': indent, 'tag': element.tag}
+
+ return contents + closing_tag
+
+
+def format_element(element, indent=0, keep_text_on_same_line=False):
+ """Formats the specified element.
+ Indentation is applied according to the specified indentation level.
+ """
+ children = element.getchildren()
+
+ if children:
+ return format_element_with_children(element, indent,
+ keep_text_on_same_line)
+ elif is_comment_element(element):
+ return format_comment_element(element, indent)
+
+ text = get_element_text(element)
+ if text:
+ return format_element_with_text(element, indent,
+ same_line=keep_text_on_same_line)
+ else:
+ return format_empty_element_tag(element, indent)
+
+
+def get_xml_declaration(raw_xml):
+ """Gets the declaration tag associated with the specified xml contents.
+ If no declaration tag is found, an empty string is returned.
+ """
+ raw_xml = force_text(raw_xml)
+
+ if raw_xml.find('') + 2
+ return force_text(raw_xml[:end_index], encoding='ascii')
+ return ''
+
+
+def get_encoding_from_declaration(declaration):
+ """Parses the specified xml declaration tag to find the encoding version
+ that is specified in it. If no encoding attribute exists, ASCII is
+ returned as the default value.
+ """
+ search_text = "encoding="
+ encoding_index = declaration.find(search_text)
+
+ if encoding_index > -1:
+ value = declaration[encoding_index + len(search_text):]
+ quote_type = value[0]
+
+ return value.split(quote_type)[1]
+
+ return 'ASCII'
+
+
+def parse_xml_to_tree(xml):
+ """Parses the specified xml contents into an abstract syntax tree.
+ The contents will be coerced into byte form if it is not already a
+ bytestring.
+ """
+ declaration = get_xml_declaration(xml)
+ encoding = get_encoding_from_declaration(declaration)
+
+ xml_as_bytes = force_bytes(xml, encoding=encoding)
+ parser = etree.XMLParser(strip_cdata=False)
+ return etree.fromstring(xml_as_bytes, parser)
+
+
+def get_siblings_before_root(root):
+ """Gets a list of root level non-content nodes that are siblings
+ of the root, and appear before the root. The items in the list are
+ formatted as strings.
+ """
+ root_level_siblings = []
+
+ root_level_it = root.getprevious()
+ while root_level_it is not None:
+ formatted_element = format_element(root_level_it, 0)
+ root_level_siblings.insert(0, formatted_element)
+
+ root_level_it = root_level_it.getprevious()
+
+ return root_level_siblings
+
+
+def get_siblings_after_root(root):
+ """Gets a list of root level non-content nodes that are siblings
+ of the root, and appear after the root. The items in the list are
+ formatted as strings.
+ """
+ root_level_siblings = []
+
+ root_level_it = root.getnext()
+ while root_level_it is not None:
+ formatted_element = format_element(root_level_it, 0)
+ root_level_siblings.append(formatted_element)
+
+ root_level_it = root_level_it.getnext()
+
+ return root_level_siblings
+
+
+def get_formatted_tree(root, keep_text_on_same_line=False):
+ """Gets the contents of the specified XML abstract syntax tree formatted
+ with proper indentation.
+ """
+ siblings_before_root = get_siblings_before_root(root)
+ siblings_after_root = get_siblings_after_root(root)
+
+ return '\n'.join(siblings_before_root) \
+ + format_element(
+ root, keep_text_on_same_line=keep_text_on_same_line) \
+ + '\n'.join(siblings_after_root)
+
+
+def get_external_dtd_string(docinfo):
+ base_dtd = '\n'
+ # TODO
+ return base_dtd % (
+ docinfo.root_name, docinfo.system_url)
+
+
+def get_internal_dtd_string(docinfo):
+ base_dtd = '\n'
+
+ elements = []
+ indent_str = get_indent_str(1)
+
+ for el in docinfo.internalDTD.elements():
+ content = el.content
+ if content.type == 'element':
+ type_contents = '(%s)' % content.name
+ else:
+ type_contents = '(#%s)' % content.type.upper()
+
+ entity_details = '\n%s' % (
+ indent_str, el.name, type_contents)
+ elements.append(entity_details)
+ return base_dtd % (docinfo.root_name, '[%s\n]' % ''.join(elements))
+
+
+def get_dtd_string(docinfo):
+ if docinfo.system_url is not None:
+ return get_external_dtd_string(docinfo)
+ elif docinfo.internalDTD is not None:
+ return get_internal_dtd_string(docinfo)
+
+ return ''
+
+
+def prettify_xml(xml, keep_text_on_same_line=False):
+ """Prettifies the specified xml contents, creating consistently nested
+ tags and content.
+ """
+ if not xml:
+ return ''
+
+ root = parse_xml_to_tree(xml)
+
+ declaration = get_xml_declaration(xml)
+ formatted_contents = get_formatted_tree(root, keep_text_on_same_line)
+
+ if declaration:
+ docinfo = root.getroottree().docinfo
+ dtd = get_dtd_string(docinfo)
+ return declaration + '\n' + dtd + formatted_contents
+
+ return formatted_contents
+
+
+def render_xml_as_html(xml, keep_text_on_same_line=False):
+ """Converts the specified xml into syntax-highlighted HTML, with proper
+ escaping of the contents according to html requirements.
+ """
+ pretty_contents = prettify_xml(xml, keep_text_on_same_line)
+
+ html_contents = highlight(
+ pretty_contents, XmlLexer(), NoWrapperHtmlFormatter())
+
+ return html_contents
+
+
+def iter_htmlified_xml_lines(html):
+ """Iterates through each line in the specified html contents. Yields
+ each line wrapped by
tags.
+ """
+ for line in html.split('\n'):
+ if line.strip():
+ yield '
%s
' % line
+
+
+def get_render_text_on_line_setting(query_params):
+ renderOnSameLine = query_params.get('renderTextContentOnSameLine', False)
+ if type(renderOnSameLine) != bool:
+ return renderOnSameLine == 'true'
+ return renderOnSameLine
+
+
+class XMLReviewUI(TextBasedReviewUI):
+ """A Review UI for XML files.
+
+ This renders the XML to HTML, and allows users to comment on each
+ tag and property value.
+ """
+ supported_mimetypes = ['application/xml', 'text/xml']
+ object_key = 'xml'
+ can_render_text = True
+
+ js_model_class = 'RB.XMLBasedReviewable'
+ js_view_class = 'RB.XMLReviewableView'
+
+ def generate_render(self, **kwargs):
+ try:
+ with self.obj.file as f:
+ f.open()
+ contents = f.read()
+ rendered = render_xml_as_html(
+ contents, get_render_text_on_line_setting(kwargs))
+
+ for line in iter_htmlified_xml_lines(rendered):
+ yield line
+ except Exception as e:
+ logging.error('Failed to parse resulting XML HTML for '
+ 'file attachment %d: %s',
+ self.obj.pk, e,
+ exc_info=True)
+ yield _('Error while rendering XML content: %s') % e
+
+ def get_source_lexer(self, filename, data):
+ return XmlLexer()
diff --git a/reviewboard/reviews/urls.py b/reviewboard/reviews/urls.py
index 1b9fe7d5ee658a33f95f46248420c1fc898c40fb..81db63aff5c41b77f259c980213a056d7fc806bc 100644
--- a/reviewboard/reviews/urls.py
+++ b/reviewboard/reviews/urls.py
@@ -1,9 +1,11 @@
from __future__ import unicode_literals
from django.conf.urls import include, url
+from djblets.urls.resolvers import DynamicURLResolver
from reviewboard.reviews import views
+dynamic_review_ui_urls = DynamicURLResolver()
download_diff_urls = [
url(r'^orig/$',
@@ -94,15 +96,20 @@ review_request_urls = [
views.CommentDiffFragmentsView.as_view(),
name='diff-comment-fragments'),
+
# File attachments
- url(r'^file/(?P\d+)/$',
- views.ReviewFileAttachmentView.as_view(),
- name='file-attachment'),
+ url(r'^file/(?P\d+)/', include([
+ dynamic_review_ui_urls,
+ url('$', views.ReviewFileAttachmentView.as_view(),
+ name='file-attachment'),
+ ])),
url(r'^file/(?P\d+)'
- r'-(?P\d+)/$',
- views.ReviewFileAttachmentView.as_view(),
- name='file-attachment'),
+ r'-(?P\d+)/', include([
+ url('$', views.ReviewFileAttachmentView.as_view(),
+ name='file-attachment'),
+ dynamic_review_ui_urls,
+ ])),
# Screenshots
url(r'^s/(?P\d+)/$',
diff --git a/reviewboard/reviews/views.py b/reviewboard/reviews/views.py
index 809b4207d14179aadeaba8f2760f2263385a7338..0509a0f121a2dea53f244cd03d663067bc9b52ed 100644
--- a/reviewboard/reviews/views.py
+++ b/reviewboard/reviews/views.py
@@ -15,13 +15,11 @@ from django.http import (Http404,
HttpResponse,
HttpResponseBadRequest,
HttpResponseNotFound)
-from django.shortcuts import get_object_or_404, get_list_or_404, render
-from django.template.defaultfilters import date
+from django.shortcuts import get_object_or_404, get_list_or_404
from django.utils import six, timezone
-from django.utils.formats import localize
from django.utils.html import escape, format_html, strip_tags
from django.utils.safestring import mark_safe
-from django.utils.timezone import is_aware, localtime, make_aware, utc
+from django.utils.timezone import is_aware, make_aware, utc
from django.utils.translation import ugettext_lazy as _, ugettext
from django.views.generic.base import (ContextMixin, RedirectView,
TemplateView, View)
@@ -29,15 +27,12 @@ from djblets.siteconfig.models import SiteConfiguration
from djblets.util.compat.django.template.loader import render_to_string
from djblets.util.dates import get_latest_timestamp
from djblets.util.http import set_last_modified
-from djblets.views.generic.base import (CheckRequestMethodViewMixin,
- PrePostDispatchViewMixin)
from djblets.views.generic.etag import ETagViewMixin
from reviewboard.accounts.mixins import (CheckLoginRequiredViewMixin,
LoginRequiredViewMixin,
UserProfileRequiredViewMixin)
from reviewboard.accounts.models import ReviewRequestVisit, Profile
-from reviewboard.admin.decorators import check_read_only
from reviewboard.admin.mixins import CheckReadOnlyViewMixin
from reviewboard.admin.read_only import is_site_read_only_for
from reviewboard.attachments.models import (FileAttachment,
@@ -72,271 +67,14 @@ from reviewboard.reviews.models import (Comment,
Review,
ReviewRequest,
Screenshot)
-from reviewboard.reviews.ui.base import FileAttachmentReviewUI
+from reviewboard.reviews.views_mixins import (ReviewRequestViewMixin,
+ ReviewFileAttachmentViewMixin)
from reviewboard.scmtools.errors import FileNotFoundError
from reviewboard.scmtools.models import Repository
from reviewboard.site.mixins import CheckLocalSiteAccessViewMixin
from reviewboard.site.urlresolvers import local_site_reverse
-class ReviewRequestViewMixin(CheckRequestMethodViewMixin,
- CheckLoginRequiredViewMixin,
- CheckLocalSiteAccessViewMixin,
- PrePostDispatchViewMixin):
- """Common functionality for all review request-related pages.
-
- This performs checks to ensure that the user has access to the page,
- returning an error page if not. It also provides common functionality
- for fetching a review request for the given page, returning suitable
- context for the template, and generating an image used to represent
- the site when posting to social media sites.
- """
-
- permission_denied_template_name = \
- 'reviews/review_request_permission_denied.html'
-
- def pre_dispatch(self, request, review_request_id, *args, **kwargs):
- """Look up objects and permissions before dispatching the request.
-
- This will first look up the review request, returning an error page
- if it's not accessible. It will then store the review request before
- calling the handler for the HTTP request.
-
- Args:
- request (django.http.HttpRequest):
- The HTTP request from the client.
-
- review_request_id (int):
- The ID of the review request being accessed.
-
- *args (tuple):
- Positional arguments to pass to the handler.
-
- **kwargs (dict):
- Keyword arguments to pass to the handler.
-
- These will be arguments provided by the URL pattern.
-
- Returns:
- django.http.HttpResponse:
- The resulting HTTP response to send to the client, if there's
- a Permission Denied.
- """
- self.review_request = self.get_review_request(
- review_request_id=review_request_id,
- local_site=self.local_site)
-
- if not self.review_request.is_accessible_by(request.user):
- return self.render_permission_denied(request)
-
- return None
-
- def render_permission_denied(self, request):
- """Render a Permission Denied page.
-
- This will be shown to the user if they're not able to view the
- review request.
-
- Args:
- request (django.http.HttpRequest):
- The HTTP request from the client.
-
- Returns:
- django.http.HttpResponse:
- The resulting HTTP response to send to the client.
- """
- return render(request,
- self.permission_denied_template_name,
- status=403)
-
- def get_review_request(self, review_request_id, local_site=None):
- """Return the review request for the given display ID.
-
- Args:
- review_request_id (int):
- The review request's display ID.
-
- local_site (reviewboard.site.models.LocalSite):
- The Local Site the review request is on.
-
- Returns:
- reviewboard.reviews.models.review_request.ReviewRequest:
- The review request for the given display ID and Local Site.
-
- Raises:
- django.http.Http404:
- The review request could not be found.
- """
- q = ReviewRequest.objects.all()
-
- if local_site:
- q = q.filter(local_site=local_site,
- local_id=review_request_id)
- else:
- q = q.filter(pk=review_request_id)
-
- q = q.select_related('submitter', 'repository')
-
- return get_object_or_404(q)
-
- def get_diff(self, revision=None, draft=None):
- """Return a diff on the review request matching the given criteria.
-
- If a draft is provided, and ``revision`` is either ``None`` or matches
- the revision on the draft's DiffSet, that DiffSet will be returned.
-
- Args:
- revision (int, optional):
- The revision of the diff to retrieve. If not provided, the
- latest DiffSet will be returned.
-
- draft (reviewboard.reviews.models.review_request_draft.
- ReviewRequestDraft, optional):
- The draft of the review request.
-
- Returns:
- reviewboard.diffviewer.models.diffset.DiffSet:
- The resulting DiffSet.
-
- Raises:
- django.http.Http404:
- The diff does not exist.
- """
- # Normalize the revision, since it might come in as a string.
- if revision:
- revision = int(revision)
-
- # This will try to grab the diff associated with a draft if the review
- # request has an associated draft and is either the revision being
- # requested or no revision is being requested.
- if (draft and draft.diffset_id and
- (revision is None or draft.diffset.revision == revision)):
- return draft.diffset
-
- query = Q(history=self.review_request.diffset_history_id)
-
- # Grab a revision if requested.
- if revision is not None:
- query = query & Q(revision=revision)
-
- try:
- return DiffSet.objects.filter(query).latest()
- except DiffSet.DoesNotExist:
- raise Http404
-
- def get_social_page_image_url(self, file_attachments):
- """Return the URL to an image used for social media sharing.
-
- This will look for the first attachment in a list of attachments that
- can be used to represent the review request on social media sites and
- chat services. If a suitable attachment is found, its URL will be
- returned.
-
- Args:
- file_attachments (list of reviewboard.attachments.models.
- FileAttachment):
- A list of file attachments used on a review request.
-
- Returns:
- unicode:
- The URL to the first image file attachment, if found, or ``None``
- if no suitable attachments were found.
- """
- for file_attachment in file_attachments:
- if file_attachment.mimetype.startswith('image/'):
- return file_attachment.get_absolute_url()
-
- return None
-
- def get_review_request_status_html(self, review_request_details,
- close_info, extra_info=[]):
- """Return HTML describing the current status of a review request.
-
- This will return a description of the submitted, discarded, or open
- state for the review request, for use in the rendering of the page.
-
- Args:
- review_request_details (reviewboard.reviews.models
- .base_review_request_details
- .BaseReviewRequestDetails):
- The review request or draft being viewed.
-
- close_info (dict):
- A dictionary of information on the closed state of the
- review request.
-
- extra_info (list of dict):
- A list of dictionaries showing additional status information.
- Each must have a ``text`` field containing a format string
- using ``{keyword}``-formatted variables, a ``timestamp`` field
- (which will be normalized to the local timestamp), and an
- optional ``extra_vars`` for the format string.
-
- Returns:
- unicode:
- The status text as HTML for the page.
- """
- review_request = self.review_request
- status = review_request.status
- review_request_details = review_request_details
-
- if status == ReviewRequest.SUBMITTED:
- timestamp = close_info['timestamp']
-
- if timestamp:
- text = ugettext('Created {created_time} and submitted '
- '{timestamp}')
- else:
- text = ugettext('Created {created_time} and submitted')
- elif status == ReviewRequest.DISCARDED:
- timestamp = close_info['timestamp']
-
- if timestamp:
- text = ugettext('Created {created_time} and discarded '
- '{timestamp}')
- else:
- text = ugettext('Created {created_time} and discarded')
- elif status == ReviewRequest.PENDING_REVIEW:
- text = ugettext('Created {created_time} and updated {timestamp}')
- timestamp = review_request_details.last_updated
- else:
- logging.error('Unexpected review request status %r for '
- 'review request %s',
- status, review_request.display_id,
- request=self.request)
-
- return ''
-
- parts = [
- {
- 'text': text,
- 'timestamp': timestamp,
- 'extra_vars': {
- 'created_time': date(localtime(review_request.time_added)),
- },
- },
- ] + extra_info
-
- html_parts = []
-
- for part in parts:
- if part['timestamp']:
- timestamp = localtime(part['timestamp'])
- timestamp_html = format_html(
- '',
- timestamp.isoformat(),
- localize(timestamp))
- else:
- timestamp_html = ''
-
- html_parts.append(format_html(
- part['text'],
- timestamp=timestamp_html,
- **part.get('extra_vars', {})))
-
- return mark_safe(' — '.join(html_parts))
-
-
#
# Helper functions
#
@@ -2004,6 +1742,7 @@ class PreviewReplyEmailView(ReviewRequestViewMixin, BasePreviewEmailView):
class ReviewFileAttachmentView(ReviewRequestViewMixin,
+ ReviewFileAttachmentViewMixin,
UserProfileRequiredViewMixin,
View):
"""Displays a file attachment with a review UI."""
@@ -2033,44 +1772,18 @@ class ReviewFileAttachmentView(ReviewRequestViewMixin,
The resulting HTTP response from the handler.
"""
review_request = self.review_request
- draft = review_request.get_draft(request.user)
- # Make sure the attachment returned is part of either the review request
- # or an accessible draft.
- review_request_q = (Q(review_request=review_request) |
- Q(inactive_review_request=review_request))
+ file_attachment, file_attachment_revision = self.get_attachments(
+ request, file_attachment_id, review_request,
+ file_attachment_diff_id)
- if draft:
- review_request_q |= Q(drafts=draft) | Q(inactive_drafts=draft)
+ review_ui = self.set_attachment_ui(file_attachment)
- file_attachment = get_object_or_404(
- FileAttachment,
- Q(pk=file_attachment_id) & review_request_q)
-
- review_ui = file_attachment.review_ui
-
- if not review_ui:
- review_ui = FileAttachmentReviewUI(review_request, file_attachment)
-
- if file_attachment_diff_id:
- file_attachment_revision = get_object_or_404(
- FileAttachment,
- Q(pk=file_attachment_diff_id) &
- Q(attachment_history=file_attachment.attachment_history) &
- review_request_q)
+ if file_attachment_revision:
review_ui.set_diff_against(file_attachment_revision)
- try:
- is_enabled_for = review_ui.is_enabled_for(
- user=request.user,
- review_request=review_request,
- file_attachment=file_attachment)
- except Exception as e:
- logging.error('Error when calling is_enabled_for for '
- 'FileAttachmentReviewUI %r: %s',
- review_ui, e, exc_info=1)
- is_enabled_for = False
-
+ is_enabled_for = self.set_enabled_for(request, review_ui,
+ review_request, file_attachment)
if review_ui and is_enabled_for:
return review_ui.render_to_response(request)
else:
diff --git a/reviewboard/reviews/views_mixins.py b/reviewboard/reviews/views_mixins.py
new file mode 100644
index 0000000000000000000000000000000000000000..b6feec91d7eee0a042d925de71839e8989956b64
--- /dev/null
+++ b/reviewboard/reviews/views_mixins.py
@@ -0,0 +1,332 @@
+from __future__ import unicode_literals
+
+import logging
+
+from django.db.models import Q
+from django.http import Http404
+from django.shortcuts import get_object_or_404, render
+from django.template.defaultfilters import date
+from django.utils.formats import localize
+from django.utils.html import format_html
+from django.utils.safestring import mark_safe
+from django.utils.timezone import localtime
+from django.utils.translation import ugettext
+from djblets.views.generic.base import (CheckRequestMethodViewMixin,
+ PrePostDispatchViewMixin)
+from reviewboard.accounts.mixins import CheckLoginRequiredViewMixin
+from reviewboard.attachments.models import FileAttachment
+from reviewboard.diffviewer.models import DiffSet
+from reviewboard.reviews.models import ReviewRequest
+# from reviewboard.reviews.ui.base import FileAttachmentReviewUI
+from reviewboard.site.mixins import CheckLocalSiteAccessViewMixin
+
+
+class ReviewRequestViewMixin(CheckRequestMethodViewMixin,
+ CheckLoginRequiredViewMixin,
+ CheckLocalSiteAccessViewMixin,
+ PrePostDispatchViewMixin):
+ """Common functionality for all review request-related pages.
+
+ This performs checks to ensure that the user has access to the page,
+ returning an error page if not. It also provides common functionality
+ for fetching a review request for the given page, returning suitable
+ context for the template, and generating an image used to represent
+ the site when posting to social media sites.
+ """
+
+ permission_denied_template_name = \
+ 'reviews/review_request_permission_denied.html'
+
+ def pre_dispatch(self, request, review_request_id, *args, **kwargs):
+ """Look up objects and permissions before dispatching the request.
+
+ This will first look up the review request, returning an error page
+ if it's not accessible. It will then store the review request before
+ calling the handler for the HTTP request.
+
+ Args:
+ request (django.http.HttpRequest):
+ The HTTP request from the client.
+
+ review_request_id (int):
+ The ID of the review request being accessed.
+
+ *args (tuple):
+ Positional arguments to pass to the handler.
+
+ **kwargs (dict):
+ Keyword arguments to pass to the handler.
+
+ These will be arguments provided by the URL pattern.
+
+ Returns:
+ django.http.HttpResponse:
+ The resulting HTTP response to send to the client, if there's
+ a Permission Denied.
+ """
+ self.review_request = self.get_review_request(
+ review_request_id=review_request_id,
+ local_site=self.local_site)
+
+ if not self.review_request.is_accessible_by(request.user):
+ return self.render_permission_denied(request)
+
+ return None
+
+ def render_permission_denied(self, request):
+ """Render a Permission Denied page.
+
+ This will be shown to the user if they're not able to view the
+ review request.
+
+ Args:
+ request (django.http.HttpRequest):
+ The HTTP request from the client.
+
+ Returns:
+ django.http.HttpResponse:
+ The resulting HTTP response to send to the client.
+ """
+ return render(request,
+ self.permission_denied_template_name,
+ status=403)
+
+ def get_review_request(self, review_request_id, local_site=None):
+ """Return the review request for the given display ID.
+
+ Args:
+ review_request_id (int):
+ The review request's display ID.
+
+ local_site (reviewboard.site.models.LocalSite):
+ The Local Site the review request is on.
+
+ Returns:
+ reviewboard.reviews.models.review_request.ReviewRequest:
+ The review request for the given display ID and Local Site.
+
+ Raises:
+ django.http.Http404:
+ The review request could not be found.
+ """
+ q = ReviewRequest.objects.all()
+
+ if local_site:
+ q = q.filter(local_site=local_site,
+ local_id=review_request_id)
+ else:
+ q = q.filter(pk=review_request_id)
+
+ q = q.select_related('submitter', 'repository')
+
+ return get_object_or_404(q)
+
+ def get_diff(self, revision=None, draft=None):
+ """Return a diff on the review request matching the given criteria.
+
+ If a draft is provided, and ``revision`` is either ``None`` or matches
+ the revision on the draft's DiffSet, that DiffSet will be returned.
+
+ Args:
+ revision (int, optional):
+ The revision of the diff to retrieve. If not provided, the
+ latest DiffSet will be returned.
+
+ draft (reviewboard.reviews.models.review_request_draft.
+ ReviewRequestDraft, optional):
+ The draft of the review request.
+
+ Returns:
+ reviewboard.diffviewer.models.diffset.DiffSet:
+ The resulting DiffSet.
+
+ Raises:
+ django.http.Http404:
+ The diff does not exist.
+ """
+ # Normalize the revision, since it might come in as a string.
+ if revision:
+ revision = int(revision)
+
+ # This will try to grab the diff associated with a draft if the review
+ # request has an associated draft and is either the revision being
+ # requested or no revision is being requested.
+ if (draft and draft.diffset_id and
+ (revision is None or draft.diffset.revision == revision)):
+ return draft.diffset
+
+ query = Q(history=self.review_request.diffset_history_id)
+
+ # Grab a revision if requested.
+ if revision is not None:
+ query = query & Q(revision=revision)
+
+ try:
+ return DiffSet.objects.filter(query).latest()
+ except DiffSet.DoesNotExist:
+ raise Http404
+
+ def get_social_page_image_url(self, file_attachments):
+ """Return the URL to an image used for social media sharing.
+
+ This will look for the first attachment in a list of attachments that
+ can be used to represent the review request on social media sites and
+ chat services. If a suitable attachment is found, its URL will be
+ returned.
+
+ Args:
+ file_attachments (list of reviewboard.attachments.models.
+ FileAttachment):
+ A list of file attachments used on a review request.
+
+ Returns:
+ unicode:
+ The URL to the first image file attachment, if found, or ``None``
+ if no suitable attachments were found.
+ """
+ for file_attachment in file_attachments:
+ if file_attachment.mimetype.startswith('image/'):
+ return file_attachment.get_absolute_url()
+
+ return None
+
+ def get_review_request_status_html(self, review_request_details,
+ close_info, extra_info=[]):
+ """Return HTML describing the current status of a review request.
+
+ This will return a description of the submitted, discarded, or open
+ state for the review request, for use in the rendering of the page.
+
+ Args:
+ review_request_details (reviewboard.reviews.models
+ .base_review_request_details
+ .BaseReviewRequestDetails):
+ The review request or draft being viewed.
+
+ close_info (dict):
+ A dictionary of information on the closed state of the
+ review request.
+
+ extra_info (list of dict):
+ A list of dictionaries showing additional status information.
+ Each must have a ``text`` field containing a format string
+ using ``{keyword}``-formatted variables, a ``timestamp`` field
+ (which will be normalized to the local timestamp), and an
+ optional ``extra_vars`` for the format string.
+
+ Returns:
+ unicode:
+ The status text as HTML for the page.
+ """
+ review_request = self.review_request
+ status = review_request.status
+ review_request_details = review_request_details
+
+ if status == ReviewRequest.SUBMITTED:
+ timestamp = close_info['timestamp']
+
+ if timestamp:
+ text = ugettext('Created {created_time} and submitted '
+ '{timestamp}')
+ else:
+ text = ugettext('Created {created_time} and submitted')
+ elif status == ReviewRequest.DISCARDED:
+ timestamp = close_info['timestamp']
+
+ if timestamp:
+ text = ugettext('Created {created_time} and discarded '
+ '{timestamp}')
+ else:
+ text = ugettext('Created {created_time} and discarded')
+ elif status == ReviewRequest.PENDING_REVIEW:
+ text = ugettext('Created {created_time} and updated {timestamp}')
+ timestamp = review_request_details.last_updated
+ else:
+ logging.error('Unexpected review request status %r for '
+ 'review request %s',
+ status, review_request.display_id,
+ request=self.request)
+
+ return ''
+
+ parts = [
+ {
+ 'text': text,
+ 'timestamp': timestamp,
+ 'extra_vars': {
+ 'created_time': date(localtime(review_request.time_added)),
+ },
+ },
+ ] + extra_info
+
+ html_parts = []
+
+ for part in parts:
+ if part['timestamp']:
+ timestamp = localtime(part['timestamp'])
+ timestamp_html = format_html(
+ '',
+ timestamp.isoformat(),
+ localize(timestamp))
+ else:
+ timestamp_html = ''
+
+ html_parts.append(format_html(
+ part['text'],
+ timestamp=timestamp_html,
+ **part.get('extra_vars', {})))
+
+ return mark_safe(' — '.join(html_parts))
+
+
+class ReviewFileAttachmentViewMixin(CheckLoginRequiredViewMixin):
+
+ def get_attachments(self, request, file_attachment_id, review_request,
+ file_attachment_diff_id=None):
+ # Make sure the attachment returned is part of either the review
+ # request or an accessible draft.
+ draft = review_request.get_draft(request.user)
+ review_request_q = (Q(review_request=review_request) |
+ Q(inactive_review_request=review_request))
+
+ if draft:
+ review_request_q |= Q(drafts=draft) | Q(inactive_drafts=draft)
+
+ file_attachment = get_object_or_404(
+ FileAttachment,
+ Q(pk=file_attachment_id) & review_request_q)
+
+ if file_attachment_diff_id:
+ file_attachment_revision = get_object_or_404(
+ FileAttachment,
+ Q(pk=file_attachment_diff_id) &
+ Q(attachment_history=file_attachment.attachment_history) &
+ review_request_q)
+ else:
+ file_attachment_revision = None
+
+ return file_attachment, file_attachment_revision
+
+ def set_attachment_ui(self, file_attachment):
+ review_ui = file_attachment.review_ui
+
+ # if not review_ui:
+ # review_ui = FileAttachmentReviewUI(self.review_request,
+ # file_attachment)
+
+ return review_ui
+
+ def set_enabled_for(self, request, review_ui, review_request,
+ file_attachment):
+ try:
+ # print(request.user)
+ is_enabled_for = review_ui.is_enabled_for(
+ user=request.user,
+ review_request=review_request,
+ file_attachment=file_attachment)
+ except Exception as e:
+ logging.error('Error when calling is_enabled_for for '
+ 'FileAttachmentReviewUI %r: %s',
+ review_ui, e, exc_info=1)
+ is_enabled_for = False
+ return is_enabled_for
diff --git a/reviewboard/static/rb/css/common.less b/reviewboard/static/rb/css/common.less
index d9fab7776a7f3940c66b270b47a65b3c1ddef892..b9d224cbfb2c6ff452cc09cfffe4aea214533c3f 100644
--- a/reviewboard/static/rb/css/common.less
+++ b/reviewboard/static/rb/css/common.less
@@ -510,4 +510,11 @@ html[xmlns] .clearfix {
height: 1%;
}
+/**
+ * Disables interaction with an element and any of it's children
+ */
+.rb-u-disabled-container {
+ pointer-events: none;
+}
+
// vim: set et ts=2 sw=2:
diff --git a/reviewboard/static/rb/css/pages/text-review-ui.less b/reviewboard/static/rb/css/pages/text-review-ui.less
index 57adde1f721e0be8c3033c91335b388d8a208990..b45a546daf4be4a0f8f42d40ec9b74fe51410e6e 100644
--- a/reviewboard/static/rb/css/pages/text-review-ui.less
+++ b/reviewboard/static/rb/css/pages/text-review-ui.less
@@ -135,7 +135,7 @@
}
}
-#attachment_revision_selector, #revision_label {
+#attachment_revision_selector, #revision_label, .render-options {
padding-top: 4px;
padding-left: 12px;
}
@@ -166,4 +166,4 @@
padding-left: 2em;
}
}
-}
+}
\ No newline at end of file
diff --git a/reviewboard/static/rb/js/models/xmlBasedCommentBlockModel.es6.js b/reviewboard/static/rb/js/models/xmlBasedCommentBlockModel.es6.js
new file mode 100644
index 0000000000000000000000000000000000000000..ced390b6214eeb2417a0d2735c1e5517e9bdf59b
--- /dev/null
+++ b/reviewboard/static/rb/js/models/xmlBasedCommentBlockModel.es6.js
@@ -0,0 +1,23 @@
+/**
+ * Represents the comments on an element in a XML file attachment.
+ *
+ * XMLCommentBlock deals with creating and representing comments
+ * that exist on a specific element of some content for XML files.
+ *
+ * Model Attributes:
+ * renderTextContentOnSameLine (boolean):
+ * Whether or not the text content of nodes in the file should
+ * be rendered on the same line as the opening tag of the node.
+ *
+ * See Also:
+ * :js:class:`RB.TextCommentBlock`:
+ * For the attributes defined on all text comment blocks.
+ */
+RB.XMLCommentBlock = RB.TextCommentBlock.extend({
+ defaults: _.defaults({
+ renderTextContentOnSameLine: false,
+ }, RB.TextCommentBlock.prototype.defaults),
+
+ serializedFields: ['renderTextContentOnSameLine'].concat(
+ RB.TextCommentBlock.prototype.serializedFields),
+});
diff --git a/reviewboard/static/rb/js/models/xmlBasedReviewableModel.es6.js b/reviewboard/static/rb/js/models/xmlBasedReviewableModel.es6.js
new file mode 100644
index 0000000000000000000000000000000000000000..ed7b897516bc8ba521efe38e13783ccd4ff048e1
--- /dev/null
+++ b/reviewboard/static/rb/js/models/xmlBasedReviewableModel.es6.js
@@ -0,0 +1,19 @@
+/**
+ * Provides review capabilities for XML file attachments.
+ *
+ * Model Attributes:
+ * renderTextContentOnSameLine (boolean):
+ * Whether or not the text content of nodes in the file should
+ * be rendered on the same line as the opening tag of the node.
+ */
+RB.XMLBasedReviewable = RB.TextBasedReviewable.extend({
+ defaults: _.defaults({
+ renderTextContentOnSameLine: false,
+ }, RB.TextBasedReviewable.prototype.defaults),
+
+ commentBlockModel: RB.XMLCommentBlock,
+
+ defaultCommentBlockFields: [
+ 'renderTextContentOnSameLine',
+ ].concat(RB.TextBasedReviewable.prototype.defaultCommentBlockFields),
+});
diff --git a/reviewboard/static/rb/js/views/abstractReviewableView.es6.js b/reviewboard/static/rb/js/views/abstractReviewableView.es6.js
index f83d63a48df30b914db7f8d284948c7c597a67d4..294de2c9c3002c3f21830cd250ac22b3c1feecdc 100644
--- a/reviewboard/static/rb/js/views/abstractReviewableView.es6.js
+++ b/reviewboard/static/rb/js/views/abstractReviewableView.es6.js
@@ -33,6 +33,7 @@ RB.AbstractReviewableView = Backbone.View.extend({
this.commentDlg = null;
this._activeCommentBlock = null;
this.renderedInline = options.renderedInline || false;
+ this.commentBlockViews = [];
},
/**
@@ -47,11 +48,16 @@ RB.AbstractReviewableView = Backbone.View.extend({
*/
render() {
this.renderContent();
+ this.renderCommentBlocks();
+ return this;
+ },
+ /**
+ * Renders all comment blocks into the view.
+ */
+ renderCommentBlocks() {
this.model.commentBlocks.each(this._addCommentBlockView, this);
this.model.commentBlocks.on('add', this._addCommentBlockView, this);
-
- return this;
},
/**
@@ -127,6 +133,43 @@ RB.AbstractReviewableView = Backbone.View.extend({
});
},
+ /**
+ * Removes all comments from the view, and re-attaches them
+ */
+ refreshComments() {
+ this._hideCommentDlg();
+ this._disposeComments();
+ this.renderCommentBlocks();
+ },
+
+ _hideCommentDlg() {
+ if (this.commentDlg) {
+ this.commentDlg.close();
+ }
+ },
+
+ _disposeComments() {
+ this.commentBlockViews.forEach((view) => {
+ view.dispose();
+ });
+ this.commentBlockViews = [];
+ },
+
+ /**
+ * Gets whether the specified comment block should be rendered or not.
+ *
+ * Args:
+ * commentBlock (RB.AbstractCommentBlock):
+ * The comment block to check the status of.
+ *
+ * Returns:
+ * boolean:
+ * Whether the comment block should be rendered or not.
+ */
+ shouldRenderCommentBlock(commentBlock) {
+ return true;
+ },
+
/**
* Add a CommentBlockView for the given CommentBlock.
*
@@ -139,12 +182,16 @@ RB.AbstractReviewableView = Backbone.View.extend({
* The comment block to add a view for.
*/
_addCommentBlockView(commentBlock) {
+ if (!this.shouldRenderCommentBlock(commentBlock)) {
+ return;
+ }
const commentBlockView = new this.commentBlockView({
model: commentBlock
});
commentBlockView.on('clicked', () => this.showCommentDlg(commentBlockView));
commentBlockView.render();
+ this.commentBlockViews.push(commentBlockView);
this.trigger('commentBlockViewAdded', commentBlockView);
},
-});
+});
\ No newline at end of file
diff --git a/reviewboard/static/rb/js/views/tests/textBasedReviewableViewTests.es6.js b/reviewboard/static/rb/js/views/tests/textBasedReviewableViewTests.es6.js
index 33324fd6b4c8deda983d9a5412d0f33c06fe6989..750fa80b1ab35a1bf3cfd66c87c66659c6f4c7ce 100644
--- a/reviewboard/static/rb/js/views/tests/textBasedReviewableViewTests.es6.js
+++ b/reviewboard/static/rb/js/views/tests/textBasedReviewableViewTests.es6.js
@@ -11,9 +11,19 @@ suite('rb/views/TextBasedReviewableView', function() {