diff --git a/Changelog.md b/Changelog.md index 30df84e6c..7269ddd80 100644 --- a/Changelog.md +++ b/Changelog.md @@ -16,12 +16,15 @@ * Use typeahead.js from rails-assets.org [#7192](https://github.com/diaspora/diaspora/pull/7192) * Refactor ShareVisibilitesController to use PostService [#7196](https://github.com/diaspora/diaspora/pull/7196) * Unify desktop and mobile head elements [#7194](https://github.com/diaspora/diaspora/pull/7194) +* Refactor flash messages on ajax errors for comments, likes, reshares and aspect memberships [#7202](https://github.com/diaspora/diaspora/pull/7202) ## Bug fixes * Fix fetching comments after fetching likes [#7167](https://github.com/diaspora/diaspora/pull/7167) * Hide 'reshare' button on already reshared posts [#7169](https://github.com/diaspora/diaspora/pull/7169) * Only reload profile header when changing aspect memberships [#7183](https://github.com/diaspora/diaspora/pull/7183) * Fix visiblity on invitation modal when opening it from the stream [#7191](https://github.com/diaspora/diaspora/pull/7191) +* Add avatar fallback on tags page [#7198](https://github.com/diaspora/diaspora/pull/7198) +* Update notifications when changing the stream [#7199](https://github.com/diaspora/diaspora/pull/7199) ## Features * Show spinner when loading comments in the stream [#7170](https://github.com/diaspora/diaspora/pull/7170) diff --git a/app/assets/javascripts/app/app.js b/app/assets/javascripts/app/app.js index d300be7e6..d95852882 100644 --- a/app/assets/javascripts/app/app.js +++ b/app/assets/javascripts/app/app.js @@ -115,6 +115,7 @@ var app = { // so we use Backbone.history.navigate instead. var change = Backbone.history.navigate(link.attr("href").substring(1) ,true); if(change === undefined) { Backbone.history.loadUrl(link.attr("href").substring(1)); } + app.notificationsCollection.fetch(); }); }, diff --git a/app/assets/javascripts/app/models/post/interactions.js b/app/assets/javascripts/app/models/post/interactions.js index 12d0a82d6..3615387e2 100644 --- a/app/assets/javascripts/app/models/post/interactions.js +++ b/app/assets/javascripts/app/models/post/interactions.js @@ -72,8 +72,8 @@ app.models.Post.Interactions = Backbone.Model.extend({ self.set({"likes_count" : self.get("likes_count") + 1}); self.likes.trigger("change"); }, - error: function() { - app.flashMessages.error(Diaspora.I18n.t("failed_to_like")); + error: function(model, response) { + app.flashMessages.handleAjaxError(response); } }); @@ -95,8 +95,8 @@ app.models.Post.Interactions = Backbone.Model.extend({ var self = this; options = options || {}; - this.comments.make(text).fail(function () { - app.flashMessages.error(Diaspora.I18n.t("failed_to_comment")); + this.comments.make(text).fail(function(response) { + app.flashMessages.handleAjaxError(response); if (options.error) { options.error(); } }).done(function() { self.post.set({participation: true}); @@ -123,8 +123,8 @@ app.models.Post.Interactions = Backbone.Model.extend({ interactions.set({"reshares_count": interactions.get("reshares_count") + 1}); interactions.reshares.trigger("change"); }) - .fail(function(){ - app.flashMessages.error(Diaspora.I18n.t("reshares.duplicate")); + .fail(function(response) { + app.flashMessages.handleAjaxError(response); }); app.instrument("track", "Reshare"); diff --git a/app/assets/javascripts/app/views/aspect_membership_view.js b/app/assets/javascripts/app/views/aspect_membership_view.js index 238349e1d..34ba8bd77 100644 --- a/app/assets/javascripts/app/views/aspect_membership_view.js +++ b/app/assets/javascripts/app/views/aspect_membership_view.js @@ -125,7 +125,7 @@ app.views.AspectMembership = app.views.Base.extend({ _displayError: function(model, resp) { this._done(); this.dropdown.closest(".aspect_membership_dropdown").removeClass("open"); // close the dropdown - app.flashMessages.error(resp.responseText); + app.flashMessages.handleAjaxError(resp); }, // remove the membership with the given id @@ -134,7 +134,7 @@ app.views.AspectMembership = app.views.Base.extend({ this.listenToOnce(membership, "sync", this._successDestroyCb); this.listenToOnce(membership, "error", this._displayError); - return membership.destroy(); + return membership.destroy({wait: true}); }, _successDestroyCb: function(aspectMembership) { diff --git a/app/assets/javascripts/app/views/flash_messages_view.js b/app/assets/javascripts/app/views/flash_messages_view.js index cb5b214b2..b5c2286d6 100644 --- a/app/assets/javascripts/app/views/flash_messages_view.js +++ b/app/assets/javascripts/app/views/flash_messages_view.js @@ -16,5 +16,13 @@ app.views.FlashMessages = app.views.Base.extend({ error: function(message){ this._flash(message, true); + }, + + handleAjaxError: function(response) { + if (response.status === 0) { + this.error(Diaspora.I18n.t("errors.connection")); + } else { + this.error(response.responseText); + } } }); diff --git a/app/assets/javascripts/app/views/tags_view.js b/app/assets/javascripts/app/views/tags_view.js index 3913f8798..9b70df1e6 100644 --- a/app/assets/javascripts/app/views/tags_view.js +++ b/app/assets/javascripts/app/views/tags_view.js @@ -5,6 +5,8 @@ app.views.Tags = Backbone.View.extend({ if(app.publisher) { app.publisher.setText("#"+ opts.hashtagName + " "); } + // add avatar fallback if it can't be loaded + $(app.views.Base.prototype.avatars.selector).error(app.views.Base.prototype.avatars.fallback); } }); // @license-end diff --git a/app/assets/javascripts/mobile/mobile_post_actions.js b/app/assets/javascripts/mobile/mobile_post_actions.js index 6f7c18ee6..56df0e533 100644 --- a/app/assets/javascripts/mobile/mobile_post_actions.js +++ b/app/assets/javascripts/mobile/mobile_post_actions.js @@ -98,8 +98,12 @@ success: function() { Diaspora.Mobile.PostActions.toggleActive(link); }, - error: function() { - alert(Diaspora.I18n.t("failed_to_reshare")); + error: function(response) { + if (response.status === 0) { + alert(Diaspora.I18n.t("errors.connection")); + } else { + alert(response.responseText); + } }, complete: function() { Diaspora.Mobile.PostActions.hideLoader(link); diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 9abc7503b..bde3e2fbb 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -12,11 +12,17 @@ class CommentsController < ApplicationController end def create - comment = comment_service.create(params[:post_id], params[:text]) + begin + comment = comment_service.create(params[:post_id], params[:text]) + rescue ActiveRecord::RecordNotFound + render text: I18n.t("comments.create.error"), status: 404 + return + end + if comment respond_create_success(comment) else - render nothing: true, status: 404 + render text: I18n.t("comments.create.error"), status: 422 end end diff --git a/app/controllers/likes_controller.rb b/app/controllers/likes_controller.rb index adf92827e..5c2c65789 100644 --- a/app/controllers/likes_controller.rb +++ b/app/controllers/likes_controller.rb @@ -26,7 +26,7 @@ class LikesController < ApplicationController format.json { render :json => @like.as_api_response(:backbone), :status => 201 } end else - render :nothing => true, :status => 422 + render text: I18n.t("likes.create.error"), status: 422 end end diff --git a/app/controllers/reshares_controller.rb b/app/controllers/reshares_controller.rb index 385e6acfc..7345e6343 100644 --- a/app/controllers/reshares_controller.rb +++ b/app/controllers/reshares_controller.rb @@ -14,7 +14,7 @@ class ResharesController < ApplicationController current_user.dispatch_post(@reshare) render :json => ExtremePostPresenter.new(@reshare, current_user), :status => 201 else - render :nothing => true, :status => 422 + render text: I18n.t("reshares.create.error"), status: 422 end end diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index cdff0411b..f6b53ca63 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -240,6 +240,8 @@ en: explanation: "Post to diaspora* from anywhere by bookmarking this link => %{link}." comments: + create: + error: "Failed to comment." new_comment: comment: "Comment" commenting: "Commenting..." @@ -591,6 +593,10 @@ en: source_package: "Download the source code package" be_excellent: "Be excellent to each other! ♥" + likes: + create: + error: "Failed to like." + notifications: started_sharing: zero: "%{actors} started sharing with you." @@ -991,6 +997,8 @@ en: invalid_invite: "The invite link you provided is no longer valid!" reshares: + create: + error: "Failed to reshare." reshare: reshared_via: "Reshared via" reshare_confirmation: "Reshare %{author}’s post?" diff --git a/config/locales/javascript/javascript.en.yml b/config/locales/javascript/javascript.en.yml index b7e71a955..a4583ee4f 100644 --- a/config/locales/javascript/javascript.en.yml +++ b/config/locales/javascript/javascript.en.yml @@ -87,6 +87,9 @@ en: success: "Your new aspect <%= name %> was created" failure: "Aspect creation failed." + errors: + connection: "Unable to connect to the server." + timeago: prefixAgo: "" prefixFromNow: "" @@ -186,9 +189,6 @@ en: one: "In <%= count %> aspect" other: "In <%= count %> aspects" show_more: "Show more" - failed_to_like: "Failed to like. Maybe the author is ignoring you?" - failed_to_reshare: "Failed to reshare!" - failed_to_comment: "Failed to comment. Maybe the author is ignoring you?" failed_to_post_message: "Failed to post message!" failed_to_remove: "Failed to remove the entry!" comments: @@ -196,7 +196,6 @@ en: hide: "Hide comments" no_comments: "There are no comments yet." reshares: - duplicate: "That good, eh? You’ve already reshared that post!" successful: "The post was successfully reshared!" post: "Reshare <%= name %>’s post?" aspect_navigation: diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb index 39492d191..704df125e 100644 --- a/spec/controllers/comments_controller_spec.rb +++ b/spec/controllers/comments_controller_spec.rb @@ -67,6 +67,7 @@ describe CommentsController, :type => :controller do expect(alice).not_to receive(:comment) post :create, comment_hash expect(response.code).to eq("404") + expect(response.body).to eq(I18n.t("comments.create.error")) end end diff --git a/spec/controllers/reshares_controller_spec.rb b/spec/controllers/reshares_controller_spec.rb index 571f5651b..c6f50f2d5 100644 --- a/spec/controllers/reshares_controller_spec.rb +++ b/spec/controllers/reshares_controller_spec.rb @@ -46,7 +46,7 @@ describe ResharesController, :type => :controller do it 'doesn\'t allow the user to reshare the post again' do post_request! expect(response.code).to eq('422') - expect(response.body.strip).to be_empty + expect(response.body).to eq(I18n.t("reshares.create.error")) end end diff --git a/spec/javascripts/app/app_spec.js b/spec/javascripts/app/app_spec.js index 06f9a14b0..a1c987596 100644 --- a/spec/javascripts/app/app_spec.js +++ b/spec/javascripts/app/app_spec.js @@ -96,4 +96,39 @@ describe("app", function() { expect(app._changeLocation).toHaveBeenCalledWith("/users/sign_in"); }); }); + + describe("setupBackboneLinks", function() { + it("calls Backbone.history.start", function() { + spyOn(Backbone.history, "start"); + app.setupBackboneLinks(); + expect(Backbone.history.start).toHaveBeenCalledWith({pushState: true}); + }); + + context("when clicking a backbone link", function() { + beforeEach(function() { + app.stream = {basePath: function() { return "/stream"; }}; + app.notificationsCollection = {fetch: $.noop}; + spyOn(Backbone.history, "start"); + this.link = $(""); + spec.content().append(this.link); + app.setupBackboneLinks(); + }); + + afterEach(function() { + app.stream = undefined; + }); + + it("calls Backbone.history.navigate", function() { + spyOn(Backbone.history, "navigate"); + this.link.click(); + expect(Backbone.history.navigate).toHaveBeenCalledWith("backbone-link", true); + }); + + it("fetches new notifications", function() { + spyOn(app.notificationsCollection, "fetch"); + this.link.click(); + expect(app.notificationsCollection.fetch).toHaveBeenCalled(); + }); + }); + }); }); diff --git a/spec/javascripts/app/models/post/interacations_spec.js b/spec/javascripts/app/models/post/interacations_spec.js index 835504576..019ca307a 100644 --- a/spec/javascripts/app/models/post/interacations_spec.js +++ b/spec/javascripts/app/models/post/interacations_spec.js @@ -6,6 +6,8 @@ describe("app.models.Post.Interactions", function(){ this.interactions = this.post.interactions; this.author = factory.author({guid: "loggedInAsARockstar"}); loginAs({guid: "loggedInAsARockstar"}); + spec.content().append($("
")); + app.flashMessages = new app.views.FlashMessages({el: spec.content().find("#flash-container")}); this.userLike = new app.models.Like({author : this.author}); }); @@ -47,6 +49,16 @@ describe("app.models.Post.Interactions", function(){ jasmine.Ajax.requests.mostRecent().respondWith(ajaxSuccess); expect(this.interactions.likes.trigger).toHaveBeenCalledWith("change"); }); + + it("displays a flash message on errors", function() { + spyOn(app.flashMessages, "handleAjaxError").and.callThrough(); + this.interactions.like(); + jasmine.Ajax.requests.mostRecent().respondWith({status: 400, responseText: "error message"}); + + expect(app.flashMessages.handleAjaxError).toHaveBeenCalled(); + expect(app.flashMessages.handleAjaxError.calls.argsFor(0)[0].responseText).toBe("error message"); + expect(spec.content().find(".flash-message")).toBeErrorFlashMessage("error message"); + }); }); describe("unlike", function(){ @@ -110,6 +122,16 @@ describe("app.models.Post.Interactions", function(){ jasmine.Ajax.requests.mostRecent().respondWith(ajaxSuccess); expect(this.post.get("participation")).toBeTruthy(); }); + + it("displays a flash message on errors", function() { + spyOn(app.flashMessages, "handleAjaxError").and.callThrough(); + this.interactions.reshare(); + jasmine.Ajax.requests.mostRecent().respondWith({status: 400, responseText: "error message"}); + + expect(app.flashMessages.handleAjaxError).toHaveBeenCalled(); + expect(app.flashMessages.handleAjaxError.calls.argsFor(0)[0].responseText).toBe("error message"); + expect(spec.content().find(".flash-message")).toBeErrorFlashMessage("error message"); + }); }); describe("userLike", function(){ @@ -264,6 +286,16 @@ describe("app.models.Post.Interactions", function(){ jasmine.Ajax.requests.mostRecent().respondWith({status: 400}); expect(error).toHaveBeenCalled(); }); + + it("displays a flash message", function() { + spyOn(app.flashMessages, "handleAjaxError").and.callThrough(); + this.interactions.comment("text"); + jasmine.Ajax.requests.mostRecent().respondWith({status: 400, responseText: "error message"}); + + expect(app.flashMessages.handleAjaxError).toHaveBeenCalled(); + expect(app.flashMessages.handleAjaxError.calls.argsFor(0)[0].responseText).toBe("error message"); + expect(spec.content().find(".flash-message")).toBeErrorFlashMessage("error message"); + }); }); }); }); diff --git a/spec/javascripts/app/views/aspect_membership_view_spec.js b/spec/javascripts/app/views/aspect_membership_view_spec.js index 270aa4173..db671ce6b 100644 --- a/spec/javascripts/app/views/aspect_membership_view_spec.js +++ b/spec/javascripts/app/views/aspect_membership_view_spec.js @@ -55,9 +55,12 @@ describe("app.views.AspectMembership", function(){ }); it('displays an error when it fails', function() { + spyOn(app.flashMessages, "handleAjaxError").and.callThrough(); this.newAspect.trigger('click'); jasmine.Ajax.requests.mostRecent().respondWith(resp_fail); + expect(app.flashMessages.handleAjaxError).toHaveBeenCalled(); + expect(app.flashMessages.handleAjaxError.calls.argsFor(0)[0].responseText).toBe("error message"); expect(spec.content().find(".flash-message")).toBeErrorFlashMessage("error message"); }); }); @@ -95,9 +98,12 @@ describe("app.views.AspectMembership", function(){ }); it('displays an error when it fails', function() { + spyOn(app.flashMessages, "handleAjaxError").and.callThrough(); this.oldAspect.trigger('click'); jasmine.Ajax.requests.mostRecent().respondWith(resp_fail); + expect(app.flashMessages.handleAjaxError).toHaveBeenCalled(); + expect(app.flashMessages.handleAjaxError.calls.argsFor(0)[0].responseText).toBe("error message"); expect(spec.content().find(".flash-message")).toBeErrorFlashMessage("error message"); }); }); diff --git a/spec/javascripts/app/views/comment_stream_view_spec.js b/spec/javascripts/app/views/comment_stream_view_spec.js index bec0ce5a2..ecac07333 100644 --- a/spec/javascripts/app/views/comment_stream_view_spec.js +++ b/spec/javascripts/app/views/comment_stream_view_spec.js @@ -116,9 +116,6 @@ describe("app.views.CommentStream", function(){ it("doesn't add the comment to the view", function() { this.request.respondWith({status: 500}); expect(this.view.$(".comment-content p").text()).not.toEqual("a new comment"); - expect(this.view.$(".flash-message")).toBeErrorFlashMessage( - "Failed to comment. Maybe the author is ignoring you?" - ); }); it("doesn't reset the comment box value", function() { diff --git a/spec/javascripts/app/views/flash_messages_view-spec.js b/spec/javascripts/app/views/flash_messages_view-spec.js index bcae20b2c..ad4b40ec6 100644 --- a/spec/javascripts/app/views/flash_messages_view-spec.js +++ b/spec/javascripts/app/views/flash_messages_view-spec.js @@ -30,4 +30,18 @@ describe("app.views.FlashMessages", function(){ expect($(".flash-message").text().trim()).toBe("error!"); }); }); + + describe("handleAjaxError", function() { + it("shows a generic error if the connection failed", function() { + spyOn(flashMessages, "error"); + flashMessages.handleAjaxError({status: 0}); + expect(flashMessages.error).toHaveBeenCalledWith(Diaspora.I18n.t("errors.connection")); + }); + + it("shows the error given in the responseText otherwise", function() { + spyOn(flashMessages, "error"); + flashMessages.handleAjaxError({status: 400, responseText: "some specific ajax error"}); + expect(flashMessages.error).toHaveBeenCalledWith("some specific ajax error"); + }); + }); }); diff --git a/spec/javascripts/mobile/mobile_post_actions_spec.js b/spec/javascripts/mobile/mobile_post_actions_spec.js index 5cdca9000..80d410ea3 100644 --- a/spec/javascripts/mobile/mobile_post_actions_spec.js +++ b/spec/javascripts/mobile/mobile_post_actions_spec.js @@ -223,10 +223,16 @@ describe("Diaspora.Mobile.PostActions", function(){ expect(Diaspora.Mobile.PostActions.toggleActive).toHaveBeenCalledWith(this.reshareLink); }); - it("pops an alert on error", function(){ + it("pops an alert on server errors", function() { this.reshareLink.click(); - jasmine.Ajax.requests.mostRecent().respondWith({status: 400}); - expect(window.alert).toHaveBeenCalledWith(Diaspora.I18n.t("failed_to_reshare")); + jasmine.Ajax.requests.mostRecent().respondWith({status: 400, responseText: "reshare failed"}); + expect(window.alert).toHaveBeenCalledWith("reshare failed"); + }); + + it("pops an alert on network errors", function() { + this.reshareLink.click(); + jasmine.Ajax.requests.mostRecent().abort(); + expect(window.alert).toHaveBeenCalledWith(Diaspora.I18n.t("errors.connection")); }); }); });