diff --git a/reviewboard/reviews/templatetags/reviewtags.py b/reviewboard/reviews/templatetags/reviewtags.py
index e56e5a11ac77ddf3b5c72189e8a3c655a6bf5bc7..0087d755af0fec84e8d24edd81eeb59b3ba2d087 100644
--- a/reviewboard/reviews/templatetags/reviewtags.py
+++ b/reviewboard/reviews/templatetags/reviewtags.py
@@ -245,7 +245,7 @@ def reply_list(context, entry, comment, context_type, context_id):
The ``context_id`` parameter has to do with the internal IDs used by
the JavaScript code for storing and categorizing the comments.
"""
- def generate_reply_html(reply, timestamp, text):
+ def generate_reply_html(reply, timestamp, text, comment_id=None):
new_context = context
new_context.update({
'context_id': context_id,
@@ -254,7 +254,8 @@ def reply_list(context, entry, comment, context_type, context_id):
'timestamp': timestamp,
'text': text,
'reply_user': reply.user,
- 'draft': not reply.public
+ 'draft': not reply.public,
+ 'comment_id': comment_id,
})
return render_to_string('reviews/review_reply.html', new_context)
@@ -284,7 +285,8 @@ def reply_list(context, entry, comment, context_type, context_id):
for reply_comment in comment.public_replies(user):
s += generate_reply_html(reply_comment.get_review(),
reply_comment.timestamp,
- reply_comment.text)
+ reply_comment.text,
+ reply_comment.pk)
elif context_type == "body_top" or context_type == "body_bottom":
replies = getattr(review, "public_%s_replies" % context_type)()
diff --git a/reviewboard/static/rb/js/models/reviewReplyModel.js b/reviewboard/static/rb/js/models/reviewReplyModel.js
index 33b7dc6fdf5b863c84f8b4c44f4104451f978abb..4dd1092fe10b17937ddca2777ffd9866af23f8a5 100644
--- a/reviewboard/static/rb/js/models/reviewReplyModel.js
+++ b/reviewboard/static/rb/js/models/reviewReplyModel.js
@@ -14,6 +14,12 @@ RB.ReviewReply = RB.BaseResource.extend({
rspNamespace: 'reply',
listKey: 'replies',
+ COMMENT_LINK_NAMES: [
+ 'diff_comments',
+ 'file_attachment_comments',
+ 'screenshot_comments'
+ ],
+
toJSON: function() {
return {
'public': this.get('public'),
@@ -31,6 +37,75 @@ RB.ReviewReply = RB.BaseResource.extend({
result.public = rspData.public;
return result;
+ },
+
+ /*
+ * Discards the reply if it's empty.
+ *
+ * If the reply doesn't have any remaining comments on the server, then
+ * this will discard the reply.
+ *
+ * When we've finished checking, options.success will be called. It
+ * will be passed true if discarded, or false otherwise.
+ */
+ discardIfEmpty: function(options, context) {
+ options = _.bindCallbacks(options || {}, context);
+ options.success = options.success || function() {};
+
+ this.ready({
+ ready: function() {
+ if (this.isNew() ||
+ this.get('bodyTop') ||
+ this.get('bodyBottom')) {
+ options.success(false);
+ return;
+ }
+
+ this._checkCommentsLink(0, options, context);
+ },
+
+ error: options.error
+ }, this);
+ },
+
+ /*
+ * Checks if there are comments, given the comment type.
+ *
+ * This is part of the discardIfEmpty logic.
+ *
+ * If there are comments, we'll give up and call options.success(false).
+ *
+ * If there are no comments, we'll move on to the next comment type. If
+ * we're done, the reply is discarded, and options.success(true) is called.
+ */
+ _checkCommentsLink: function(linkNameIndex, options, context) {
+ var self = this,
+ linkName = this.COMMENT_LINK_NAMES[linkNameIndex],
+ url = this.get('links')[linkName].href;
+
+ RB.apiCall({
+ type: 'GET',
+ url: url,
+ success: function(rsp) {
+ if (rsp[linkName].length > 0) {
+ if (options.success) {
+ options.success(false);
+ }
+ } else if (linkNameIndex < self.COMMENT_LINK_NAMES.length - 1) {
+ self._checkCommentsLink(linkNameIndex + 1, options,
+ context);
+ } else {
+ self.destroy(
+ _.defaults({
+ success: function() {
+ options.success(true);
+ }
+ }, options),
+ context);
+ }
+ },
+ error: options.error
+ });
}
});
_.extend(RB.ReviewReply.prototype, RB.DraftResourceModelMixin);
diff --git a/reviewboard/static/rb/js/models/tests/reviewReplyModelTests.js b/reviewboard/static/rb/js/models/tests/reviewReplyModelTests.js
index 90ed2e3aba66d9cd166968e99d0423c7161da9bd..17d7300283643029f539d44b9932b5b46e7b5c59 100644
--- a/reviewboard/static/rb/js/models/tests/reviewReplyModelTests.js
+++ b/reviewboard/static/rb/js/models/tests/reviewReplyModelTests.js
@@ -92,6 +92,157 @@ describe('models/ReviewReply', function() {
});
});
+ describe('discardIfEmpty', function() {
+ var callbacks;
+
+ beforeEach(function() {
+ callbacks = {
+ success: function() {},
+ error: function() {}
+ };
+
+ spyOn(model, 'destroy')
+ .andCallFake(function(options) {
+ if (options && _.isFunction(options.success)) {
+ options.success();
+ }
+ });
+ spyOn(parentObject, 'ready')
+ .andCallFake(function(options, context) {
+ if (options && _.isFunction(options.ready)) {
+ options.ready.call(context);
+ }
+ });
+ spyOn(model, 'ready')
+ .andCallFake(function(options, context) {
+ if (options && _.isFunction(options.ready)) {
+ options.ready.call(context);
+ }
+ });
+ spyOn(callbacks, 'success');
+ spyOn(callbacks, 'error');
+ });
+
+ it('With isNew=true', function() {
+ expect(model.isNew()).toBe(true);
+ expect(model.get('loaded')).toBe(false);
+
+ model.discardIfEmpty(callbacks);
+
+ expect(model.destroy).not.toHaveBeenCalled();
+ });
+
+ describe('With isNew=false', function() {
+ var commentsData = {};
+
+ beforeEach(function() {
+ model.set({
+ id: 123,
+ loaded: true,
+ links: {
+ self: {
+ href: '/api/foos/replies/123/'
+ },
+ diff_comments: {
+ href: '/api/diff-comments/'
+ },
+ screenshot_comments: {
+ href: '/api/screenshot-comments/'
+ },
+ file_attachment_comments: {
+ href: '/api/file-attachment-comments/'
+ }
+ }
+ });
+
+ spyOn(RB, 'apiCall').andCallFake(function(options) {
+ var links = model.get('links'),
+ data = {},
+ key = _.find(
+ RB.ReviewReply.prototype.COMMENT_LINK_NAMES,
+ function(name) {
+ return options.url === links[name].href;
+ });
+
+ if (key) {
+ data[key] = commentsData[key] || [];
+ options.success(data);
+ } else {
+ options.error({
+ status: 404
+ });
+ }
+ });
+ spyOn(Backbone.Model.prototype, 'fetch')
+ .andCallFake(function(options) {
+ if (options && _.isFunction(options.success)) {
+ options.success();
+ }
+ });
+ });
+
+ it('With no comments or body replies', function() {
+ model.discardIfEmpty(callbacks);
+
+ expect(model.destroy).toHaveBeenCalled();
+ expect(callbacks.success).toHaveBeenCalledWith(true);
+ });
+
+ it('With bodyTop', function() {
+ model.set({
+ bodyTop: 'hi'
+ });
+ model.discardIfEmpty(callbacks);
+
+ expect(model.destroy).not.toHaveBeenCalled();
+ expect(callbacks.success).toHaveBeenCalledWith(false);
+ });
+
+ it('With bodyBottom', function() {
+ model.set({
+ bodyBottom: 'hi'
+ });
+ model.discardIfEmpty(callbacks);
+
+ expect(model.destroy).not.toHaveBeenCalled();
+ expect(callbacks.success).toHaveBeenCalledWith(false);
+ });
+
+ it('With diff comment', function() {
+ commentsData['diff_comments'] = [{
+ id: 1
+ }];
+
+ model.discardIfEmpty(callbacks);
+
+ expect(model.destroy).not.toHaveBeenCalled();
+ expect(callbacks.success).toHaveBeenCalledWith(false);
+ });
+
+ it('With screenshot comment', function() {
+ commentsData['screenshot_comments'] = [{
+ id: 1
+ }];
+
+ model.discardIfEmpty(callbacks);
+
+ expect(model.destroy).not.toHaveBeenCalled();
+ expect(callbacks.success).toHaveBeenCalledWith(false);
+ });
+
+ it('With file attachment comment', function() {
+ commentsData['file_attachment_comments'] = [{
+ id: 1
+ }];
+
+ model.discardIfEmpty(callbacks);
+
+ expect(model.destroy).not.toHaveBeenCalled();
+ expect(callbacks.success).toHaveBeenCalledWith(false);
+ });
+ });
+ });
+
describe('ready', function() {
var callbacks;
diff --git a/reviewboard/static/rb/js/reviews.js b/reviewboard/static/rb/js/reviews.js
index f46f65b53ebfdf302113d31b746b4dc5c0e34d4c..fa0b4a31f64ee5076df4bdb485868794d013a3c0 100644
--- a/reviewboard/static/rb/js/reviews.js
+++ b/reviewboard/static/rb/js/reviews.js
@@ -430,9 +430,10 @@ $.fn.commentSection = function(review_id, context_id, context_type) {
*/
function createCommentEditor(els) {
return els.each(function() {
- var self = $(this);
+ var $editor = $(this),
+ $item = $("#" + $editor[0].id + "-item");
- self
+ $editor
.inlineEditor({
cls: "inline-comment-editor",
editIconPath: STATIC_URLS["rb/images/edit.png"],
@@ -444,11 +445,12 @@ $.fn.commentSection = function(review_id, context_id, context_type) {
gEditCount++;
},
"complete": function(e, value) {
- var replyClass;
+ var replyClass,
+ options;
gEditCount--;
- self.html(linkifyText(self.text()));
+ $editor.html(linkifyText($editor.text()));
if (context_type == "body_top") {
review_reply.set('bodyTop', value);
@@ -467,25 +469,40 @@ $.fn.commentSection = function(review_id, context_id, context_type) {
return;
}
- obj = new replyClass({
- parentObject: review_reply,
- replyToID: context_id,
- text: value
- });
+ obj = $item.data('comment-obj');
+
+ if (!obj) {
+ obj = new replyClass({
+ parentObject: review_reply,
+ replyToID: context_id,
+ id: $item.data('comment-id')
+ });
+
+ $item.data('comment-obj', obj);
+ }
}
- obj.save({
- buttons: bannerButtonsEl,
- success: function() {
- removeCommentFormIfEmpty(self);
- showReplyDraftBanner(review_id);
+ obj.ready({
+ ready: function() {
+ if (value) {
+ obj.set('text', value);
+ obj.save({
+ buttons: bannerButtonsEl,
+ success: function() {
+ $item.data('comment-id', obj.id);
+ showReplyDraftBanner(review_id);
+ }
+ });
+ } else {
+ removeCommentFormIfEmpty($item, $editor);
+ }
}
});
},
"cancel": function(e) {
gEditCount--;
addCommentLink.fadeIn();
- removeCommentFormIfEmpty(self);
+ removeCommentFormIfEmpty($item, $editor);
}
})
});
@@ -494,29 +511,54 @@ $.fn.commentSection = function(review_id, context_id, context_type) {
/*
* Removes a comment form if the contents are empty.
*
+ * @param {jQuery} itemEl The comment item element.
* @param {jQuery} editorEl The inline editor element.
*/
- function removeCommentFormIfEmpty(editorEl) {
- var value = editorEl.inlineEditor("value");
+ function removeCommentFormIfEmpty($item, $editor) {
+ var value = $editor.inlineEditor("value"),
+ obj;
if (value.stripTags().strip() != "") {
return;
}
- $("#" + editorEl[0].id + "-item").hide("slow", function() {
- $(this).remove();
+ obj = $item.data('comment-obj');
+ console.assert(obj,
+ 'comment-obj data is not populated for the comment ' +
+ 'editor');
- if ($(".inline-comment-editor", reviewEl).length == 0) {
- bannersEl.children().remove();
- }
+ if (obj.isNew()) {
+ removeCommentForm($item, obj);
+ } else {
+ obj.destroy({
+ success: function() {
+ removeCommentForm($item, obj);
+ }
+ });
+ }
+ }
- addCommentLink.fadeIn();
+ function removeCommentForm($item, obj) {
+ $item
+ .data({
+ 'comment-id': null,
+ 'comment-obj': null
+ })
+ .fadeOut(function() {
+ $(this).remove();
+ addCommentLink.fadeIn();
- /* Find out if we need to discard this. */
- review_reply.discardIfEmpty({
- buttons: bannerButtonsEl
+ /* Find out if we need to discard this. */
+ review_reply.discardIfEmpty({
+ buttons: bannerButtonsEl,
+ success: function(discarded) {
+ if (discarded) {
+ /* The reply was discarded. */
+ bannersEl.children().remove();
+ }
+ }
+ });
});
- });
}
/*
@@ -735,11 +777,15 @@ $.replyDraftBanner = function(review_reply, bannerButtonsEl) {
.append($('')
.val("Publish")
.click(function() {
- review_reply.set('public', true);
- review_reply.save({
- buttons: bannerButtonsEl,
- success: function() {
- window.location = gReviewRequestPath;
+ review_reply.ready({
+ ready: function() {
+ review_reply.set('public', true);
+ review_reply.save({
+ buttons: bannerButtonsEl,
+ success: function() {
+ window.location = gReviewRequestPath;
+ }
+ });
}
});
})
@@ -1707,13 +1753,17 @@ $(document).ready(function() {
/* Review banner's Publish button. */
$("#review-banner-publish").click(function() {
- pendingReview.set('public', true);
- pendingReview.save({
- buttons: $("input", gReviewBanner),
- success: function() {
- hideReviewBanner();
- gReviewBanner.queue(function() {
- window.location = gReviewRequestPath;
+ pendingReview.ready({
+ ready: function() {
+ pendingReview.set('public', true);
+ pendingReview.save({
+ buttons: $("input", gReviewBanner),
+ success: function() {
+ hideReviewBanner();
+ gReviewBanner.queue(function() {
+ window.location = gReviewRequestPath;
+ });
+ }
});
}
});
diff --git a/reviewboard/templates/reviews/review_reply.html b/reviewboard/templates/reviews/review_reply.html
index 69f950c3f3a59ff17eb5f1ae6d4b0ef6ba69bc99..66666f6cb59da199ee1d91f5ff329a1288d23d0f 100644
--- a/reviewboard/templates/reviews/review_reply.html
+++ b/reviewboard/templates/reviews/review_reply.html
@@ -1,7 +1,7 @@
{% load djblets_utils %}
{% load i18n %}
{% load tz %}
-