Merge branch 'next-minor' into develop

Changelog.md

@@ -13,6 +13,7 @@
 * Increase the spacing above and below post contents [#7267](https://github.com/diaspora/diaspora/pull/7267)
 * Replace fileuploader-custom with FineUploader [#7083](https://github.com/diaspora/diaspora/pull/7083)
 * Always show mobile reaction counts [#7207](https://github.com/diaspora/diaspora/pull/7207)
+* Refactor mobile alerts for error responses [#7227](https://github.com/diaspora/diaspora/pull/7227)
 
 ## Bug fixes
 * Fix background color of year on notifications page with dark theme [#7263](https://github.com/diaspora/diaspora/pull/7263)

Gemfile.lock

@@ -831,7 +831,7 @@ GEM
       unicode-display_width (~> 1.1.1)
     test_after_commit (1.1.0)
       activerecord (>= 3.2)
-    thor (0.19.4)
+    thor (0.19.1)
     thread_safe (0.3.5)
     tilt (1.4.1)
     timecop (0.8.1)

app/assets/javascripts/mobile/mobile_alert.js

@@ -14,6 +14,14 @@
 
     success: function(message) { this._flash(message, "success"); },
 
-    error: function(message) { this._flash(message, "danger"); }
+    error: function(message) { this._flash(message, "danger"); },
+
+    handleAjaxError: function(response) {
+      if (response.status === 0) {
+        this.error(Diaspora.I18n.t("errors.connection"));
+      } else {
+        this.error(response.responseText);
+      }
+    }
   };
 })();

app/assets/javascripts/mobile/mobile_comments.js

@@ -50,8 +50,9 @@
 
       $.post(form.attr("action") + "?format=mobile", form.serialize(), function(data){
         Diaspora.Mobile.Comments.updateStream(form, data);
-      }, "html").fail(function(){
+      }, "html").fail(function(response) {
-        Diaspora.Mobile.Comments.resetCommentBox(this);
+        Diaspora.Mobile.Alert.handleAjaxError(response);
+        Diaspora.Mobile.Comments.resetCommentBox(form);
       });
 
       autosize($(".add-comment-switcher:not(.hidden) textarea"));

app/assets/javascripts/mobile/mobile_conversations.js

@@ -10,9 +10,8 @@
       Diaspora.Mobile.changeLocation(Routes.conversation(data.id));
     },
 
-    conversationCreateError: function(evt, resp) {
+    conversationCreateError: function(evt, response) {
-      Diaspora.Mobile.Alert.error(resp.responseText);
+      Diaspora.Mobile.Alert.handleAjaxError(response);
-      $("html").animate({scrollTop: 0});
     }
   };
 })();

app/assets/javascripts/mobile/mobile_post_actions.js

@@ -35,6 +35,9 @@
           Diaspora.Mobile.PostActions.showLoader(link);
         },
         success: onSuccess,
+        error: function(response) {
+          Diaspora.Mobile.Alert.handleAjaxError(response);
+        },
         complete: function() {
           Diaspora.Mobile.PostActions.hideLoader(link);
         }
@@ -61,6 +64,9 @@
           Diaspora.Mobile.PostActions.showLoader(link);
         },
         success: onSuccess,
+        error: function(response) {
+          Diaspora.Mobile.Alert.handleAjaxError(response);
+        },
         complete: function() {
           Diaspora.Mobile.PostActions.hideLoader(link);
         }
@@ -99,11 +105,7 @@
             Diaspora.Mobile.PostActions.toggleActive(link);
           },
           error: function(response) {
-            if (response.status === 0) {
+            Diaspora.Mobile.Alert.handleAjaxError(response);
-              alert(Diaspora.I18n.t("errors.connection"));
-            } else {
-              alert(response.responseText);
-            }
           },
           complete: function() {
             Diaspora.Mobile.PostActions.hideLoader(link);

app/assets/stylesheets/color_themes/dark/mobile.scss

@@ -43,7 +43,7 @@ body {
   .stream-element.unread { background-color: $gray; }
   .stream-element.read { background-color: $gray-darker; }
 
-  #show_content .photo { border-color: $border-grey; }
+  .photos .photo { border-color: $border-grey; }
 
   .header-full-width { border-bottom: 1px solid $border-grey; }
 
@@ -60,6 +60,6 @@ body {
     }
   }
 }
