Merge branch 'next-minor' into develop

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)

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();
     });
   },
 

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() {
+      error: function(model, response) {
-        app.flashMessages.error(Diaspora.I18n.t("failed_to_like"));
+        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 () {
+    this.comments.make(text).fail(function(response) {
-      app.flashMessages.error(Diaspora.I18n.t("failed_to_comment"));
+      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(){
+      .fail(function(response) {
-        app.flashMessages.error(Diaspora.I18n.t("reshares.duplicate"));
+        app.flashMessages.handleAjaxError(response);
       });
 
     app.instrument("track", "Reshare");

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) {

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);
+    }
   }
 });

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

app/assets/javascripts/mobile/mobile_post_actions.js

@@ -98,8 +98,12 @@
           success: function() {
             Diaspora.Mobile.PostActions.toggleActive(link);
           },
-          error: function() {
+          error: function(response) {
-            alert(Diaspora.I18n.t("failed_to_reshare"));
+            if (response.status === 0) {
+              alert(Diaspora.I18n.t("errors.connection"));
+            } else {
+              alert(response.responseText);
+            }
           },
           complete: function() {
             Diaspora.Mobile.PostActions.hideLoader(link);

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
 

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
 

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
 

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?"

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:

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
 

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
 

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 = $("<a href='/backbone-link' rel='backbone'>");
+        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();
+      });
+    });
+  });
 });

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($("<div id='flash-container'>"));
+    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");
+      });
     });
   });
 });

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");
     });
   });

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() {

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");
+    });
+  });
 });

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});
+      jasmine.Ajax.requests.mostRecent().respondWith({status: 400, responseText: "reshare failed"});
-      expect(window.alert).toHaveBeenCalledWith(Diaspora.I18n.t("failed_to_reshare"));
+      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"));
     });
   });
 });