diff --git a/Changelog.md b/Changelog.md index c3bd5f787..636b728fa 100644 --- a/Changelog.md +++ b/Changelog.md @@ -20,6 +20,8 @@ * Disable submit button in sign up form after submission to avoid email already exists error [#4506](https://github.com/diaspora/diaspora/issues/4506) * Do not pull the 404 pages assets from Amazon S3 [#4501](https://github.com/diaspora/diaspora/pull/4501) * Fix counter background does not cover more than 2 digits on profile [#4499](https://github.com/diaspora/diaspora/issues/4499) +* Fix commenting upon submission fail [#4005] (https://github.com/diaspora/diaspora/issues/4005) + ## Features * Add oEmbed content to the mobile view [#4343](https://github.com/diaspora/diaspora/pull/4353) diff --git a/app/assets/javascripts/app/collections/comments.js b/app/assets/javascripts/app/collections/comments.js index 0b53b8cf0..d25d85128 100644 --- a/app/assets/javascripts/app/collections/comments.js +++ b/app/assets/javascripts/app/collections/comments.js @@ -13,12 +13,15 @@ app.collections.Comments = Backbone.Collection.extend({ var self = this var comment = new app.models.Comment({text: text }) - , deferred = comment.save({}, {url : self.url()}) - comment.set({author: app.currentUser.toJSON(), parent: self.post }) + var deferred = comment.save({}, { + url : self.url(), + success: function() { + comment.set({author: app.currentUser.toJSON(), parent: self.post }) + self.add(comment) + } + }); - this.add(comment) - - return deferred + return deferred; } }); diff --git a/app/assets/javascripts/app/models/post/interactions.js b/app/assets/javascripts/app/models/post/interactions.js index 1d78da1c9..f4740be81 100644 --- a/app/assets/javascripts/app/models/post/interactions.js +++ b/app/assets/javascripts/app/models/post/interactions.js @@ -82,7 +82,11 @@ app.models.Post.Interactions = Backbone.Model.extend({ var self = this; this.comments.make(text).fail(function () { - alert(Diaspora.I18n.t("failed_to_post_message")); + flash = new Diaspora.Widgets.FlashMessages; + flash.render({ + success: false, + notice: Diaspora.I18n.t("failed_to_post_message") + }); }).done(function() { self.trigger('change') //updates after sync }); diff --git a/spec/javascripts/app/views/comment_stream_view_spec.js b/spec/javascripts/app/views/comment_stream_view_spec.js index b474c5c5c..c5c3ba430 100644 --- a/spec/javascripts/app/views/comment_stream_view_spec.js +++ b/spec/javascripts/app/views/comment_stream_view_spec.js @@ -1,8 +1,8 @@ describe("app.views.CommentStream", function(){ beforeEach(function(){ - this.view = new app.views.CommentStream({model : factory.post()}) - loginAs({}) - }) + this.view = new app.views.CommentStream({model : factory.post()}); + loginAs({}); + }); describe("binds", function() { it("re-renders on a commentsExpanded trigger", function(){ @@ -10,8 +10,8 @@ describe("app.views.CommentStream", function(){ this.view.setupBindings(); this.view.model.trigger("commentsExpanded"); expect(this.view.render).toHaveBeenCalled(); - }) - }) + }); + }); describe("postRenderTemplate", function(){ it("applies infield labels", function(){ @@ -19,41 +19,65 @@ describe("app.views.CommentStream", function(){ this.view.postRenderTemplate() expect($.fn.placeholder).toHaveBeenCalled() expect($.fn.placeholder.mostRecentCall.object.selector).toBe("textarea") - }) + }); it("autoResizes the new comment textarea", function(){ spyOn($.fn, "autoResize") this.view.postRenderTemplate() expect($.fn.autoResize).toHaveBeenCalled() expect($.fn.autoResize.mostRecentCall.object.selector).toBe("textarea") - }) - }) - + }); + }); + describe("createComment", function() { beforeEach(function() { jasmine.Ajax.useMock(); this.view.render(); this.view.expandComments(); - }) - - it("submits the new comment when comment text is not empty", function() { - this.view.$(".comment_box").val('a new comment'); - this.view.createComment(); - expect(this.view.$(".comment-content p").text()).toEqual("a new comment"); - }) - + }); + + context("submission", function() { + beforeEach(function() { + this.view.$(".comment_box").val('a new comment'); + this.view.createComment(); + + this.request = mostRecentAjaxRequest(); + }); + + it("fires an AJAX request", function() { + params = JSON.parse(this.request.params); + // TODO: use this, once jasmine-ajax is updated to latest version + //params = this.request.data(); + + expect(params.text).toEqual("a new comment"); + }); + + it("adds the comment to the view", function() { + this.request.response({status: 200}); + expect(this.view.$(".comment-content p").text()).toEqual("a new comment"); + }); + + it("doesn't add the comment to the view, when the request fails", function(){ + Diaspora.I18n.loadLocale({failed_to_post_message: "posting failed!"}); + this.request.response({status: 500}); + + expect(this.view.$(".comment-content p").text()).not.toEqual("a new comment"); + expect($('*[id^="flash"]')).toBeErrorFlashMessage("posting failed!"); + }); + }); + it("clears the comment box when there are only spaces", function() { this.view.$(".comment_box").val(' '); this.view.createComment(); expect(this.view.$(".comment_box").val()).toEqual(""); - }) - + }); + it("resets comment box height", function() { this.view.$(".comment_box").val('a new comment'); this.view.createComment(); expect(this.view.$(".comment_box").attr("style")).not.toContain("height"); - }) - }) + }); + }); describe("appendComment", function(){ it("appends this.model as 'parent' to the comment", function(){ @@ -62,8 +86,8 @@ describe("app.views.CommentStream", function(){ spyOn(comment, "set") this.view.appendComment(comment) expect(comment.set).toHaveBeenCalled() - }) - }) + }); + }); describe("expandComments", function() { it("refills the comment textbox on success", function() { @@ -78,34 +102,34 @@ describe("app.views.CommentStream", function(){ mostRecentAjaxRequest().response({ comments : [] }); expect(this.view.$("textarea").val()).toEqual("great post!"); - }) - }) - + }); + }); + describe("pressing a key when typing on the new comment box", function(){ it("should not submit the form when enter key is pressed", function(){ this.view.render(); var form = this.view.$("form") var submitCallback = jasmine.createSpy().andReturn(false);form.submit(submitCallback); - + var e = $.Event("keydown", { keyCode: 13 }); e.shiftKey = false; this.view.keyDownOnCommentBox(e); - + expect(submitCallback).not.toHaveBeenCalled(); - }) - + }); + it("should submit the form when enter is pressed with ctrl", function(){ this.view.render(); var form = this.view.$("form") var submitCallback = jasmine.createSpy().andReturn(false); form.submit(submitCallback); - + var e = $.Event("keydown", { keyCode: 13 }); e.ctrlKey = true; this.view.keyDownOnCommentBox(e); - + expect(submitCallback).toHaveBeenCalled(); - }) - }) - -}) + }); + }); + +}); diff --git a/spec/javascripts/helpers/SpecHelper.js b/spec/javascripts/helpers/SpecHelper.js index 56c54e2c9..19cb1dfdd 100644 --- a/spec/javascripts/helpers/SpecHelper.js +++ b/spec/javascripts/helpers/SpecHelper.js @@ -25,7 +25,34 @@ beforeEach(function() { $.extend(Page.prototype, Diaspora.EventBroker.extend(Diaspora.BaseWidget)); Diaspora.page = new Page(); - Diaspora.page.publish("page/ready", [$(document.body)]) + Diaspora.page.publish("page/ready", [$(document.body)]); + + + // matches flash messages with success/error and contained text + var flashMatcher = function(flash, id, text) { + textContained = true; + if( text ) { + textContained = (flash.text().indexOf(text) !== -1); + } + + return flash.is(id) && + flash.hasClass('expose') && + textContained; + }; + + // add custom matchers for flash messages + this.addMatchers({ + toBeSuccessFlashMessage: function(containedText) { + var flash = this.actual; + return flashMatcher(flash, '#flash_notice', containedText); + }, + + toBeErrorFlashMessage: function(containedText) { + var flash = this.actual; + return flashMatcher(flash, '#flash_error', containedText); + } + }); + }); afterEach(function() {