-// scss-lint:enable IdSelector, SelectorFormat, NestingDepth, SelectorDepth
+// scss-lint:enable SelectorFormat, NestingDepth, SelectorDepth
 
 @import 'mobile/mobile';

app/assets/stylesheets/mobile/flash_messages.scss

@@ -0,0 +1,20 @@
+.flash-messages-container {
+  background-color: rgba($black, .5);
+  padding: 0 10px;
+  position: fixed;
+  top: $mobile-navbar-height;
+  width: 100%;
+  z-index: 9;
+
+  .flash-messages {
+    width: 100%;
+  }
+
+  .alert {
+    margin-bottom: 10px;
+  }
+
+  .alert:first-of-type {
+    margin-top: 10px;
+  }
+}

app/assets/stylesheets/mobile/mobile.scss

@@ -11,6 +11,7 @@
 @import "mobile/header";
 @import "mobile/tags";
 @import "mobile/conversations";
+@import 'mobile/flash_messages';
 @import "mobile/settings";
 @import "mobile/stream_element";
 @import "mobile/comments";
@@ -269,11 +270,8 @@ footer {
   font-size: 14px;
 }
 
-#show_content {
+.photos {
   padding-bottom: 24px;
-  border-bottom: 1px solid #bbb;
-  font-size: larger;
-  text-align: center;
 
   img:not(.avatar) {
     max-width: 100%;
@@ -287,37 +285,30 @@ footer {
     box-shadow: 0 1px 2px rgba(0, 0, 0, 0.2);
     border-color: #DDDDDD #BBBBBB #AAAAAA;
     border-bottom-width: 0px;
-    margin-top: 12px;
     min-height: 100px;
-    line-height: 5;
-  }
-
-  .controls {
-    font: {
-      size: smaller; };
-  }
-
-  &.photos {
-    border-bottom: 0 !important;
   }
 }
 
-#photo_controls {
+.photo-controls .arrow {
-  .arrow {
+  bottom: -.2em;
   font-size: 10em;
+  position: fixed;
   text-decoration: none;
   text-shadow: 0 0 3px $white;
-    position: fixed;
-    bottom: -0.2em;
   z-index: 1;
+
   &.left {
-      left: -0.2em;
+    left: -.2em;
   }
+
   &.right {
-      right: -0.2em;
+    right: -.2em;
     text-align: right;
   }
-    .entypo-chevron-left, .entypo-chevron-right { margin-right: 0; }
+
+  .entypo-chevron-left,
+  .entypo-chevron-right {
+    margin-right: 0;
   }
 }
 

app/controllers/likes_controller.rb

@@ -31,7 +31,12 @@ class LikesController < ApplicationController
   end
 
   def destroy
+    begin
       @like = Like.find_by_id_and_author_id!(params[:id], current_user.person.id)
+    rescue ActiveRecord::RecordNotFound
+      render text: I18n.t("likes.destroy.error"), status: 404
+      return
+    end
 
     current_user.retract(@like)
     respond_to do |format|

app/views/conversations/new.mobile.haml

@@ -24,7 +24,6 @@
     });
 
 .col-md-6.col-md-offset-3#new_conversation_pane
-  #flash-messages
   .container-fluid.row
     %h3
       = t("conversations.index.new_conversation")

app/views/layouts/application.mobile.haml

@@ -28,6 +28,9 @@
       - if user_signed_in?
         = render "layouts/drawer"
 
+      .flash-messages-container
+        .flash-messages#flash-messages
+
       #main{:role => "main"}
         - if current_page?(:activity_stream)
           %h3

app/views/photos/show.mobile.haml

@@ -5,7 +5,7 @@
 :css
   footer{display: none;}
 
-#show_content.photos
+.photos
   .photo
     = image_tag photo.url(:scaled_full)
   .stream-element.photo_mobile
@@ -24,7 +24,7 @@
                 = timeago(photo.created_at)
 
 -if additional_photos && additional_photos.length > 1
