diff --git a/reviewboard/reviews/markdown_utils.py b/reviewboard/reviews/markdown_utils.py new file mode 100644 index 0000000000000000000000000000000000000000..2a694f97af627ed6bfc9e75ae82c81ea7be4207d --- /dev/null +++ b/reviewboard/reviews/markdown_utils.py @@ -0,0 +1,53 @@ +import re + +from markdown import Markdown + + +ESCAPED_CHARS_RE = \ + re.compile(r'([%s])' % re.escape(''.join(Markdown.ESCAPED_CHARS))) +UNESCAPED_CHARS_RE = \ + re.compile(r'\\([%s])' % re.escape(''.join(Markdown.ESCAPED_CHARS))) + + +def markdown_escape(text): + """Escapes text for use in Markdown. + + This will escape the provided text so that none of the characters will + be rendered specially by Markdown. + """ + return ESCAPED_CHARS_RE.sub(r'\\\1', text) + + +def markdown_unescape(escaped_text): + """Unescapes Markdown-escaped text. + + This will unescape the provided Markdown-formatted text so that any + escaped characters will be unescaped. + """ + return UNESCAPED_CHARS_RE.sub(r'\1', escaped_text) + + +def markdown_escape_field(model, field_name): + """Escapes Markdown text in a model's field. + + This is a convenience around markdown_escape to escape the contents of + a particular field in a model. + """ + setattr(model, field_name, markdown_escape(getattr(model, field_name))) + + +def markdown_unescape_field(model, field_name): + """Unescapes Markdown text in a model's field. + + This is a convenience around markdown_unescape to unescape the contents of + a particular field in a model. + """ + setattr(model, field_name, markdown_unescape(getattr(model, field_name))) + + +def markdown_set_field_escaped(model, field, escaped): + """Escapes or unescapes the specified field in a model.""" + if escaped: + markdown_escape_field(model, field) + else: + markdown_unescape_field(model, field) diff --git a/reviewboard/reviews/models.py b/reviewboard/reviews/models.py index 200a28fbb909325c09a28393e24ce1b464c51090..ca42b8182a5f282cfe8d0a84c5476bbe992c8036 100644 --- a/reviewboard/reviews/models.py +++ b/reviewboard/reviews/models.py @@ -257,7 +257,7 @@ class BaseReviewRequestDetails(models.Model): testing_done = models.TextField(_("testing done"), blank=True) bugs_closed = models.CharField(_("bugs"), max_length=300, blank=True) branch = models.CharField(_("branch"), max_length=300, blank=True) - rich_text = models.BooleanField(_("rich text"), default=True) + rich_text = models.BooleanField(_("rich text"), default=False) def _get_review_request(self): raise NotImplementedError @@ -895,7 +895,7 @@ class ReviewRequest(BaseReviewRequestDetails): def can_publish(self): return not self.public or get_object_or_none(self.draft) is not None - def close(self, type, user=None, description=None): + def close(self, type, user=None, description=None, rich_text=False): """ Closes the review request. The type must be one of SUBMITTED or DISCARDED. @@ -908,7 +908,9 @@ class ReviewRequest(BaseReviewRequestDetails): raise AttributeError("%s is not a valid close type" % type) if self.status != type: - changedesc = ChangeDescription(public=True, text=description or "") + changedesc = ChangeDescription(public=True, + text=description or "", + rich_text=rich_text) changedesc.record_field_change('status', self.status, type) changedesc.save() @@ -1215,10 +1217,11 @@ class ReviewRequestDraft(BaseReviewRequestDetails): 'testing_done': review_request.testing_done, 'bugs_closed': review_request.bugs_closed, 'branch': review_request.branch, + 'rich_text': review_request.rich_text, }) if draft.changedesc is None and review_request.public: - changedesc = ChangeDescription() + changedesc = ChangeDescription(rich_text=draft.rich_text) changedesc.save() draft.changedesc = changedesc @@ -1491,7 +1494,7 @@ class BaseComment(models.Model): verbose_name=_("reply to")) timestamp = models.DateTimeField(_('timestamp'), default=timezone.now) text = models.TextField(_("comment text")) - rich_text = models.BooleanField(_("rich text"), default=True) + rich_text = models.BooleanField(_("rich text"), default=False) extra_data = JSONField(null=True) @@ -1686,7 +1689,8 @@ class FileAttachmentComment(BaseComment): FileAttachment, verbose_name=_('diff against file attachment'), related_name="diffed_against_comments", - null=True) + null=True, + blank=True) @property def thumbnail(self): @@ -1784,7 +1788,7 @@ class Review(models.Model): related_name="review", blank=True) - rich_text = models.BooleanField(_("rich text"), default=True) + rich_text = models.BooleanField(_("rich text"), default=False) # XXX Deprecated. This will be removed in a future release. reviewed_diffset = models.ForeignKey( diff --git a/reviewboard/reviews/tests.py b/reviewboard/reviews/tests.py index 246d726494c1dc95277a721281cd02040686af71..975038467dcafd5f415a4b828b06349c0f72fafc 100644 --- a/reviewboard/reviews/tests.py +++ b/reviewboard/reviews/tests.py @@ -14,6 +14,8 @@ from kgb import SpyAgency from reviewboard.accounts.models import Profile, LocalSiteProfile from reviewboard.attachments.models import FileAttachment from reviewboard.reviews.forms import DefaultReviewerForm, GroupForm +from reviewboard.reviews.markdown_utils import (markdown_escape, + markdown_unescape) from reviewboard.reviews.models import (Comment, DefaultReviewer, Group, @@ -2608,3 +2610,18 @@ class UserInfoboxTests(TestCase): user.save() self.client.get(local_site_reverse('user-infobox', args=['test'])) + + +class MarkdownUtilsTests(TestCase): + UNESCAPED_TEXT = '\\`*_{}[]()>#+-.!' + ESCAPED_TEXT = '\\\\\\`\\*\\_\\{\\}\\[\\]\\(\\)\\>\\#\\+\\-\\.\\!' + + def test_markdown_escape(self): + """Testing markdown_escape""" + self.assertEqual(markdown_escape(self.UNESCAPED_TEXT), + self.ESCAPED_TEXT) + + def test_markdown_unescape(self): + """Testing markdown_unescape""" + self.assertEqual(markdown_unescape(self.ESCAPED_TEXT), + self.UNESCAPED_TEXT) diff --git a/reviewboard/settings.py b/reviewboard/settings.py index ef498bca1537ff455250a5fdd1c35f93cb54c43b..1fd7d238031f6f1280623927edd975d07218a5ae 100644 --- a/reviewboard/settings.py +++ b/reviewboard/settings.py @@ -358,6 +358,7 @@ PIPELINE_JS = dict({ 'rb/js/utils/tests/keyBindingUtilsTests.js', 'rb/js/utils/tests/linkifyUtilsTests.js', 'rb/js/utils/tests/propertyUtilsTests.js', + 'rb/js/utils/tests/textUtilsTests.js', 'rb/js/views/tests/collectionViewTests.js', 'rb/js/views/tests/commentDialogViewTests.js', 'rb/js/views/tests/commentIssueBarViewTests.js', diff --git a/reviewboard/static/rb/js/models/commentEditorModel.js b/reviewboard/static/rb/js/models/commentEditorModel.js index eb8bfcd10435049de14c7dd34dc75b43d6df6764..5033da901eb901201c558e539a4d2e56cd1e055d 100644 --- a/reviewboard/static/rb/js/models/commentEditorModel.js +++ b/reviewboard/static/rb/js/models/commentEditorModel.js @@ -23,6 +23,7 @@ RB.CommentEditor = Backbone.Model.extend({ publishedComments: [], publishedCommentsType: null, reviewRequest: null, + richText: true, statusText: '', text: '' }; @@ -190,7 +191,8 @@ RB.CommentEditor = Backbone.Model.extend({ comment.set({ text: this.get('text'), issueOpened: this.get('openIssue'), - extraData: _.clone(this.get('extraData')) + extraData: _.clone(this.get('extraData')), + richText: this.get('richText') }); comment.save({ diff --git a/reviewboard/static/rb/js/models/reviewReplyEditorModel.js b/reviewboard/static/rb/js/models/reviewReplyEditorModel.js index 8828467b0bd5eeb6a012ee45047f5d98a71be9bf..34b5f89f4b80bc94c0ecf2e7389a96a832fa9ae6 100644 --- a/reviewboard/static/rb/js/models/reviewReplyEditorModel.js +++ b/reviewboard/static/rb/js/models/reviewReplyEditorModel.js @@ -75,6 +75,7 @@ RB.ReviewReplyEditor = Backbone.Model.extend({ if (text) { obj.set(valueAttr, text); + obj.set('richText', true); obj.save({ success: function() { this.set('hasDraft', true); diff --git a/reviewboard/static/rb/js/models/reviewRequestEditorModel.js b/reviewboard/static/rb/js/models/reviewRequestEditorModel.js index e87af87f48243e8ed2fcd3ccb746808fea492e00..9a1b16b81879c5b6fe28d6113048deb44947a9a2 100644 --- a/reviewboard/static/rb/js/models/reviewRequestEditorModel.js +++ b/reviewboard/static/rb/js/models/reviewRequestEditorModel.js @@ -95,6 +95,21 @@ RB.ReviewRequestEditor = Backbone.Model.extend({ return; } + if (!reviewRequest.draft.get('richText') && + (fieldName === 'changeDescription' || + fieldName === 'description' || + fieldName === 'testingDone')) { + /* + * The review request was not previously rich text, but we want + * it to be. + * + * It is expected that the view will, at this point, have converted + * the initial text on the client side to valid Markdown (by + * escaping the text on load). + */ + data.rich_text = true; + } + jsonFieldName = this._jsonFieldMap[fieldName] || fieldName; data[jsonFieldName] = value; diff --git a/reviewboard/static/rb/js/resources/models/baseCommentModel.js b/reviewboard/static/rb/js/resources/models/baseCommentModel.js index 89f907826978f4b342a34f4ad7b85c3dd59daa29..d554bdb46a449b36115cbaa6060a174dfd4edc95 100644 --- a/reviewboard/static/rb/js/resources/models/baseCommentModel.js +++ b/reviewboard/static/rb/js/resources/models/baseCommentModel.js @@ -47,8 +47,9 @@ RB.BaseComment = RB.BaseResource.extend({ */ toJSON: function() { var data = { - text: this.get('text'), - issue_opened: this.get('issueOpened') + issue_opened: this.get('issueOpened'), + rich_text: this.get('richText'), + text: this.get('text') }, parentObject, isPublic; diff --git a/reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js b/reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js index a4619eb2b8c1fb26a46e4a8bcb79248032414974..10bc7a2a91b98137fac1b639f59f94ecb0ebaf8e 100644 --- a/reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js +++ b/reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js @@ -37,7 +37,8 @@ RB.BaseCommentReply = RB.BaseResource.extend({ */ toJSON: function() { var data = { - text: this.get('text') + text: this.get('text'), + rich_text: this.get('richText') }; if (!this.get('loaded')) { diff --git a/reviewboard/static/rb/js/resources/models/reviewModel.js b/reviewboard/static/rb/js/resources/models/reviewModel.js index f1caa5231f7888d8a28d6aa1d825d80620a16967..091521651ca32b5062b76ffd39c0ad0ba826d499 100644 --- a/reviewboard/static/rb/js/resources/models/reviewModel.js +++ b/reviewboard/static/rb/js/resources/models/reviewModel.js @@ -18,7 +18,8 @@ RB.Review = RB.BaseResource.extend({ toJSON: function() { var data = { - ship_it: (this.get('shipIt') ? 1 : 0), + rich_text: this.get('richText'), + ship_it: this.get('shipIt'), body_top: this.get('bodyTop'), body_bottom: this.get('bodyBottom') }; diff --git a/reviewboard/static/rb/js/resources/models/tests/baseCommentModelTests.js b/reviewboard/static/rb/js/resources/models/tests/baseCommentModelTests.js index 7db5769969d48e814a9195b7cf61ebc2d60787de..844fd100ae452965180700474caf7c1c6075778d 100644 --- a/reviewboard/static/rb/js/resources/models/tests/baseCommentModelTests.js +++ b/reviewboard/static/rb/js/resources/models/tests/baseCommentModelTests.js @@ -131,6 +131,16 @@ describe('resources/models/BaseComment', function() { }); }); + describe('richText field', function() { + it('With value', function() { + var data; + + model.set('richText', true); + data = model.toJSON(); + expect(data.rich_text).toBe(true); + }); + }); + describe('text field', function() { it('With value', function() { var data; diff --git a/reviewboard/static/rb/js/resources/models/tests/baseCommentReplyModelTests.js b/reviewboard/static/rb/js/resources/models/tests/baseCommentReplyModelTests.js index 5121384272b987f8990becc5493654e5152e6a47..4ae3f3a6853f48046c2ed046442b187e26bc8689 100644 --- a/reviewboard/static/rb/js/resources/models/tests/baseCommentReplyModelTests.js +++ b/reviewboard/static/rb/js/resources/models/tests/baseCommentReplyModelTests.js @@ -79,6 +79,16 @@ describe('resources/models/BaseCommentReply', function() { }); }); + describe('richText field', function() { + it('With value', function() { + var data; + + model.set('richText', true); + data = model.toJSON(); + expect(data.rich_text).toBe(true); + }); + }); + describe('text field', function() { it('With value', function() { var data; diff --git a/reviewboard/static/rb/js/resources/models/tests/reviewModelTests.js b/reviewboard/static/rb/js/resources/models/tests/reviewModelTests.js index 129508e984ff8df44cd3ba0a0cda5887f536f8df..4d3e602b86055803fa3b9d7061c3ee0207703ba6 100644 --- a/reviewboard/static/rb/js/resources/models/tests/reviewModelTests.js +++ b/reviewboard/static/rb/js/resources/models/tests/reviewModelTests.js @@ -96,13 +96,23 @@ describe('resources/models/Review', function() { }); }); + describe('richText field', function() { + it('With value', function() { + var data; + + model.set('richText', true); + data = model.toJSON(); + expect(data.rich_text).toBe(true); + }); + }); + describe('shipIt field', function() { it('With value', function() { var data; model.set('shipIt', true); data = model.toJSON(); - expect(data.ship_it).toBe(1); + expect(data.ship_it).toBe(true); }); }); }); diff --git a/reviewboard/static/rb/js/utils/tests/textUtilsTests.js b/reviewboard/static/rb/js/utils/tests/textUtilsTests.js new file mode 100644 index 0000000000000000000000000000000000000000..a3f54499672ef51de74d8bd25190d30cd2f04769 --- /dev/null +++ b/reviewboard/static/rb/js/utils/tests/textUtilsTests.js @@ -0,0 +1,6 @@ +describe('utils/textUtils', function() { + it('escapeMarkDown', function() { + expect(RB.escapeMarkdown('hello \\`*_{}[]()>#+-.! world.')).toBe( + 'hello \\\\\\`\\*\\_\\{\\}\\[\\]\\(\\)\\>\\#\\+\\-\\.\\! world\\.'); + }); +}); diff --git a/reviewboard/static/rb/js/utils/textUtils.js b/reviewboard/static/rb/js/utils/textUtils.js index 0741162c72ec85324d1715f875c9a02c89bf88f9..b7dd20fd22986867e78243a75e127326b5f06ea1 100644 --- a/reviewboard/static/rb/js/utils/textUtils.js +++ b/reviewboard/static/rb/js/utils/textUtils.js @@ -1,6 +1,9 @@ (function() { +var ESCAPED_CHARS_RE = /([\\`\*_\{\}\[\]\(\)\>\#\+\-\.\!])/g; + + // If `marked` is defined, initialize it with our preferred options if (marked !== undefined) { marked.setOptions({ @@ -23,13 +26,27 @@ if (marked !== undefined) { * Format the given text and put it into $el. * * If the given element is expected to be rich text, this will format the text - * using markdown. If not, it will add links to review requests and bug - * trackers but otherwise leave the text alone. + * using Markdown. + * + * If it's not expected to be rich text, but we want to force conversion to + * rich text, this will escape the text and turn it into valid Markdown. + * + * Otherwise, if it's not expected and won't be converted, then it will add + * links to review requests and bug trackers but otherwise leave the text alone. */ -RB.formatText = function($el, text, bugTrackerURL) { - var markedUp = text; +RB.formatText = function($el, text, bugTrackerURL, options) { + var markedUp; + elRichText = $el.data('rich-text'); + + if (options && options.forceRichText && !elRichText) { + text = RB.escapeMarkdown(text); + $el.data('rich-text', true); + elRichText = true; + } - if ($el.data('rich-text')) { + markedUp = text; + + if (elRichText) { /* * If there's an inline editor attached to this element, set up some * options first. Primarily, we need to pass in the raw value of the @@ -58,11 +75,22 @@ RB.formatText = function($el, text, bugTrackerURL) { $el .empty() .append(markedUp) - .addClass('rich-text'); + .addClass('rich-text') + .find('a') + .attr('target', '_blank'); } else { $el.html(RB.LinkifyUtils.linkifyText(text, bugTrackerURL)); } }; +/* + * Escapes text, turning it into valid Markdown, without causing any existing + * characters to be interpreted as Markdown. + */ +RB.escapeMarkdown = function(text) { + return text.replace(ESCAPED_CHARS_RE, '\\$1'); +} + + }()); diff --git a/reviewboard/static/rb/js/views/reviewBoxView.js b/reviewboard/static/rb/js/views/reviewBoxView.js index 48d0777665a665e737321925fdff3330d53b9924..366456128b3c05f84a13f8df676157e1de9b0af7 100644 --- a/reviewboard/static/rb/js/views/reviewBoxView.js +++ b/reviewboard/static/rb/js/views/reviewBoxView.js @@ -92,7 +92,9 @@ RB.ReviewBoxView = RB.CollapsableBoxView.extend({ this.$('pre.reviewtext').each(function() { var $el = $(this); - RB.formatText($el, $el.text(), bugTrackerURL); + RB.formatText($el, $el.text(), bugTrackerURL, { + forceRichText: true + }); }); }, diff --git a/reviewboard/static/rb/js/views/reviewDialogView.js b/reviewboard/static/rb/js/views/reviewDialogView.js index 3ee0cbeac38fba54330b60a79d80158d1afa9655..4fd917d0737a3c355fa63dece2b69faa6682a478 100644 --- a/reviewboard/static/rb/js/views/reviewDialogView.js +++ b/reviewboard/static/rb/js/views/reviewDialogView.js @@ -71,6 +71,7 @@ BaseCommentView = Backbone.View.extend({ save: function(options) { this.model.set({ issueOpened: this.$issueOpened.prop('checked'), + richText: true, text: this.$textarea.val() }); this.model.save(options); @@ -80,7 +81,8 @@ BaseCommentView = Backbone.View.extend({ * Renders the comment view. */ render: function() { - var $editFields; + var $editFields, + text = this.model.get('text'); this.$el .append(this.renderThumbnail()) @@ -90,11 +92,21 @@ BaseCommentView = Backbone.View.extend({ openAnIssueText: gettext('Open an issue') }))); - this.$textarea = this.$('textarea') - .text(this.model.get('text')); + this.$textarea = this.$('textarea'); this.$issueOpened = this.$('.issue-opened') .prop('checked', this.model.get('issueOpened')); + if (!this.model.get('richText')) { + /* + * If this comment is modified and saved, it'll be saved as + * Markdown. Escape it so that nothing currently there is + * unintentionally interpreted as Markdown later. + */ + text = RB.escapeMarkdown(text); + } + + this.$textarea.text(text); + $editFields = this.$('.edit-fields'); RB.ReviewDialogCommentHook.each(function(hook) { @@ -364,14 +376,29 @@ RB.ReviewDialogView = Backbone.View.extend({ this.model.ready({ ready: function() { + var bodyBottom, + bodyTop; + this._renderDialog(); if (this.model.isNew()) { this._$spinner.remove(); this._$spinner = null; } else { - this._$bodyBottom.text(this.model.get('bodyBottom') || ''); - this._$bodyTop.text(this.model.get('bodyTop') || ''); + bodyBottom = this.model.get('bodyBottom') || ''; + bodyTop = this.model.get('bodyTop') || ''; + + if (!this.model.get('richText')) { + /* + * When saving, these will convert to Markdown, + * so escape them before-hand. + */ + bodyBottom = RB.escapeMarkdown(bodyBottom); + bodyTop = RB.escapeMarkdown(bodyTop); + } + + this._$bodyBottom.text(bodyBottom); + this._$bodyTop.text(bodyTop); this._$shipIt.prop('checked', this.model.get('shipIt')); this._loadComments(); @@ -567,7 +594,8 @@ RB.ReviewDialogView = Backbone.View.extend({ shipIt: this._$shipIt.prop('checked'), bodyTop: this._$bodyTop.val(), bodyBottom: this._$bodyBottom.val(), - public: publish + public: publish, + richText: true }); this.model.save({ diff --git a/reviewboard/static/rb/js/views/reviewReplyEditorView.js b/reviewboard/static/rb/js/views/reviewReplyEditorView.js index 6490bce1760988387704c1094097f0926943aaf2..45bd80979f86d49c4c88ddeaf2160c48ad235c5f 100644 --- a/reviewboard/static/rb/js/views/reviewReplyEditorView.js +++ b/reviewboard/static/rb/js/views/reviewReplyEditorView.js @@ -68,7 +68,10 @@ RB.ReviewReplyEditorView = Backbone.View.extend({ var reviewRequest = this.model.get('review').get('parentObject'); if (this._$editor) { - RB.formatText(this._$editor, text, reviewRequest.get('bugTrackerURL')); + RB.formatText(this._$editor, text, + reviewRequest.get('bugTrackerURL'), { + forceRichText: true + }); } }, this); diff --git a/reviewboard/static/rb/js/views/reviewRequestEditorView.js b/reviewboard/static/rb/js/views/reviewRequestEditorView.js index fadbaaeea592d65ca068cb175096487202fd843d..664c3d11c34c2fc569682c2593f8eb90b7823796 100644 --- a/reviewboard/static/rb/js/views/reviewRequestEditorView.js +++ b/reviewboard/static/rb/js/views/reviewRequestEditorView.js @@ -359,7 +359,9 @@ RB.ReviewRequestEditorView = Backbone.View.extend({ formatText: function($el, text) { var reviewRequest = this.model.get('reviewRequest'); - RB.formatText($el, text || '', reviewRequest.get('bugTrackerURL')); + RB.formatText($el, text || '', reviewRequest.get('bugTrackerURL'), { + forceRichText: true + }); }, /* diff --git a/reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js b/reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js index e6186a93a86df052c24f14dad4dda42674531274..cc1a8a197bc788b2b50d80a3b204689269a09758 100644 --- a/reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js +++ b/reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js @@ -391,6 +391,26 @@ describe('views/ReviewRequestEditorView', function() { expect(reviewRequest.close).toHaveBeenCalled(); }); + describe('Formatting', function() { + it('Links', function() { + reviewRequest.draft.set('changeDescription', + 'Testing /r/123'); + + expect(view.$('#changedescription').html()).toBe( + '

Testing ' + + '/r/123

'); + }); + + it('Markdown', function() { + reviewRequest.draft.set('changeDescription', + '`This` is a **test**'); + + expect(view.$('#changedescription').html()).toBe( + '

This is a ' + + 'test

'); + }); + }); + editCountTests(); } @@ -430,11 +450,22 @@ describe('views/ReviewRequestEditorView', function() { hasEditorTest(); savingTest(); - it('Formatting', function() { - reviewRequest.draft.set('description', 'Testing /r/123'); + describe('Formatting', function() { + it('Links', function() { + reviewRequest.draft.set('description', 'Testing /r/123'); + + expect(view.$('#description').html()).toBe( + '

Testing ' + + '/r/123

'); + }); + + it('Markdown', function() { + reviewRequest.draft.set('description', + '`This` is a **test**'); - expect(view.$('#description').html()).toBe( - 'Testing /r/123'); + expect(view.$('#description').html()).toBe( + '

This is a test

'); + }); }); editCountTests(); @@ -460,11 +491,22 @@ describe('views/ReviewRequestEditorView', function() { hasEditorTest(); savingTest(); - it('Formatting', function() { - reviewRequest.draft.set('testingDone', 'Testing /r/123'); + describe('Formatting', function() { + it('Links', function() { + reviewRequest.draft.set('testingDone', 'Testing /r/123'); + + expect(view.$('#testing_done').html()).toBe( + '

Testing ' + + '/r/123

'); + }); + + it('Markdown', function() { + reviewRequest.draft.set('testingDone', + '`This` is a **test**'); - expect(view.$('#testing_done').html()).toBe( - 'Testing /r/123'); + expect(view.$('#testing_done').html()).toBe( + '

This is a test

'); + }); }); editCountTests(); diff --git a/reviewboard/testing/testcase.py b/reviewboard/testing/testcase.py index 1d2c05ecce912b0ef16851e5609dbcba894d7bc7..a339c6cbcb43960c569584fd7c4caebae5c11831 100644 --- a/reviewboard/testing/testcase.py +++ b/reviewboard/testing/testcase.py @@ -95,7 +95,7 @@ class TestCase(DjbletsTestCase): def create_diff_comment(self, review, filediff, interfilediff=None, text='My comment', issue_opened=False, first_line=1, num_lines=5, extra_fields=None, - reply_to=None): + reply_to=None, **kwargs): """Creates a Comment for testing. The comment is tied to the given Review and FileDiff (and, optionally, @@ -115,7 +115,8 @@ class TestCase(DjbletsTestCase): text=text, issue_opened=issue_opened, issue_status=issue_status, - reply_to=reply_to) + reply_to=reply_to, + **kwargs) if extra_fields: comment.extra_data = extra_fields @@ -384,7 +385,7 @@ class TestCase(DjbletsTestCase): def create_screenshot_comment(self, review, screenshot, text='My comment', x=1, y=1, w=5, h=5, issue_opened=False, - extra_fields=None, reply_to=None): + extra_fields=None, reply_to=None, **kwargs): """Creates a ScreenshotComment for testing. The comment is tied to the given Review and Screenshot. It's @@ -404,7 +405,8 @@ class TestCase(DjbletsTestCase): h=h, issue_opened=issue_opened, issue_status=issue_status, - reply_to=reply_to) + reply_to=reply_to, + **kwargs) if extra_fields: comment.extra_data = extra_fields diff --git a/reviewboard/webapi/resources/base_comment.py b/reviewboard/webapi/resources/base_comment.py index 21f566ee63fe9272591fdd92cfcee1d85be7cfd5..3df6df88f5c42e26819426dbf13c7fd82c4a2d0c 100644 --- a/reviewboard/webapi/resources/base_comment.py +++ b/reviewboard/webapi/resources/base_comment.py @@ -2,6 +2,7 @@ from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from django.utils.formats import localize from djblets.webapi.errors import DOES_NOT_EXIST +from reviewboard.reviews.markdown_utils import markdown_set_field_escaped from reviewboard.reviews.models import BaseComment from reviewboard.webapi.base import WebAPIResource from reviewboard.webapi.resources import resources @@ -66,11 +67,12 @@ class BaseCommentResource(WebAPIResource): def has_delete_permissions(self, request, obj, *args, **kwargs): return obj.is_mutable_by(request.user) - def create_comment(self, fields, text, issue_opened=False, extra_fields={}, - **kwargs): + def create_comment(self, review, fields, text, issue_opened=False, + rich_text=False, extra_fields={}, **kwargs): comment_kwargs = { - 'text': text.strip(), 'issue_opened': bool(issue_opened), + 'rich_text': rich_text, + 'text': text.strip(), } for field in fields: @@ -89,25 +91,42 @@ class BaseCommentResource(WebAPIResource): return new_comment def update_comment(self, comment, update_fields=(), extra_fields={}, + is_reply=False, **kwargs): - # If we've updated the comment from having no issue opened, - # to having an issue opened, we need to set the issue status - # to OPEN. - if not comment.issue_opened and kwargs.get('issue_opened', False): - comment.issue_status = BaseComment.OPEN - - # If we've updated the comment from having an issue opened to having - # no issue opened, set the issue status back to null. - if comment.issue_opened and not kwargs.get('issue_opened', True): - comment.issue_status = None - - for field in ('text', 'issue_opened') + update_fields: + if not is_reply: + # If we've updated the comment from having no issue opened, + # to having an issue opened, we need to set the issue status + # to OPEN. + if not comment.issue_opened and kwargs.get('issue_opened', False): + comment.issue_status = BaseComment.OPEN + + # If we've updated the comment from having an issue opened to + # having no issue opened, set the issue status back to null. + if comment.issue_opened and not kwargs.get('issue_opened', True): + comment.issue_status = None + + old_rich_text = comment.rich_text + + for field in ('text', 'issue_opened', 'rich_text') + update_fields: value = kwargs.get(field, None) if value is not None: + if isinstance(value, basestring): + value = value.strip() + setattr(comment, field, value) - self._import_extra_data(comment.extra_data, extra_fields) + if 'rich_text' in kwargs: + rich_text = kwargs['rich_text'] + + if rich_text != old_rich_text and 'text' not in kwargs: + # rich_text has been changed, but new comment text has not. + # Escape or unescape the comment text as necessary. + markdown_set_field_escaped(comment, 'text', rich_text) + + if not is_reply: + self._import_extra_data(comment.extra_data, extra_fields) + comment.save() def update_issue_status(self, request, comment_resource, *args, **kwargs): diff --git a/reviewboard/webapi/resources/base_review.py b/reviewboard/webapi/resources/base_review.py index 9e53b655e1520a0bb0b73d4e2212d8f9e3e4f6c1..90bad4f365dcbb71e5f3e6f9a8a07dc3ae07a936 100644 --- a/reviewboard/webapi/resources/base_review.py +++ b/reviewboard/webapi/resources/base_review.py @@ -7,6 +7,7 @@ from djblets.webapi.decorators import (webapi_login_required, from djblets.webapi.errors import (DOES_NOT_EXIST, NOT_LOGGED_IN, PERMISSION_DENIED) +from reviewboard.reviews.markdown_utils import markdown_set_field_escaped from reviewboard.reviews.models import Review from reviewboard.webapi.base import WebAPIResource from reviewboard.webapi.decorators import webapi_check_local_site @@ -110,6 +111,12 @@ class BaseReviewResource(WebAPIResource): 'If a review is public, it cannot be made ' 'private again.', }, + 'rich_text': { + 'type': bool, + 'description': 'Whether the body_top and body_bottom text ' + 'is in rich-text (Markdown) format. ' + 'The default is false.', + }, }, ) def create(self, request, *args, **kwargs): @@ -123,6 +130,9 @@ class BaseReviewResource(WebAPIResource): any number of the fields. If nothing is provided, the review will start off as blank. + If ``rich_text`` is provided and changed to true, then the ``body_top`` + and ``body_bottom`` are expected to be in valid Markdown format. + If the user submitting this review already has a pending draft review on this review request, then this will update the existing draft and return :http:`303`. Otherwise, this will create a new draft and @@ -181,18 +191,30 @@ class BaseReviewResource(WebAPIResource): 'If a review is public, it cannot be made ' 'private again.', }, + 'rich_text': { + 'type': bool, + 'description': 'Whether the body_top and body_bottom text ' + 'is in rich-text (Markdown) format. ' + 'The default is false.', + }, }, ) def update(self, request, *args, **kwargs): - """Updates a review. - - This updates the fields of a draft review. Published reviews cannot - be updated. + """Updates the fields of an unpublished review. Only the owner of a review can make changes. One or more fields can be updated at once. - The only special field is ``public``, which, if set to ``1``, will + If ``rich_text`` is provided and changed to true, then the ``body_top`` + and ``body_bottom`` fields will be set to be interpreted as Markdown. + When setting to true and not specifying one or both of those fields, + the existing text will be escaped so as not to be unintentionally + interpreted as Markdown. + + If ``rich_text`` is changed to false, and one or both of those fields + are not provided, the existing text will be unescaped. + + The only special field is ``public``, which, if set to true, will publish the review. The review will then be made publicly visible. Once public, the review cannot be modified or made private again. """ @@ -236,7 +258,9 @@ class BaseReviewResource(WebAPIResource): # to the user. return self._no_access_error(request.user) - for field in ('ship_it', 'body_top', 'body_bottom'): + old_rich_text = review.rich_text + + for field in ('ship_it', 'body_top', 'body_bottom', 'rich_text'): value = kwargs.get(field, None) if value is not None: @@ -245,6 +269,17 @@ class BaseReviewResource(WebAPIResource): setattr(review, field, value) + if 'rich_text' in kwargs: + rich_text = kwargs['rich_text'] + + if rich_text != old_rich_text: + # rich_text has been changed, but new comment text has not. + # Escape or unescape the comment text as necessary. + for text_field in ('body_top', 'body_bottom'): + if text_field not in kwargs: + markdown_set_field_escaped(review, text_field, + rich_text) + review.save() if public: diff --git a/reviewboard/webapi/resources/review_diff_comment.py b/reviewboard/webapi/resources/review_diff_comment.py index d400552b5fc5d63eb589be4c514ec6599b18663c..b1dd7bdae53bfa5a868930265c1564e5b77b4225 100644 --- a/reviewboard/webapi/resources/review_diff_comment.py +++ b/reviewboard/webapi/resources/review_diff_comment.py @@ -68,6 +68,11 @@ class ReviewDiffCommentResource(BaseDiffCommentResource): 'type': bool, 'description': 'Whether the comment opens an issue.', }, + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, }, allow_unknown=True, ) @@ -77,6 +82,9 @@ class ReviewDiffCommentResource(BaseDiffCommentResource): This will create a new diff comment on this review. The review must be a draft review. + + If ``rich_text`` is provided and set to true, then the the ``text`` + field is expected to be in valid Markdown format. """ try: review_request = \ @@ -119,6 +127,7 @@ class ReviewDiffCommentResource(BaseDiffCommentResource): } new_comment = self.create_comment( + review=review, filediff=filediff, interfilediff=interfilediff, fields=('filediff', 'interfilediff', 'first_line', 'num_lines'), @@ -153,7 +162,12 @@ class ReviewDiffCommentResource(BaseDiffCommentResource): 'issue_status': { 'type': ('dropped', 'open', 'resolved'), 'description': 'The status of an open issue.', - } + }, + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, }, allow_unknown=True, ) @@ -161,6 +175,14 @@ class ReviewDiffCommentResource(BaseDiffCommentResource): """Updates a diff comment. This can update the text or line range of an existing comment. + + If ``rich_text`` is provided and changed to true, then the ``text`` + field will be set to be interpreted as Markdown. When setting to true + and not specifying any new text, the existing text will be escaped so + as not to be unintentionally interpreted as Markdown. + + If ``rich_text`` is changed to false, and new text is not provided, + the existing text will be unescaped. """ try: resources.review_request.get_object(request, *args, **kwargs) diff --git a/reviewboard/webapi/resources/review_file_attachment_comment.py b/reviewboard/webapi/resources/review_file_attachment_comment.py index 2183c22f6b982547c2edb1dcef43cb62a3e25110..f2bdd70ed79b6558fcf13c8448996b9094ab08d3 100644 --- a/reviewboard/webapi/resources/review_file_attachment_comment.py +++ b/reviewboard/webapi/resources/review_file_attachment_comment.py @@ -60,6 +60,11 @@ class ReviewFileAttachmentCommentResource(BaseFileAttachmentCommentResource): 'type': bool, 'description': 'Whether the comment opens an issue.', }, + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, }, allow_unknown=True ) @@ -70,6 +75,9 @@ class ReviewFileAttachmentCommentResource(BaseFileAttachmentCommentResource): This will create a new comment on a file as part of a review. The comment contains text and dimensions for the area being commented on. + + If ``rich_text`` is provided and set to true, then the the ``text`` + field is expected to be in valid Markdown format. """ try: review_request = \ @@ -110,6 +118,7 @@ class ReviewFileAttachmentCommentResource(BaseFileAttachmentCommentResource): } new_comment = self.create_comment( + review=review, file_attachment=file_attachment, diff_against_file_attachment=diff_against_file_attachment, fields=('file_attachment', 'diff_against_file_attachment'), @@ -136,7 +145,12 @@ class ReviewFileAttachmentCommentResource(BaseFileAttachmentCommentResource): 'issue_status': { 'type': ('dropped', 'open', 'resolved'), 'description': 'The status of an open issue.', - } + }, + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, }, allow_unknown=True ) @@ -145,6 +159,14 @@ class ReviewFileAttachmentCommentResource(BaseFileAttachmentCommentResource): This can update the text or region of an existing comment. It can only be done for comments that are part of a draft review. + + If ``rich_text`` is provided and changed to true, then the ``text`` + field will be set to be interpreted as Markdown. When setting to true + and not specifying any new text, the existing text will be escaped so + as not to be unintentionally interpreted as Markdown. + + If ``rich_text`` is changed to false, and new text is not provided, + the existing text will be unescaped. """ try: resources.review_request.get_object(request, *args, **kwargs) diff --git a/reviewboard/webapi/resources/review_reply.py b/reviewboard/webapi/resources/review_reply.py index 54ae78203b45e683bc2fb82c0a53f83df6d37179..3aa4352022183c447f2f1b6b73c44fe5d1d6380a 100644 --- a/reviewboard/webapi/resources/review_reply.py +++ b/reviewboard/webapi/resources/review_reply.py @@ -6,6 +6,7 @@ from djblets.webapi.decorators import (webapi_login_required, from djblets.webapi.errors import (DOES_NOT_EXIST, NOT_LOGGED_IN, PERMISSION_DENIED) +from reviewboard.reviews.markdown_utils import markdown_set_field_escaped from reviewboard.reviews.models import Review from reviewboard.webapi.decorators import webapi_check_local_site @@ -105,6 +106,12 @@ class ReviewReplyResource(BaseReviewResource): 'If a reply is public, it cannot be made ' 'private again.', }, + 'rich_text': { + 'type': bool, + 'description': 'Whether the body_top and body_bottom text ' + 'is in rich-text (Markdown) format. ' + 'The default is false.', + }, }, ) def create(self, request, *args, **kwargs): @@ -118,6 +125,9 @@ class ReviewReplyResource(BaseReviewResource): any number of the fields. If nothing is provided, the reply will start off as blank. + If ``rich_text`` is provided and changed to true, then the ``body_top`` + and ``body_bottom`` are expected to be in valid Markdown format. + If the user submitting this reply already has a pending draft reply on this review, then this will update the existing draft and return :http:`303`. Otherwise, this will create a new draft and @@ -175,6 +185,12 @@ class ReviewReplyResource(BaseReviewResource): 'If a reply is public, it cannot be made ' 'private again.', }, + 'rich_text': { + 'type': bool, + 'description': 'Whether the body_top and body_bottom text ' + 'is in rich-text (Markdown) format. ' + 'The default is false.', + }, }, ) def update(self, request, *args, **kwargs): @@ -186,7 +202,16 @@ class ReviewReplyResource(BaseReviewResource): Only the owner of a reply can make changes. One or more fields can be updated at once. - The only special field is ``public``, which, if set to ``1``, will + If ``rich_text`` is provided and changed to true, then the ``body_top`` + and ``body_bottom`` fields will be set to be interpreted as Markdown. + When setting to true and not specifying one or both of those fields, + the existing text will be escaped so as not to be unintentionally + interpreted as Markdown. + + If ``rich_text`` is changed to false, and one or both of those fields + are not provided, the existing text will be unescaped. + + The only special field is ``public``, which, if set to true, will publish the reply. The reply will then be made publicly visible. Once public, the reply cannot be modified or made private again. """ @@ -223,6 +248,8 @@ class ReviewReplyResource(BaseReviewResource): # to the user. return self._no_access_error(request.user) + old_rich_text = reply.rich_text + for field in ('body_top', 'body_bottom'): value = kwargs.get(field, None) @@ -236,6 +263,19 @@ class ReviewReplyResource(BaseReviewResource): setattr(reply, '%s_reply_to' % field, reply_to) + if 'rich_text' in kwargs: + rich_text = kwargs['rich_text'] + + if rich_text != old_rich_text: + reply.rich_text = rich_text + + # rich_text has been changed, but new comment text has not. + # Escape or unescape the comment text as necessary. + for text_field in ('body_top', 'body_bottom'): + if text_field not in kwargs: + markdown_set_field_escaped(reply, text_field, + rich_text) + if public: reply.publish(user=request.user) else: diff --git a/reviewboard/webapi/resources/review_reply_diff_comment.py b/reviewboard/webapi/resources/review_reply_diff_comment.py index 0525c4bd2b7bac5835debb6f4f07c310f18da7f6..e7dee222d98a366dc523adb04df246ca6e5a6288 100644 --- a/reviewboard/webapi/resources/review_reply_diff_comment.py +++ b/reviewboard/webapi/resources/review_reply_diff_comment.py @@ -58,12 +58,22 @@ class ReviewReplyDiffCommentResource(BaseDiffCommentResource): 'description': 'The comment text.', }, }, + optional={ + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, + } ) - def create(self, request, reply_to_id, text, *args, **kwargs): + def create(self, request, reply_to_id, *args, **kwargs): """Creates a new reply to a diff comment on the parent review. This will create a new diff comment as part of this reply. The reply must be a draft reply. + + If ``rich_text`` is provided and set to true, then the the ``text`` + field is expected to be in valid Markdown format. """ try: resources.review_request.get_object(request, *args, **kwargs) @@ -101,8 +111,7 @@ class ReviewReplyDiffCommentResource(BaseDiffCommentResource): num_lines=comment.num_lines) is_new = True - new_comment.text = text.strip() - new_comment.save() + self.update_comment(new_comment, is_reply=True, **kwargs) data = { self.item_result_key: new_comment, @@ -123,7 +132,12 @@ class ReviewReplyDiffCommentResource(BaseDiffCommentResource): @webapi_login_required @webapi_response_errors(DOES_NOT_EXIST, NOT_LOGGED_IN, PERMISSION_DENIED) @webapi_request_fields( - required={ + optional={ + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, 'text': { 'type': str, 'description': 'The new comment text.', @@ -135,6 +149,14 @@ class ReviewReplyDiffCommentResource(BaseDiffCommentResource): This can only update the text in the comment. The comment being replied to cannot change. + + If ``rich_text`` is provided and changed to true, then the ``text`` + field will be set to be interpreted as Markdown. When setting to true + and not specifying any new text, the existing text will be escaped so + as not to be unintentionally interpreted as Markdown. + + If ``rich_text`` is changed to false, and new text is not provided, + the existing text will be unescaped. """ try: resources.review_request.get_object(request, *args, **kwargs) @@ -146,13 +168,7 @@ class ReviewReplyDiffCommentResource(BaseDiffCommentResource): if not resources.review_reply.has_modify_permissions(request, reply): return self._no_access_error(request.user) - for field in ('text',): - value = kwargs.get(field, None) - - if value is not None: - setattr(diff_comment, field, value) - - diff_comment.save() + self.update_comment(diff_comment, is_reply=True, **kwargs) return 200, { self.item_result_key: diff_comment, diff --git a/reviewboard/webapi/resources/review_reply_file_attachment_comment.py b/reviewboard/webapi/resources/review_reply_file_attachment_comment.py index 5fee6a6bd538f2341617b7342bd7e0411cb7e145..f260b70f706544871f0beecce8854eec27ac7426 100644 --- a/reviewboard/webapi/resources/review_reply_file_attachment_comment.py +++ b/reviewboard/webapi/resources/review_reply_file_attachment_comment.py @@ -61,13 +61,23 @@ class ReviewReplyFileAttachmentCommentResource( 'description': 'The comment text.', }, }, + optional={ + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, + } ) - def create(self, request, reply_to_id, text, *args, **kwargs): + def create(self, request, reply_to_id, *args, **kwargs): """Creates a reply to a file comment on a review. This will create a reply to a file comment on a review. The new comment will contain the same dimensions of the comment being replied to, but may contain new text. + + If ``rich_text`` is provided and set to true, then the the ``text`` + field is expected to be in valid Markdown format. """ try: resources.review_request.get_object(request, *args, **kwargs) @@ -104,8 +114,7 @@ class ReviewReplyFileAttachmentCommentResource( reply_to=comment) is_new = True - new_comment.text = text.strip() - new_comment.save() + self.update_comment(new_comment, is_reply=True, **kwargs) data = { self.item_result_key: new_comment, @@ -126,7 +135,12 @@ class ReviewReplyFileAttachmentCommentResource( @webapi_login_required @webapi_response_errors(DOES_NOT_EXIST, NOT_LOGGED_IN, PERMISSION_DENIED) @webapi_request_fields( - required={ + optional={ + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, 'text': { 'type': str, 'description': 'The new comment text.', @@ -138,6 +152,14 @@ class ReviewReplyFileAttachmentCommentResource( This can only update the text in the comment. The comment being replied to cannot change. + + If ``rich_text`` is provided and changed to true, then the ``text`` + field will be set to be interpreted as Markdown. When setting to true + and not specifying any new text, the existing text will be escaped so + as not to be unintentionally interpreted as Markdown. + + If ``rich_text`` is changed to false, and new text is not provided, + the existing text will be unescaped. """ try: resources.review_request.get_object(request, *args, **kwargs) @@ -149,13 +171,7 @@ class ReviewReplyFileAttachmentCommentResource( if not resources.review_reply.has_modify_permissions(request, reply): return self._no_access_error(request.user) - for field in ('text',): - value = kwargs.get(field, None) - - if value is not None: - setattr(file_comment, field, value) - - file_comment.save() + self.update_comment(file_comment, is_reply=True, **kwargs) return 200, { self.item_result_key: file_comment, diff --git a/reviewboard/webapi/resources/review_reply_screenshot_comment.py b/reviewboard/webapi/resources/review_reply_screenshot_comment.py index 9c75225c1338d63d8cf9579dcb5ef8179de269ca..1747ca04404ecf38e5e8311be88c2d67f24f282c 100644 --- a/reviewboard/webapi/resources/review_reply_screenshot_comment.py +++ b/reviewboard/webapi/resources/review_reply_screenshot_comment.py @@ -59,13 +59,23 @@ class ReviewReplyScreenshotCommentResource(BaseScreenshotCommentResource): 'description': 'The comment text.', }, }, + optional={ + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, + } ) - def create(self, request, reply_to_id, text, *args, **kwargs): + def create(self, request, reply_to_id, *args, **kwargs): """Creates a reply to a screenshot comment on a review. This will create a reply to a screenshot comment on a review. The new comment will contain the same dimensions of the comment being replied to, but may contain new text. + + If ``rich_text`` is provided and set to true, then the the ``text`` + field is expected to be in valid Markdown format. """ try: resources.review_request.get_object(request, *args, **kwargs) @@ -107,8 +117,7 @@ class ReviewReplyScreenshotCommentResource(BaseScreenshotCommentResource): h=comment.h) is_new = True - new_comment.text = text.strip() - new_comment.save() + self.update_comment(new_comment, is_reply=True, **kwargs) data = { self.item_result_key: new_comment, @@ -129,7 +138,12 @@ class ReviewReplyScreenshotCommentResource(BaseScreenshotCommentResource): @webapi_login_required @webapi_response_errors(DOES_NOT_EXIST, NOT_LOGGED_IN, PERMISSION_DENIED) @webapi_request_fields( - required={ + optional={ + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, 'text': { 'type': str, 'description': 'The new comment text.', @@ -141,6 +155,14 @@ class ReviewReplyScreenshotCommentResource(BaseScreenshotCommentResource): This can only update the text in the comment. The comment being replied to cannot change. + + If ``rich_text`` is provided and changed to true, then the ``text`` + field will be set to be interpreted as Markdown. When setting to true + and not specifying any new text, the existing text will be escaped so + as not to be unintentionally interpreted as Markdown. + + If ``rich_text`` is changed to false, and new text is not provided, + the existing text will be unescaped. """ try: resources.review_request.get_object(request, *args, **kwargs) @@ -152,13 +174,7 @@ class ReviewReplyScreenshotCommentResource(BaseScreenshotCommentResource): if not resources.review_reply.has_modify_permissions(request, reply): return self._no_access_error(request.user) - for field in ('text',): - value = kwargs.get(field, None) - - if value is not None: - setattr(screenshot_comment, field, value) - - screenshot_comment.save() + self.update_comment(screenshot_comment, is_reply=True, **kwargs) return 200, { self.item_result_key: screenshot_comment, diff --git a/reviewboard/webapi/resources/review_request_draft.py b/reviewboard/webapi/resources/review_request_draft.py index ca573eb9cf7a1c6a8251c1a5e1685263f4a3c70b..8cb351a8cb95b0161f4848f856db856b78d90d3b 100644 --- a/reviewboard/webapi/resources/review_request_draft.py +++ b/reviewboard/webapi/resources/review_request_draft.py @@ -11,6 +11,7 @@ from djblets.webapi.decorators import (webapi_login_required, from djblets.webapi.errors import (DOES_NOT_EXIST, INVALID_FORM_DATA, NOT_LOGGED_IN, PERMISSION_DENIED) +from reviewboard.reviews.markdown_utils import markdown_set_field_escaped from reviewboard.reviews.models import Group, ReviewRequest, ReviewRequestDraft from reviewboard.webapi.base import WebAPIResource from reviewboard.webapi.decorators import webapi_check_local_site @@ -196,6 +197,13 @@ class ReviewRequestDraftResource(WebAPIResource): 'If a review is public, it cannot be made ' 'private again.', }, + 'rich_text': { + 'type': bool, + 'description': 'Whether or not the review request ' + 'description, testing_done and ' + 'changedescription fields are in rich-text ' + '(Markdown) format.', + }, 'summary': { 'type': str, 'description': 'The new review request summary.', @@ -223,6 +231,16 @@ class ReviewRequestDraftResource(WebAPIResource): All fields from the review request will be copied over to the draft, unless overridden in the request. + + If ``rich_text`` is provided and changed to true, then the + ``changedescription``, ``description`` and ``testing_done`` fields + will be set to be interpreted as Markdown. When setting to true and not + specifying any new text, the existing text from the review request + will be escaped so as not to be unintentionally interpreted as + Markdown. + + If ``rich_text`` is changed to false, and new text is not provided, + the existing text from the review request will be unescaped. """ # A draft is a singleton. Creating and updating it are the same # operations in practice. @@ -266,6 +284,13 @@ class ReviewRequestDraftResource(WebAPIResource): 'review request, and the old draft will be ' 'deleted.', }, + 'rich_text': { + 'type': bool, + 'description': 'Whether or not the review request ' + 'description, testing_done and ' + 'changedescription fields are in rich-text ' + '(Markdown) format.', + }, 'summary': { 'type': str, 'description': 'The new review request summary.', @@ -298,6 +323,15 @@ class ReviewRequestDraftResource(WebAPIResource): review request itself, making it public, and sending out a notification (such as an e-mail) if configured on the server. The current draft will then be deleted. + + If ``rich_text`` is provided and changed to true, then the + ``changedescription``, ``description`` and ``testing_done`` fields + will be set to be interpreted as Markdown. When setting to true and not + specifying any new text, the existing text will be escaped so as not to + be unintentionally interpreted as Markdown. + + If ``rich_text`` is changed to false, and new text is not provided, + the existing text will be unescaped. """ try: review_request = resources.review_request.get_object( @@ -313,6 +347,10 @@ class ReviewRequestDraftResource(WebAPIResource): modified_objects = [] invalid_fields = {} + old_rich_text = draft.rich_text + old_changedesc_rich_text = (draft.changedesc_id is not None and + draft.changedesc.rich_text) + for field_name, field_info in self.fields.iteritems(): if (field_info.get('mutable', True) and kwargs.get(field_name, None) is not None): @@ -326,8 +364,33 @@ class ReviewRequestDraftResource(WebAPIResource): elif field_modified_objects: modified_objects += field_modified_objects + if 'rich_text' in kwargs: + rich_text = kwargs['rich_text'] + + # If the caller has changed the rich_text setting, we will need to + # update any affected fields we already have stored that weren't + # changed in this request by escaping or unescaping their + # contents. + if rich_text != old_rich_text: + for text_field in ('description', 'testing_done'): + if text_field not in kwargs: + markdown_set_field_escaped(draft, text_field, + rich_text) + + if draft.changedesc_id and rich_text != old_changedesc_rich_text: + changedesc = draft.changedesc + changedesc.rich_text = rich_text + + if 'changedescription' not in kwargs: + # The change description's rich_text was not necessarily + # in sync with the draft's, so we're handling all this + # separately. + markdown_set_field_escaped(changedesc, 'text', rich_text) + + modified_objects.append(draft.changedesc) + if always_save or not invalid_fields: - for obj in modified_objects: + for obj in set(modified_objects): obj.save() draft.save() diff --git a/reviewboard/webapi/resources/review_screenshot_comment.py b/reviewboard/webapi/resources/review_screenshot_comment.py index 75c7761f5abea7b62d11592db2bfcd8d60f44f1c..dfb58f506881451df3e865df991eadca1f84c0e4 100644 --- a/reviewboard/webapi/resources/review_screenshot_comment.py +++ b/reviewboard/webapi/resources/review_screenshot_comment.py @@ -65,6 +65,11 @@ class ReviewScreenshotCommentResource(BaseScreenshotCommentResource): 'type': bool, 'description': 'Whether or not the comment opens an issue.', }, + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, }, allow_unknown=True, ) @@ -74,6 +79,9 @@ class ReviewScreenshotCommentResource(BaseScreenshotCommentResource): This will create a new comment on a screenshot as part of a review. The comment contains text and dimensions for the area being commented on. + + If ``rich_text`` is provided and set to true, then the the ``text`` + field is expected to be in valid Markdown format. """ try: review_request = \ @@ -96,6 +104,7 @@ class ReviewScreenshotCommentResource(BaseScreenshotCommentResource): } new_comment = self.create_comment( + review=review, screenshot=screenshot, fields=('screenshot', 'x', 'y', 'w', 'h'), **kwargs) @@ -138,6 +147,11 @@ class ReviewScreenshotCommentResource(BaseScreenshotCommentResource): 'type': ('dropped', 'open', 'resolved'), 'description': 'The status of an open issue.', }, + 'rich_text': { + 'type': bool, + 'description': 'Whether the comment text is in rich-text ' + '(Markdown) format. The default is false.', + }, }, allow_unknown=True ) @@ -146,6 +160,14 @@ class ReviewScreenshotCommentResource(BaseScreenshotCommentResource): This can update the text or region of an existing comment. It can only be done for comments that are part of a draft review. + + If ``rich_text`` is provided and changed to true, then the ``text`` + field will be set to be interpreted as Markdown. When setting to true + and not specifying any new text, the existing text will be escaped so + as not to be unintentionally interpreted as Markdown. + + If ``rich_text`` is changed to false, and new text is not provided, + the existing text will be unescaped. """ try: resources.review_request.get_object(request, *args, **kwargs) diff --git a/reviewboard/webapi/tests/base.py b/reviewboard/webapi/tests/base.py index 20a1018a8d455b4dfc9a7cdd0004bb14d8f07215..9117f3106d9186012f83d9d1f92199db5f25bd32 100644 --- a/reviewboard/webapi/tests/base.py +++ b/reviewboard/webapi/tests/base.py @@ -73,6 +73,9 @@ class BaseWebAPITestCase(TestCase, EmailTestHelper): expected_mimetype, content_type='', extra={}): response = api_func(path, query, follow=follow_redirects, content_type=content_type, extra=extra) + + print "Raw response: %s" % response.content + self.assertEqual(response.status_code, expected_status) if expected_status >= 400: diff --git a/reviewboard/webapi/tests/mixins_comment.py b/reviewboard/webapi/tests/mixins_comment.py new file mode 100644 index 0000000000000000000000000000000000000000..103da7495b9f6fc35cd5b3a0e392a50a41ea8568 --- /dev/null +++ b/reviewboard/webapi/tests/mixins_comment.py @@ -0,0 +1,134 @@ +from reviewboard.webapi.tests.mixins import test_template + + +class BaseCommentListMixin(object): + @test_template + def test_post_with_rich_text_true(self): + """Testing the POST API with rich_text=true""" + self._test_post_with_rich_text(True) + + @test_template + def test_post_with_rich_text_false(self): + """Testing the POST API with rich_text=false""" + self._test_post_with_rich_text(False) + + def _test_post_with_rich_text(self, rich_text): + comment_text = '`This` is a **test**' + + url, mimetype, data, objs = \ + self.setup_basic_post_test(self.user, False, None, True) + data['text'] = comment_text + data['rich_text'] = rich_text + reply = objs[0] + + rsp = self.apiPost(url, data, expected_mimetype=mimetype) + self.assertEqual(rsp['stat'], 'ok') + self.assertTrue(self.resource.item_result_key in rsp) + + comment_rsp = rsp[self.resource.item_result_key] + self.assertEqual(comment_rsp['text'], comment_text) + self.assertEqual(comment_rsp['rich_text'], rich_text) + + comment = self.resource.model.objects.get(pk=comment_rsp['id']) + self.compare_item(comment_rsp, comment) + + +class BaseCommentItemMixin(object): + def compare_item(self, item_rsp, comment): + self.assertEqual(item_rsp['id'], comment.pk) + self.assertEqual(item_rsp['text'], comment.text) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) + + @test_template + def test_put_with_rich_text_true_and_text(self): + """Testing the PUT API with rich_text=true and text specified""" + self._test_put_with_rich_text_and_text(True) + + @test_template + def test_put_with_rich_text_false_and_text(self): + """Testing the PUT API with rich_text=false and text specified""" + self._test_put_with_rich_text_and_text(False) + + @test_template + def test_put_with_rich_text_true_and_not_text(self): + """Testing the PUT API + with rich_text=true and text not specified escapes text + """ + self._test_put_with_rich_text_and_not_text( + True, + '`Test` **diff** comment', + '\\`Test\\` \\*\\*diff\\*\\* comment') + + @test_template + def test_put_with_rich_text_false_and_not_text(self): + """Testing the PUT API + with rich_text=false and text not specified + """ + self._test_put_with_rich_text_and_not_text( + False, + '\\`Test\\` \\*\\*diff\\*\\* comment', + '`Test` **diff** comment') + + def _test_put_with_rich_text_and_text(self, rich_text): + comment_text = '`Test` **diff** comment' + + url, mimetype, data, reply_comment, objs = \ + self.setup_basic_put_test(self.user, False, None, True) + + data['rich_text'] = rich_text + data['text'] = comment_text + + rsp = self.apiPut(url, data, expected_mimetype=mimetype) + + self.assertEqual(rsp['stat'], 'ok') + self.assertTrue(self.resource.item_result_key in rsp) + + comment_rsp = rsp[self.resource.item_result_key] + self.assertEqual(comment_rsp['text'], comment_text) + self.assertEqual(comment_rsp['rich_text'], rich_text) + + comment = self.resource.model.objects.get(pk=comment_rsp['id']) + self.compare_item(comment_rsp, comment) + + def _test_put_with_rich_text_and_not_text(self, rich_text, text, + expected_text): + comment_text = '`Test` **diff** comment' + + url, mimetype, data, reply_comment, objs = \ + self.setup_basic_put_test(self.user, False, None, True) + reply_comment.text = text + reply_comment.rich_text = not rich_text + reply_comment.save() + + data['rich_text'] = rich_text + + if 'text' in data: + del data['text'] + + rsp = self.apiPut(url, data, expected_mimetype=mimetype) + + self.assertEqual(rsp['stat'], 'ok') + self.assertTrue(self.resource.item_result_key in rsp) + + comment_rsp = rsp[self.resource.item_result_key] + self.assertEqual(comment_rsp['text'], expected_text) + self.assertEqual(comment_rsp['rich_text'], rich_text) + + comment = self.resource.model.objects.get(pk=comment_rsp['id']) + self.compare_item(comment_rsp, comment) + + +class CommentListMixin(BaseCommentListMixin): + pass + + +class CommentItemMixin(BaseCommentItemMixin): + pass + + +class CommentReplyListMixin(BaseCommentListMixin): + pass + + +class CommentReplyItemMixin(BaseCommentItemMixin): + pass diff --git a/reviewboard/webapi/tests/mixins_review.py b/reviewboard/webapi/tests/mixins_review.py new file mode 100644 index 0000000000000000000000000000000000000000..361dc72a909194825bfb20f4523ac731e9f6a061 --- /dev/null +++ b/reviewboard/webapi/tests/mixins_review.py @@ -0,0 +1,171 @@ +from reviewboard.webapi.tests.mixins import test_template + + +class ReviewListMixin(object): + @test_template + def test_post_with_rich_text_true(self): + """Testing the POST API with rich_text=true""" + self._test_post_with_rich_text(False) + + @test_template + def test_post_with_rich_text_false(self): + """Testing the POST API with rich_text=false""" + self._test_post_with_rich_text(False) + + def _test_post_with_rich_text(self, rich_text): + body_top = '`This` is **body_top**' + body_bottom = '`This` is **body_bottom**' + + url, mimetype, data, objs = \ + self.setup_basic_post_test(self.user, False, None, True) + review_request = objs[0] + + data['body_top'] = body_top + data['body_bottom'] = body_bottom + data['rich_text'] = rich_text + + rsp = self.apiPost(url, data, expected_mimetype=mimetype) + + self.assertEqual(rsp['stat'], 'ok') + review_rsp = rsp[self.resource.item_result_key] + self.assertEqual(review_rsp['body_top'], body_top) + self.assertEqual(review_rsp['body_bottom'], body_bottom) + self.assertEqual(review_rsp['rich_text'], rich_text) + self.compare_item(review_rsp, + self.resource.model.objects.get(pk=review_rsp['id'])) + + +class ReviewItemMixin(object): + @test_template + def test_put_with_rich_text_true_all_fields(self): + """Testing the PUT API + with rich_text=true and all fields specified + """ + self._test_put_with_rich_text_all_fields(True) + + def test_put_with_rich_text_false_all_fields(self): + """Testing the PUT API + with rich_text=false and all fields specified + """ + self._test_put_with_rich_text_all_fields(False) + + @test_template + def test_put_with_rich_text_true_escaping_all_fields(self): + """Testing the PUT API + with changing rich_text to true and escaping all fields + """ + self._test_put_with_rich_text_escaping_all_fields( + True, + '`This` is **body_top**', + '`This` is **body_bottom**', + '\\`This\\` is \\*\\*body\\_top\\*\\*', + '\\`This\\` is \\*\\*body\\_bottom\\*\\*') + + @test_template + def test_put_with_rich_text_false_escaping_all_fields(self): + """Testing the PUT API + with changing rich_text to false and unescaping all fields + """ + self._test_put_with_rich_text_escaping_all_fields( + False, + '\\`This\\` is \\*\\*body\\_top\\*\\*', + '\\`This\\` is \\*\\*body\\_bottom\\*\\*', + '`This` is **body_top**', + '`This` is **body_bottom**') + + @test_template + def test_put_with_rich_text_true_escaping_unspecified_fields(self): + """Testing the PUT API + with changing rich_text to true and escaping unspecified fields + """ + self._test_put_with_rich_text_escaping_unspecified_fields( + True, + '`This` is **body_top**', + '\\`This\\` is \\*\\*body\\_top\\*\\*') + + @test_template + def test_put_with_rich_text_false_escaping_unspecified_fields(self): + """Testing the PUT API + with changing rich_text to false and unescaping unspecified fields + """ + self._test_put_with_rich_text_escaping_unspecified_fields( + False, + '\\`This\\` is \\*\\*body\\_top\\*\\*', + '`This` is **body_top**') + + def _test_put_with_rich_text_all_fields(self, rich_text): + body_top = '`This` is **body_top**' + body_bottom = '`This` is **body_bottom**' + + url, mimetype, data, review, objs = \ + self.setup_basic_put_test(self.user, False, None, True) + + data.update({ + 'rich_text': rich_text, + 'body_top': body_top, + 'body_bottom': body_bottom, + }) + + rsp = self.apiPut(url, data, expected_mimetype=mimetype) + + self.assertEqual(rsp['stat'], 'ok') + review_rsp = rsp[self.resource.item_result_key] + self.assertEqual(review_rsp['rich_text'], rich_text) + self.assertEqual(review_rsp['body_top'], body_top) + self.assertEqual(review_rsp['body_bottom'], body_bottom) + self.compare_item(review_rsp, + self.resource.model.objects.get(pk=review_rsp['id'])) + + def _test_put_with_rich_text_escaping_all_fields( + self, rich_text, body_top, body_bottom, + expected_body_top, expected_body_bottom): + + url, mimetype, data, review, objs = \ + self.setup_basic_put_test(self.user, False, None, True) + review.rich_text = not rich_text + review.body_top = body_top + review.body_bottom = body_bottom + review.save() + + for field in ('body_top', 'body_bottom'): + if field in data: + del data[field] + + data['rich_text'] = rich_text + + rsp = self.apiPut(url, data, expected_mimetype=mimetype) + + self.assertEqual(rsp['stat'], 'ok') + review_rsp = rsp[self.resource.item_result_key] + self.assertEqual(review_rsp['rich_text'], rich_text) + self.assertEqual(review_rsp['body_top'], expected_body_top) + self.assertEqual(review_rsp['body_bottom'], expected_body_bottom) + self.compare_item(review_rsp, + self.resource.model.objects.get(pk=review_rsp['id'])) + + def _test_put_with_rich_text_escaping_unspecified_fields( + self, rich_text, body_top, expected_body_top): + + body_bottom = '`This` is **body_bottom**' + + url, mimetype, data, review, objs = \ + self.setup_basic_put_test(self.user, False, None, True) + review.rich_text = not rich_text + review.body_top = body_top + review.save() + + data['rich_text'] = rich_text + data['body_bottom'] = body_bottom + + if 'body_top' in data: + del data['body_top'] + + rsp = self.apiPut(url, data, expected_mimetype=mimetype) + + self.assertEqual(rsp['stat'], 'ok') + review_rsp = rsp[self.resource.item_result_key] + self.assertEqual(review_rsp['rich_text'], rich_text) + self.assertEqual(review_rsp['body_top'], expected_body_top) + self.assertEqual(review_rsp['body_bottom'], body_bottom) + self.compare_item(review_rsp, + self.resource.model.objects.get(pk=review_rsp['id'])) diff --git a/reviewboard/webapi/tests/test_review.py b/reviewboard/webapi/tests/test_review.py index b0d2192be763ed2e565423ea3661269e9fc161b3..a49c81320a9ab5b2e435d48e736318111e2ee979 100644 --- a/reviewboard/webapi/tests/test_review.py +++ b/reviewboard/webapi/tests/test_review.py @@ -10,11 +10,14 @@ from reviewboard.webapi.tests.mimetypes import (review_list_mimetype, from reviewboard.webapi.tests.mixins import (BasicTestsMetaclass, ReviewRequestChildItemMixin, ReviewRequestChildListMixin) +from reviewboard.webapi.tests.mixins_review import (ReviewItemMixin, + ReviewListMixin) from reviewboard.webapi.tests.urls import (get_review_item_url, get_review_list_url) -class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): +class ResourceListTests(ReviewListMixin, ReviewRequestChildListMixin, + BaseWebAPITestCase): """Testing the ReviewResource list APIs.""" __metaclass__ = BasicTestsMetaclass @@ -31,6 +34,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): self.assertEqual(item_rsp['ship_it'], review.ship_it) self.assertEqual(item_rsp['body_top'], review.body_top) self.assertEqual(item_rsp['body_bottom'], review.body_bottom) + self.assertEqual(item_rsp['rich_text'], review.rich_text) # # HTTP GET tests @@ -103,10 +107,12 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): def check_post_result(self, user, rsp, review_request): review = Review.objects.get(pk=rsp['review']['id']) + self.assertFalse(review.rich_text) self.compare_item(rsp['review'], review) -class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): +class ResourceItemTests(ReviewItemMixin, ReviewRequestChildItemMixin, + BaseWebAPITestCase): """Testing the ReviewResource item APIs.""" __metaclass__ = BasicTestsMetaclass @@ -125,6 +131,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): self.assertEqual(item_rsp['ship_it'], review.ship_it) self.assertEqual(item_rsp['body_top'], review.body_top) self.assertEqual(item_rsp['body_bottom'], review.body_bottom) + self.assertEqual(item_rsp['rich_text'], review.rich_text) # # HTTP DELETE tests @@ -149,7 +156,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): with pre-published review """ review_request = self.create_review_request(publish=True) - review = self.create_review(review_request, username=self.user, + review = self.create_review(review_request, user=self.user, publish=True) self.apiDelete(get_review_item_url(review_request, review.id), @@ -218,6 +225,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): def check_put_result(self, user, item_rsp, review, *args): self.assertEqual(item_rsp['id'], review.pk) self.assertEqual(item_rsp['body_top'], 'New body top') + self.assertFalse(item_rsp['rich_text']) review = Review.objects.get(pk=review.pk) self.compare_item(item_rsp, review) @@ -227,11 +235,11 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): with pre-published review """ review_request = self.create_review_request(publish=True) - review = self.create_review(review_request, username=self.user, + review = self.create_review(review_request, user=self.user, publish=True) self.apiPut( - get_review_item_url(review.review_request, review.id), + get_review_item_url(review_request, review.id), {'ship_it': True}, expected_status=403) diff --git a/reviewboard/webapi/tests/test_review_comment.py b/reviewboard/webapi/tests/test_review_comment.py index 327020428c57e3e7f9fdb4341a0b6994eb010a79..ed9ea36dc1d5976fda5952211291c329763c08be 100644 --- a/reviewboard/webapi/tests/test_review_comment.py +++ b/reviewboard/webapi/tests/test_review_comment.py @@ -11,6 +11,9 @@ from reviewboard.webapi.tests.mixins import ( BasicTestsMetaclass, ReviewRequestChildItemMixin, ReviewRequestChildListMixin) +from reviewboard.webapi.tests.mixins_comment import ( + CommentItemMixin, + CommentListMixin) from reviewboard.webapi.tests.urls import ( get_review_diff_comment_item_url, get_review_diff_comment_list_url) @@ -72,7 +75,8 @@ class BaseResourceTestCase(BaseWebAPITestCase): return review -class ResourceListTests(ReviewRequestChildListMixin, BaseResourceTestCase): +class ResourceListTests(CommentListMixin, ReviewRequestChildListMixin, + BaseResourceTestCase): """Testing the ReviewDiffCommentResource list APIs.""" __metaclass__ = BasicTestsMetaclass @@ -99,6 +103,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseResourceTestCase): self.assertEqual(item_rsp['issue_opened'], comment.issue_opened) self.assertEqual(item_rsp['first_line'], comment.first_line) self.assertEqual(item_rsp['num_lines'], comment.num_lines) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) # # HTTP GET tests @@ -181,6 +186,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseResourceTestCase): def check_post_result(self, user, rsp, review): comment_rsp = rsp['diff_comment'] self.assertEqual(comment_rsp['text'], 'My new text') + self.assertFalse(comment_rsp['rich_text']) comment = Comment.objects.get(pk=comment_rsp['id']) self.compare_item(comment_rsp, comment) @@ -290,7 +296,8 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseResourceTestCase): extra_fields['extra_data.bar']) -class ResourceItemTests(ReviewRequestChildItemMixin, BaseResourceTestCase): +class ResourceItemTests(CommentItemMixin, ReviewRequestChildItemMixin, + BaseResourceTestCase): """Testing the ReviewDiffCommentResource item APIs.""" __metaclass__ = BasicTestsMetaclass @@ -318,6 +325,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseResourceTestCase): self.assertEqual(item_rsp['issue_opened'], comment.issue_opened) self.assertEqual(item_rsp['first_line'], comment.first_line) self.assertEqual(item_rsp['num_lines'], comment.num_lines) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) # # HTTP DELETE tests @@ -418,6 +426,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseResourceTestCase): def check_put_result(self, user, item_rsp, comment, *args): self.assertEqual(item_rsp['id'], comment.pk) self.assertEqual(item_rsp['text'], 'My new text') + self.assertFalse(item_rsp['rich_text']) self.compare_item(item_rsp, Comment.objects.get(pk=comment.pk)) def test_put_with_issue(self): diff --git a/reviewboard/webapi/tests/test_review_reply.py b/reviewboard/webapi/tests/test_review_reply.py index 68838c7865028592bd4280a94277d2650355d82b..9224d7bfed11403bfee6d7634e98cd8d4bd74b53 100644 --- a/reviewboard/webapi/tests/test_review_reply.py +++ b/reviewboard/webapi/tests/test_review_reply.py @@ -8,6 +8,8 @@ from reviewboard.webapi.tests.mimetypes import (review_reply_item_mimetype, from reviewboard.webapi.tests.mixins import (BasicTestsMetaclass, ReviewRequestChildItemMixin, ReviewRequestChildListMixin) +from reviewboard.webapi.tests.mixins_review import (ReviewItemMixin, + ReviewListMixin) from reviewboard.webapi.tests.urls import (get_review_reply_item_url, get_review_reply_list_url) @@ -26,7 +28,8 @@ class BaseResourceTestCase(BaseWebAPITestCase): return review -class ResourceListTests(ReviewRequestChildListMixin, BaseResourceTestCase): +class ResourceListTests(ReviewListMixin, ReviewRequestChildListMixin, + BaseResourceTestCase): """Testing the ReviewReplyResource list APIs.""" __metaclass__ = BasicTestsMetaclass @@ -44,6 +47,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseResourceTestCase): self.assertEqual(item_rsp['id'], reply.pk) self.assertEqual(item_rsp['body_top'], reply.body_top) self.assertEqual(item_rsp['body_bottom'], reply.body_bottom) + self.assertEqual(item_rsp['rich_text'], reply.rich_text) # # HTTP GET tests @@ -98,6 +102,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseResourceTestCase): def check_post_result(self, user, rsp, review): reply = Review.objects.get(pk=rsp['reply']['id']) + self.assertFalse(reply.rich_text) self.compare_item(rsp['reply'], reply) def test_post_with_body_top(self): @@ -139,7 +144,8 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseResourceTestCase): self.assertEqual(reply.body_bottom, body_bottom) -class ResourceItemTests(ReviewRequestChildItemMixin, BaseResourceTestCase): +class ResourceItemTests(ReviewItemMixin, ReviewRequestChildItemMixin, + BaseResourceTestCase): """Testing the ReviewReplyResource item APIs.""" __metaclass__ = BasicTestsMetaclass @@ -158,6 +164,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseResourceTestCase): self.assertEqual(item_rsp['id'], reply.pk) self.assertEqual(item_rsp['body_top'], reply.body_top) self.assertEqual(item_rsp['body_bottom'], reply.body_bottom) + self.assertEqual(item_rsp['rich_text'], reply.rich_text) # # HTTP DELETE tests @@ -229,6 +236,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseResourceTestCase): def check_put_result(self, user, item_rsp, reply, *args): self.assertEqual(item_rsp['id'], reply.pk) self.assertEqual(item_rsp['body_top'], 'New body top') + self.assertFalse(item_rsp['rich_text']) reply = Review.objects.get(pk=reply.pk) self.compare_item(item_rsp, reply) diff --git a/reviewboard/webapi/tests/test_review_reply_diff_comment.py b/reviewboard/webapi/tests/test_review_reply_diff_comment.py index d4324c0e6845688ee5377e621f48d9965b3e7a2b..64ed7da68f6006e5cbd3b47042046126d783dde6 100644 --- a/reviewboard/webapi/tests/test_review_reply_diff_comment.py +++ b/reviewboard/webapi/tests/test_review_reply_diff_comment.py @@ -8,12 +8,16 @@ from reviewboard.webapi.tests.mixins import ( BasicTestsMetaclass, ReviewRequestChildItemMixin, ReviewRequestChildListMixin) +from reviewboard.webapi.tests.mixins_comment import ( + CommentReplyItemMixin, + CommentReplyListMixin) from reviewboard.webapi.tests.urls import ( get_review_reply_diff_comment_item_url, get_review_reply_diff_comment_list_url) -class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): +class ResourceListTests(CommentReplyListMixin, ReviewRequestChildListMixin, + BaseWebAPITestCase): """Testing the ReviewReplyDiffCommentResource list APIs.""" __metaclass__ = BasicTestsMetaclass @@ -42,6 +46,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): def compare_item(self, item_rsp, comment): self.assertEqual(item_rsp['id'], comment.pk) self.assertEqual(item_rsp['text'], comment.text) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) # # HTTP GET tests @@ -100,6 +105,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): reply_comment = Comment.objects.get(pk=rsp['diff_comment']['id']) self.assertEqual(reply_comment.text, 'Test comment') self.assertEqual(reply_comment.reply_to, comment) + self.assertFalse(reply_comment.rich_text) self.compare_item(rsp['diff_comment'], reply_comment) def test_post_with_http_303(self): @@ -136,7 +142,8 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): self.assertEqual(reply_comment.text, comment_text) -class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): +class ResourceItemTests(CommentReplyItemMixin, ReviewRequestChildItemMixin, + BaseWebAPITestCase): """Testing the ReviewReplyDiffCommentResource item APIs.""" __metaclass__ = BasicTestsMetaclass @@ -165,6 +172,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): def compare_item(self, item_rsp, comment): self.assertEqual(item_rsp['id'], comment.pk) self.assertEqual(item_rsp['text'], comment.text) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) # # HTTP DELETE tests @@ -252,3 +260,4 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): self.assertEqual(item_rsp['id'], comment.pk) self.assertEqual(item_rsp['text'], 'Test comment') self.assertEqual(comment.text, 'Test comment') + self.assertFalse(comment.rich_text) diff --git a/reviewboard/webapi/tests/test_review_reply_file_attachment_comment.py b/reviewboard/webapi/tests/test_review_reply_file_attachment_comment.py index 2673c1d612842e48df3ba3a4723ad6ada346d6cf..2719efc64217226da286252c831f52dad7ee8b01 100644 --- a/reviewboard/webapi/tests/test_review_reply_file_attachment_comment.py +++ b/reviewboard/webapi/tests/test_review_reply_file_attachment_comment.py @@ -8,12 +8,16 @@ from reviewboard.webapi.tests.mixins import ( BasicTestsMetaclass, ReviewRequestChildItemMixin, ReviewRequestChildListMixin) +from reviewboard.webapi.tests.mixins_comment import ( + CommentReplyItemMixin, + CommentReplyListMixin) from reviewboard.webapi.tests.urls import ( get_review_reply_file_attachment_comment_item_url, get_review_reply_file_attachment_comment_list_url) -class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): +class ResourceListTests(CommentReplyListMixin, ReviewRequestChildListMixin, + BaseWebAPITestCase): """Testing the ReviewReplyFileAttachmentCommentResource list APIs.""" __metaclass__ = BasicTestsMetaclass @@ -35,6 +39,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): def compare_item(self, item_rsp, comment): self.assertEqual(item_rsp['id'], comment.pk) self.assertEqual(item_rsp['text'], comment.text) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) # # HTTP GET tests @@ -95,6 +100,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): pk=rsp['file_attachment_comment']['id']) self.assertEqual(reply_comment.text, 'Test comment') self.assertEqual(reply_comment.reply_to, comment) + self.assertFalse(reply_comment.rich_text) self.compare_item(rsp['file_attachment_comment'], reply_comment) def test_post_with_inactive_file_attachment(self): @@ -160,7 +166,8 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): self.check_post_result(self.user, rsp, reply, comment, file_attachment) -class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): +class ResourceItemTests(CommentReplyItemMixin, ReviewRequestChildItemMixin, + BaseWebAPITestCase): """Testing the ReviewReplyFileAttachmentCommentResource item APIs.""" __metaclass__ = BasicTestsMetaclass @@ -186,6 +193,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): def compare_item(self, item_rsp, comment): self.assertEqual(item_rsp['id'], comment.pk) self.assertEqual(item_rsp['text'], comment.text) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) # # HTTP DELETE tests @@ -267,3 +275,4 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): self.assertEqual(item_rsp['id'], comment.pk) self.assertEqual(item_rsp['text'], 'Test comment') self.assertEqual(comment.text, 'Test comment') + self.assertFalse(comment.rich_text) diff --git a/reviewboard/webapi/tests/test_review_reply_screenshot_comment.py b/reviewboard/webapi/tests/test_review_reply_screenshot_comment.py index f329c734c45cf63352ee5431f9d2196cde6c3c50..197f271faddd51069b47fe9cf4dcbf334e8ac93b 100644 --- a/reviewboard/webapi/tests/test_review_reply_screenshot_comment.py +++ b/reviewboard/webapi/tests/test_review_reply_screenshot_comment.py @@ -8,12 +8,16 @@ from reviewboard.webapi.tests.mixins import ( BasicTestsMetaclass, ReviewRequestChildItemMixin, ReviewRequestChildListMixin) +from reviewboard.webapi.tests.mixins_comment import ( + CommentReplyItemMixin, + CommentReplyListMixin) from reviewboard.webapi.tests.urls import ( get_review_reply_screenshot_comment_item_url, get_review_reply_screenshot_comment_list_url) -class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): +class ResourceListTests(CommentReplyListMixin, ReviewRequestChildListMixin, + BaseWebAPITestCase): """Testing the ReviewReplyScreenshotCommentResource list APIs.""" __metaclass__ = BasicTestsMetaclass @@ -35,6 +39,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): def compare_item(self, item_rsp, comment): self.assertEqual(item_rsp['id'], comment.pk) self.assertEqual(item_rsp['text'], comment.text) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) # # HTTP GET tests @@ -93,6 +98,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): ScreenshotComment.objects.get(pk=rsp['screenshot_comment']['id']) self.assertEqual(reply_comment.text, 'Test comment') self.assertEqual(reply_comment.reply_to, comment) + self.assertFalse(reply_comment.rich_text) self.compare_item(rsp['screenshot_comment'], reply_comment) def test_post_with_http_303(self): @@ -128,7 +134,8 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): self.assertEqual(reply_comment.text, comment_text) -class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): +class ResourceItemTests(CommentReplyItemMixin, ReviewRequestChildItemMixin, + BaseWebAPITestCase): """Testing the ReviewReplyScreenshotCommentResource item APIs.""" __metaclass__ = BasicTestsMetaclass @@ -153,6 +160,7 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseWebAPITestCase): def compare_item(self, item_rsp, comment): self.assertEqual(item_rsp['id'], comment.pk) self.assertEqual(item_rsp['text'], comment.text) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) # # HTTP DELETE tests diff --git a/reviewboard/webapi/tests/test_review_request_draft.py b/reviewboard/webapi/tests/test_review_request_draft.py index 0e9a4011e3e44a68902ff4cb22e9bad1dee928dd..f1d6a82fc6a4b9f6ec0ea2df4a08e4411e6e332f 100644 --- a/reviewboard/webapi/tests/test_review_request_draft.py +++ b/reviewboard/webapi/tests/test_review_request_draft.py @@ -21,8 +21,13 @@ class ResourceTests(BaseWebAPITestCase): resource = resources.review_request_draft def compare_item(self, item_rsp, draft): + changedesc = draft.changedesc + self.assertEqual(item_rsp['description'], draft.description) self.assertEqual(item_rsp['testing_done'], draft.testing_done) + self.assertEqual(item_rsp['rich_text'], draft.rich_text) + self.assertEqual(item_rsp['rich_text'], changedesc.rich_text) + self.assertEqual(item_rsp['changedescription'], changedesc.text) # # HTTP DELETE tests @@ -77,6 +82,7 @@ class ResourceTests(BaseWebAPITestCase): def check_post_result(self, user, rsp, review_request): draft = review_request.get_draft() self.assertIsNotNone(draft) + self.assertFalse(draft.rich_text) self.compare_item(rsp['draft'], draft) # @@ -123,6 +129,54 @@ class ResourceTests(BaseWebAPITestCase): self.assertNotEqual(draft.changedesc, None) self.assertEqual(draft.changedesc.text, changedesc) + def test_put_with_rich_text_true_all_fields(self): + """Testing the PUT review-requests//draft/ API + with rich_text=true and all fields specified + """ + self._test_put_with_rich_text_all_fields(True) + + def test_put_with_rich_text_false_all_fields(self): + """Testing the PUT review-requests//draft/ API + with rich_text=false and all fields specified + """ + self._test_put_with_rich_text_all_fields(False) + + def test_put_with_rich_text_true_escaping_all_fields(self): + """Testing the PUT review-requests//draft/ API + with changing rich_text to true and escaping all fields + """ + self._test_put_with_rich_text_escaping_all_fields( + True, + '`This` is a **test**', + '\\`This\\` is a \\*\\*test\\*\\*') + + def test_put_with_rich_text_false_unescaping_all_fields(self): + """Testing the PUT review-requests//draft/ API + with changing rich_text to false and unescaping all fields + """ + self._test_put_with_rich_text_escaping_all_fields( + False, + '\\`This\\` is a \\*\\*test\\*\\*', + '`This` is a **test**') + + def test_put_with_rich_text_true_escaping_unspecified_fields(self): + """Testing the PUT review-requests//draft/ API + with changing rich_text to true and escaping unspecified fields + """ + self._test_put_with_rich_text_escaping_unspecified_fields( + True, + '`This` is a **test**', + '\\`This\\` is a \\*\\*test\\*\\*') + + def test_put_with_rich_text_false_unescaping_unspecified_fields(self): + """Testing the PUT review-requests//draft/ API + with changing rich_text to false and unescaping unspecified fields + """ + self._test_put_with_rich_text_escaping_unspecified_fields( + False, + '\\`This\\` is a \\*\\*test\\*\\*', + '`This` is a **test**') + def test_put_with_depends_on(self): """Testing the PUT review-requests//draft/ API with depends_on field @@ -378,3 +432,97 @@ class ResourceTests(BaseWebAPITestCase): return self._create_update_review_request( apiFunc, expected_status, review_request, self.local_site_name) + + def _test_put_with_rich_text_all_fields(self, rich_text): + text = '`This` is a **test**' + + review_request = self.create_review_request(submitter=self.user, + publish=True) + + rsp = self.apiPut( + get_review_request_draft_url(review_request), + { + 'rich_text': rich_text, + 'changedescription': text, + 'description': text, + 'testing_done': text, + }, + expected_mimetype=review_request_draft_item_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + + draft_rsp = rsp['draft'] + self.assertEqual(draft_rsp['rich_text'], rich_text) + self.assertEqual(draft_rsp['changedescription'], text) + self.assertEqual(draft_rsp['description'], text) + self.assertEqual(draft_rsp['testing_done'], text) + + draft = ReviewRequestDraft.objects.get(pk=rsp['draft']['id']) + self.compare_item(draft_rsp, draft) + + def _test_put_with_rich_text_escaping_all_fields( + self, rich_text, text, expected_text): + + review_request = self.create_review_request(submitter=self.user, + publish=True) + review_request.rich_text = not rich_text + review_request.description = text + review_request.testing_done = text + review_request.save() + + draft = ReviewRequestDraft.create(review_request) + draft.changedesc.text = text + draft.changedesc.save() + + rsp = self.apiPut( + get_review_request_draft_url(review_request), + { + 'rich_text': rich_text, + }, + expected_mimetype=review_request_draft_item_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + + draft_rsp = rsp['draft'] + self.assertEqual(draft_rsp['rich_text'], rich_text) + self.assertEqual(draft_rsp['changedescription'], expected_text) + self.assertEqual(draft_rsp['description'], expected_text) + self.assertEqual(draft_rsp['testing_done'], expected_text) + + draft = ReviewRequestDraft.objects.get(pk=rsp['draft']['id']) + self.compare_item(draft_rsp, draft) + + def _test_put_with_rich_text_escaping_unspecified_fields( + self, rich_text, text, expected_text): + + description = '`This` is the **description**' + + review_request = self.create_review_request(submitter=self.user, + publish=True) + review_request.rich_text = not rich_text + review_request.description = text + review_request.testing_done = text + review_request.save() + + draft = ReviewRequestDraft.create(review_request) + draft.changedesc.text = text + draft.changedesc.save() + + rsp = self.apiPut( + get_review_request_draft_url(review_request), + { + 'rich_text': rich_text, + 'description': description, + }, + expected_mimetype=review_request_draft_item_mimetype) + + self.assertEqual(rsp['stat'], 'ok') + + draft_rsp = rsp['draft'] + self.assertEqual(draft_rsp['rich_text'], rich_text) + self.assertEqual(draft_rsp['changedescription'], expected_text) + self.assertEqual(draft_rsp['description'], description) + self.assertEqual(draft_rsp['testing_done'], expected_text) + + draft = ReviewRequestDraft.objects.get(pk=rsp['draft']['id']) + self.compare_item(draft_rsp, draft) diff --git a/reviewboard/webapi/tests/test_review_screenshot_comment.py b/reviewboard/webapi/tests/test_review_screenshot_comment.py index d228b928709e55237490b8f8b206c1a8c1200b8f..a552940340f67a80c3614c2a315c88cf410779e7 100644 --- a/reviewboard/webapi/tests/test_review_screenshot_comment.py +++ b/reviewboard/webapi/tests/test_review_screenshot_comment.py @@ -11,6 +11,9 @@ from reviewboard.webapi.tests.mixins import ( BasicTestsMetaclass, ReviewRequestChildItemMixin, ReviewRequestChildListMixin) +from reviewboard.webapi.tests.mixins_comment import ( + CommentItemMixin, + CommentListMixin) from reviewboard.webapi.tests.urls import ( get_review_screenshot_comment_item_url, get_review_screenshot_comment_list_url) @@ -44,7 +47,8 @@ class BaseTestCase(BaseWebAPITestCase): return comment, review, review_request -class ResourceListTests(ReviewRequestChildListMixin, BaseTestCase): +class ResourceListTests(CommentListMixin, ReviewRequestChildListMixin, + BaseTestCase): """Testing the ReviewScreenshotCommentResource list APIs.""" __metaclass__ = BasicTestsMetaclass @@ -65,6 +69,7 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseTestCase): self.assertEqual(item_rsp['y'], comment.y) self.assertEqual(item_rsp['w'], comment.w) self.assertEqual(item_rsp['h'], comment.h) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) # # HTTP GET tests @@ -139,32 +144,6 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseTestCase): self.assertEqual(rsp['screenshot_comments'][0]['text'], comment_text) self.assertTrue(rsp['screenshot_comments'][0]['issue_opened']) - -class ResourceItemTests(ReviewRequestChildItemMixin, BaseTestCase): - """Testing the ReviewScreenshotCommentResource item APIs.""" - __metaclass__ = BasicTestsMetaclass - - fixtures = ['test_users'] - sample_api_url = \ - 'review-requests//reviews//screenshot-comments//' - resource = resources.review_screenshot_comment - - def compare_item(self, item_rsp, comment): - self.assertEqual(item_rsp['id'], comment.pk) - self.assertEqual(item_rsp['text'], comment.text) - self.assertEqual(item_rsp['x'], comment.x) - self.assertEqual(item_rsp['y'], comment.y) - self.assertEqual(item_rsp['w'], comment.w) - self.assertEqual(item_rsp['h'], comment.h) - - def setup_review_request_child_test(self, review_request): - screenshot = self.create_screenshot(review_request) - review = self.create_review(review_request, user=self.user) - comment = self.create_screenshot_comment(review, screenshot) - - return (get_review_screenshot_comment_item_url(review, comment.pk), - screenshot_comment_item_mimetype) - def test_post_with_extra_fields(self): """Testing the POST review-requests//reviews//screenshots-comments/ API @@ -210,6 +189,34 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseTestCase): self.assertEqual(comment.extra_data['bar'], extra_fields['extra_data.bar']) + +class ResourceItemTests(CommentItemMixin, ReviewRequestChildItemMixin, + BaseTestCase): + """Testing the ReviewScreenshotCommentResource item APIs.""" + __metaclass__ = BasicTestsMetaclass + + fixtures = ['test_users'] + sample_api_url = \ + 'review-requests//reviews//screenshot-comments//' + resource = resources.review_screenshot_comment + + def compare_item(self, item_rsp, comment): + self.assertEqual(item_rsp['id'], comment.pk) + self.assertEqual(item_rsp['text'], comment.text) + self.assertEqual(item_rsp['x'], comment.x) + self.assertEqual(item_rsp['y'], comment.y) + self.assertEqual(item_rsp['w'], comment.w) + self.assertEqual(item_rsp['h'], comment.h) + self.assertEqual(item_rsp['rich_text'], comment.rich_text) + + def setup_review_request_child_test(self, review_request): + screenshot = self.create_screenshot(review_request) + review = self.create_review(review_request, user=self.user) + comment = self.create_screenshot_comment(review, screenshot) + + return (get_review_screenshot_comment_item_url(review, comment.pk), + screenshot_comment_item_mimetype) + # # HTTP DELETE tests # @@ -287,9 +294,9 @@ class ResourceItemTests(ReviewRequestChildItemMixin, BaseTestCase): def check_put_result(self, user, item_rsp, comment, *args): comment = ScreenshotComment.objects.get(pk=comment.pk) - self.assertEqual(item_rsp['id'], comment.pk) + self.assertFalse(item_rsp['rich_text']) self.assertEqual(item_rsp['text'], 'Test comment') - self.assertEqual(comment.text, 'Test comment') + self.compare_item(item_rsp, comment) def test_put_with_issue(self): """Testing the