-  #photo_controls
+  .photo-controls
     - if previous_photo != additional_photos.last
       = link_to(content_tag(:i, nil, id: "arrow-left", class: "entypo-chevron-left"),
                 person_photo_path(previous_photo.author, previous_photo),

config/locales/diaspora/en.yml

@@ -591,6 +591,8 @@ en:
   likes:
     create:
       error: "Failed to like."
+    destroy:
+      error: "Failed to unlike."
 
   notifications:
     started_sharing:

features/mobile/multiphoto.feature

@@ -17,7 +17,7 @@ Feature: viewing photos on the mobile main page
     When I press "Share"
     And I go to the stream page
     And I click on selector "img.stream-photo"
-    Then I should see a "img" within "#show_content"
+    Then I should see a "img" within ".photos"
     And I should not see a "#arrow-right" within "#main"
     And I should not see a "#arrow-left" within "#main"
 

spec/controllers/likes_controller_spec.rb

@@ -133,14 +133,12 @@ describe LikesController, :type => :controller do
 
         it 'does not let a user destroy other likes' do
           like2 = eve.like!(@message)
-
           like_count = Like.count
-          expect {
+
           delete :destroy, :format => :json, id_field => like2.target_id, :id => like2.id
-          }.to raise_error(ActiveRecord::RecordNotFound)
+          expect(response.status).to eq(404)
-
+          expect(response.body).to eq(I18n.t("likes.destroy.error"))
           expect(Like.count).to eq(like_count)
-
         end
       end
     end

spec/javascripts/mobile/mobile_alert_spec.js

@@ -26,4 +26,18 @@ describe("Diaspora.Mobile.Alert", function() {
       expect(Diaspora.Mobile.Alert._flash).toHaveBeenCalledWith("Oh noez!", "danger");
     });
   });
+
+  describe("handleAjaxError", function() {
+    it("shows a generic error if the connection failed", function() {
+      spyOn(Diaspora.Mobile.Alert, "error");
+      Diaspora.Mobile.Alert.handleAjaxError({status: 0});
+      expect(Diaspora.Mobile.Alert.error).toHaveBeenCalledWith(Diaspora.I18n.t("errors.connection"));
+    });
+
+    it("shows the error given in the responseText otherwise", function() {
+      spyOn(Diaspora.Mobile.Alert, "error");
+      Diaspora.Mobile.Alert.handleAjaxError({status: 400, responseText: "some specific ajax error"});
+      expect(Diaspora.Mobile.Alert.error).toHaveBeenCalledWith("some specific ajax error");
+    });
+  });
 });

spec/javascripts/mobile/mobile_comments_spec.js

@@ -1,8 +1,22 @@
 describe("Diaspora.Mobile.Comments", function(){
-  describe("toggleComments", function() {
   beforeEach(function() {
     spec.loadFixture("aspects_index_mobile_post_with_comments");
+    this.bottomBar = $(".bottom-bar").first();
     this.link = $(".stream .show-comments").first();
+  });
+
+  describe("initialize", function() {
+    it("calls submitComment when the comment form has been submitted", function() {
+      spyOn(Diaspora.Mobile.Comments, "submitComment");
+      Diaspora.Mobile.Comments.initialize();
+      Diaspora.Mobile.Comments.showCommentBox($(".stream .comment-action").first());
+      $(".stream .new_comment").first().submit();
+      expect(Diaspora.Mobile.Comments.submitComment).toHaveBeenCalled();
+    });
+  });
+
+  describe("toggleComments", function() {
+    beforeEach(function() {
       spyOn(Diaspora.Mobile.Comments, "showComments");
       spyOn(Diaspora.Mobile.Comments, "hideComments");
     });
@@ -30,9 +44,6 @@ describe("Diaspora.Mobile.Comments", function(){
 
   describe("showUnloadedComments", function() {
     beforeEach(function() {
-      spec.loadFixture("aspects_index_mobile_post_with_comments");
-      this.link = $(".stream .show-comments").first();
-      this.bottomBar = this.link.closest(".bottom-bar").first();
       this.commentActionLink = this.bottomBar.find("a.comment-action");
     });
 
@@ -79,26 +90,51 @@ describe("Diaspora.Mobile.Comments", function(){
     });
   });
 
-  describe("createComment", function () {
+  describe("submitComment", function() {
     beforeEach(function() {
-      spec.loadFixture("aspects_index_mobile_post_with_comments");
+      Diaspora.Mobile.Comments.initialize();
-      var link = $(".stream .comment-action").first();
+      Diaspora.Mobile.Comments.showCommentBox($(".stream .comment-action").first());
-      Diaspora.Mobile.Comments.showCommentBox(link);
-      $(".stream .new_comment").submit(Diaspora.Mobile.Comments.submitComment);
     });
 
     it("doesn't submit an empty comment", function() {
-      var form = $(".stream .new_comment").first();
+      $(".stream .new_comment").first().submit();
-      spyOn(jQuery, "ajax");
+      expect(jasmine.Ajax.requests.count()).toBe(0);
-      form.submit();
+    });
-      expect(jQuery.ajax).not.toHaveBeenCalled();
+
+    it("submits comments with text", function() {
+      $(".stream .new_comment textarea").val("comment text");
+      $(".stream .new_comment").first().submit();
+      expect(jasmine.Ajax.requests.mostRecent().data().text).toEqual(["comment text"]);
+    });
+
+    it("calls updateStream on success", function() {
+      spyOn(Diaspora.Mobile.Comments, "updateStream");
+      $(".stream .new_comment textarea").val("comment text");
+      $(".stream .new_comment").first().submit();
+      jasmine.Ajax.requests.mostRecent().respondWith({status: 200, responseText: "foo"});
+      expect(Diaspora.Mobile.Comments.updateStream).toHaveBeenCalledWith($(".stream .new_comment").first(), "foo");
+    });
+
+    it("lets Diaspora.Mobile.Alert handle AJAX errors", function() {
+      spyOn(Diaspora.Mobile.Alert, "handleAjaxError");
+      $(".stream .new_comment textarea").val("comment text");
+      $(".stream .new_comment").first().submit();
+      jasmine.Ajax.requests.mostRecent().respondWith({status: 400, responseText: "oh noez! comment failed!"});
+      expect(Diaspora.Mobile.Alert.handleAjaxError).toHaveBeenCalled();
+      expect(Diaspora.Mobile.Alert.handleAjaxError.calls.argsFor(0)[0].responseText).toBe("oh noez! comment failed!");
+    });
+
+    it("calls resetCommentBox on errors", function() {
+      spyOn(Diaspora.Mobile.Comments, "resetCommentBox");
+      $(".stream .new_comment textarea").val("comment text");
+      $(".stream .new_comment").first().submit();
+      jasmine.Ajax.requests.mostRecent().respondWith({status: 400, responseText: "oh noez! comment failed!"});
+      expect(Diaspora.Mobile.Comments.resetCommentBox).toHaveBeenCalledWith($(".stream .new_comment").first());
     });
   });
 
   describe("increaseReactionCount", function(){
     beforeEach(function() {
-      spec.loadFixture("aspects_index_mobile_post_with_comments");
-      this.bottomBar = $(".bottom-bar").first();
       this.toggleReactionsLink = this.bottomBar.find(".show-comments").first();
     });
 
@@ -123,8 +159,6 @@ describe("Diaspora.Mobile.Comments", function(){
 
   describe("bottomBarLazy", function(){
     beforeEach(function() {
-      spec.loadFixture("aspects_index_mobile_post_with_comments");
-      this.bottomBar = $(".bottom-bar").first();
       this.bottomBarLazy = Diaspora.Mobile.Comments.bottomBarLazy(this.bottomBar);
     });
 

spec/javascripts/mobile/mobile_conversations_spec.js

@@ -4,6 +4,11 @@ describe("Diaspora.Mobile.Conversations", function() {
     Diaspora.Page = "ConversationsNew";
   });
 
+  afterEach(function() {
+    $(document).off("ajax:success", Diaspora.Mobile.Conversations.conversationCreateSuccess);
+    $(document).off("ajax:error", Diaspora.Mobile.Conversations.conversationCreateError);
+  });
+
   describe("conversationCreateSuccess", function() {
     it("is called when there was a successful ajax request for the conversation form", function() {
       spyOn(Diaspora.Mobile.Conversations, "conversationCreateSuccess");
@@ -43,10 +48,12 @@ describe("Diaspora.Mobile.Conversations", function() {
     });
 
     it("shows a flash message", function() {
-      spyOn(Diaspora.Mobile.Alert, "error");
+      spyOn(Diaspora.Mobile.Alert, "handleAjaxError").and.callThrough();
       Diaspora.Mobile.Conversations.initialize();
-      $("#new-conversation").trigger("ajax:error", [{responseText: "Oh noez! Something went wrong!"}]);
+      var response = {status: 422, responseText: "Oh noez! Something went wrong!"};
-      expect(Diaspora.Mobile.Alert.error).toHaveBeenCalledWith("Oh noez! Something went wrong!");
+      $("#new-conversation").trigger("ajax:error", response);
+      expect(Diaspora.Mobile.Alert.handleAjaxError).toHaveBeenCalledWith(response);
+      expect($("#flash-messages")).toHaveText("Oh noez! Something went wrong!");
     });
   });
 });

spec/javascripts/mobile/mobile_post_actions_spec.js

@@ -118,6 +118,14 @@ describe("Diaspora.Mobile.PostActions", function(){
       expect(this.likeCounter.text()).toBe("0");
     });
 
+    it("lets Diaspora.Mobile.Alert handle AJAX errors", function() {
+      spyOn(Diaspora.Mobile.Alert, "handleAjaxError");
+      Diaspora.Mobile.PostActions.like(this.likeCounter, this.link);
+      jasmine.Ajax.requests.mostRecent().respondWith({status: 400, responseText: "oh noez! like failed!"});
+      expect(Diaspora.Mobile.Alert.handleAjaxError).toHaveBeenCalled();
+      expect(Diaspora.Mobile.Alert.handleAjaxError.calls.argsFor(0)[0].responseText).toBe("oh noez! like failed!");
+    });
+
     it("activates link on success", function(){
       spyOn(Diaspora.Mobile.PostActions, "toggleActive");
       var data = this.link.data("url");
@@ -166,6 +174,14 @@ describe("Diaspora.Mobile.PostActions", function(){
       expect(this.likeCounter.text()).toBe("1");
     });
 
+    it("lets Diaspora.Mobile.Alert handle AJAX errors", function() {
+      spyOn(Diaspora.Mobile.Alert, "handleAjaxError");
+      Diaspora.Mobile.PostActions.unlike(this.likeCounter, this.link);
+      jasmine.Ajax.requests.mostRecent().respondWith({status: 400, responseText: "oh noez! unlike failed!"});
+      expect(Diaspora.Mobile.Alert.handleAjaxError).toHaveBeenCalled();
+      expect(Diaspora.Mobile.Alert.handleAjaxError.calls.argsFor(0)[0].responseText).toBe("oh noez! unlike failed!");
+    });
+
     it("deactivates link on success", function(){
       spyOn(Diaspora.Mobile.PostActions, "toggleActive");
       var data = this.link.data("url");
@@ -198,7 +214,6 @@ describe("Diaspora.Mobile.PostActions", function(){
       Diaspora.Mobile.PostActions.initialize();
       this.reshareLink = $(".stream .reshare-action");
       spyOn(window, "confirm").and.returnValue(true);
-      spyOn(window, "alert");
     });
 
     it("always calls showLoader before sending request and hideLoader after receiving response", function(){
@@ -223,16 +238,12 @@ describe("Diaspora.Mobile.PostActions", function(){
       expect(Diaspora.Mobile.PostActions.toggleActive).toHaveBeenCalledWith(this.reshareLink);
     });
 
-    it("pops an alert on server errors", function() {
+    it("lets Diaspora.Mobile.Alert handle AJAX errors", function() {
+      spyOn(Diaspora.Mobile.Alert, "handleAjaxError");
       this.reshareLink.click();
       jasmine.Ajax.requests.mostRecent().respondWith({status: 400, responseText: "reshare failed"});
-      expect(window.alert).toHaveBeenCalledWith("reshare failed");
+      expect(Diaspora.Mobile.Alert.handleAjaxError).toHaveBeenCalled();
-    });
+      expect(Diaspora.Mobile.Alert.handleAjaxError.calls.argsFor(0)[0].responseText).toBe("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"));
     });
   });
 });