Refactor conversations creation

closes #7131

app/assets/javascripts/app/views/conversations_form_view.js

@@ -5,7 +5,6 @@ app.views.ConversationsForm = Backbone.View.extend({
 
   events: {
     "keydown .conversation-message-text": "keyDown",
-    "submit #conversation-new": "onSubmitNewConversation"
   },
 
   initialize: function(opts) {
@@ -15,6 +14,8 @@ app.views.ConversationsForm = Backbone.View.extend({
       this.prefill = [{name: opts.prefillName, value: opts.prefillValue}];
     }
     this.prepareAutocomplete(this.contacts);
+    this.$("form#new-conversation").on("ajax:success", this.conversationCreateSuccess);
+    this.$("form#new-conversation").on("ajax:error", this.conversationCreateError);
   },
 
   prepareAutocomplete: function(data){
@@ -38,17 +39,12 @@ app.views.ConversationsForm = Backbone.View.extend({
     }
   },
 
-  getConversationParticipants: function() {
-    return this.$("#as-values-contact_ids").val().split(",");
+  conversationCreateSuccess: function(evt, data) {
+    app._changeLocation(Routes.conversation(data.id));
   },
 
-  onSubmitNewConversation: function(evt) {
-    evt.preventDefault();
-    if (this.getConversationParticipants().length === 0) {
-      evt.stopPropagation();
-      app.flashMessages.error(Diaspora.I18n.t("conversation.create.no_recipient"));
-    }
+  conversationCreateError: function(evt, resp) {
+    app.flashMessages.error(resp.responseText);
   }
 });
 // @license-end
-

app/assets/javascripts/mobile/mobile.js

@@ -25,7 +25,9 @@
 //= require mobile/profile_aspects
 //= require mobile/tag_following
 //= require mobile/publisher
+//= require mobile/mobile_alert
 //= require mobile/mobile_comments
+//= require mobile/mobile_conversations
 //= require mobile/mobile_post_actions
 //= require mobile/mobile_drawer
 // @license-end

app/assets/javascripts/mobile/mobile_application.js

@@ -8,6 +8,10 @@
 
       // init autosize plugin
       autosize($("textarea"));
+    },
+
+    changeLocation: function(href) {
+      window.location.assign(href);
     }
   };
 })();

app/assets/javascripts/mobile/mobile_conversations.js

@@ -0,0 +1,22 @@
+(function() {
+  Diaspora.Mobile.Conversations = {
+    initialize: function() {
+      if (Diaspora.Page !== "ConversationsNew") { return; }
+      $(document).on("ajax:success", "form#new-conversation", this.conversationCreateSuccess);
+      $(document).on("ajax:error", "form#new-conversation", this.conversationCreateError);
+    },
+
+    conversationCreateSuccess: function(evt, data) {
+      Diaspora.Mobile.changeLocation(Routes.conversation(data.id));
+    },
+
+    conversationCreateError: function(evt, resp) {
+      Diaspora.Mobile.Alert.error(resp.responseText);
+      $("html").animate({scrollTop: 0});
+    }
+  };
+})();
+
+$(document).ready(function() {
+  Diaspora.Mobile.Conversations.initialize();
+});

app/controllers/conversations_controller.rb

@@ -43,21 +43,16 @@ class ConversationsController < ApplicationController
     opts[:message] = { text: params[:conversation][:text] }
     @conversation = current_user.build_conversation(opts)
 
-    @response = {}
     if person_ids.present? && @conversation.save
       Diaspora::Federation::Dispatcher.defer_dispatch(current_user, @conversation)
-      @response[:success] = true
-      @response[:message] = I18n.t('conversations.create.sent')
-      @response[:conversation_id] = @conversation.id
+      flash[:notice] = I18n.t("conversations.create.sent")
+      render json: {id: @conversation.id}
     else
-      @response[:success] = false
-      @response[:message] = I18n.t('conversations.create.fail')
+      message = I18n.t("conversations.create.fail")
       if person_ids.blank?
-        @response[:message] = I18n.t("javascripts.conversation.create.no_recipient")
+        message = I18n.t("javascripts.conversation.create.no_recipient")
       end
-    end
-    respond_to do |format|
-      format.js
+      render text: message, status: 422
     end
   end
 

app/views/conversations/_new.haml

@@ -1,6 +1,6 @@
 .container-fluid
   = form_for Conversation.new, html: {id: "new-conversation",
-    class: "new-conversation form-horizontal form-do-not-clear"}, remote: true do |conversation|
+    class: "new-conversation form-horizontal"}, remote: true do |conversation|
     .form-group
       %label#toLabel{for: "contact_ids"}
         = t(".to")

app/views/conversations/create.js.erb

@@ -1,14 +0,0 @@
-var response = <%= raw @response.to_json %>;
-<% if session[:mobile_view] %>
-if(response.success) {
-  window.location.href = "<%= conversations_path(conversation_id: @conversation.id) %>";
-}
-<% else %>
-  if(response.success){
-    app.flashMessages.success(response.message);
-    $("#new-conversation").removeClass('form-do-not-clear').clearForm();
-    window.location.href = "<%= conversations_path(conversation_id: @conversation.id) %>";
-  } else {
-    app.flashMessages.error(response.message);
-  }
-<% end %>

app/views/conversations/new.mobile.haml

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

features/mobile/conversations.feature

@@ -12,9 +12,7 @@ Feature: private conversations mobile
 
   Scenario: send and delete a mobile message
     Given I send a mobile message with subject "Greetings" and text "hello, alice!" to "Alice Awesome"
-    Then I should see "Greetings" within ".ltr"
-    And I should see "Greetings" within ".ltr"
-    And I press the first ".ltr" within ".conversation"
+    Then I should see "Greetings" within ".conversation h3"
     And "Alice Awesome" should be part of active conversation
     And I should see "hello, alice!" within ".stream-element"
     When I sign in as "alice@alice.alice" on the mobile website

spec/controllers/conversations_controller_spec.rb

@@ -140,11 +140,10 @@ describe ConversationsController, :type => :controller do
         }.to change(Message, :count).by(1)
       end
 
-      it 'should set response with success to true and message to success message' do
+      it "responds with the conversation id as JSON" do
         post :create, @hash
-        expect(assigns[:response][:success]).to eq(true)
-        expect(assigns[:response][:message]).to eq(I18n.t('conversations.create.sent'))
-        expect(assigns[:response][:conversation_id]).to eq(Conversation.first.id)
+        expect(response).to be_success
+        expect(JSON.parse(response.body)["id"]).to eq(Conversation.first.id)
       end
 
       it 'sets the author to the current_user' do
@@ -193,11 +192,10 @@ describe ConversationsController, :type => :controller do
         }.to change(Message, :count).by(1)
       end
 
-      it 'should set response with success to true and message to success message' do
+      it "responds with the conversation id as JSON" do
         post :create, @hash
-        expect(assigns[:response][:success]).to eq(true)
-        expect(assigns[:response][:message]).to eq(I18n.t('conversations.create.sent'))
-        expect(assigns[:response][:conversation_id]).to eq(Conversation.first.id)
+        expect(response).to be_success
+        expect(JSON.parse(response.body)["id"]).to eq(Conversation.first.id)
       end
     end
 
@@ -225,10 +223,10 @@ describe ConversationsController, :type => :controller do
         expect(Message.count).to eq(count)
       end
 
-      it 'should set response with success to false and message to create fail' do
+      it "responds with an error message" do
         post :create, @hash
-        expect(assigns[:response][:success]).to eq(false)
-        expect(assigns[:response][:message]).to eq(I18n.t('conversations.create.fail'))
+        expect(response).not_to be_success
+        expect(response.body).to eq(I18n.t("conversations.create.fail"))
       end
     end
 
@@ -256,10 +254,10 @@ describe ConversationsController, :type => :controller do
         expect(Message.count).to eq(count)
       end
 
-      it 'should set response with success to false and message to fail due to no contact' do
+      it "responds with an error message" do
         post :create, @hash
-        expect(assigns[:response][:success]).to eq(false)
-        expect(assigns[:response][:message]).to eq(I18n.t("javascripts.conversation.create.no_recipient"))
+        expect(response).not_to be_success
+        expect(response.body).to eq(I18n.t("javascripts.conversation.create.no_recipient"))
       end
     end
 
@@ -286,6 +284,12 @@ describe ConversationsController, :type => :controller do
         post :create, @hash
         expect(Message.count).to eq(count)
       end
+
+      it "responds with an error message" do
+        post :create, @hash
+        expect(response).not_to be_success
+        expect(response.body).to eq(I18n.t("javascripts.conversation.create.no_recipient"))
+      end
     end
   end
 

spec/controllers/jasmine_fixtures/conversations_spec.rb

@@ -31,4 +31,16 @@ describe ConversationsController, :type => :controller do
       save_fixture(html_for("body"), "conversations_read")
     end
   end
+
+  describe "#new" do
+    before do
+      sign_in alice, scope: :user
+    end
+
+    it "generates a jasmine fixture", fixture: true do
+      session[:mobile_view] = true
+      get :new, format: :mobile
+      save_fixture(html_for("body"), "conversations_new_mobile")
+    end
+  end
 end

spec/javascripts/app/views/conversations_form_view_spec.js

@@ -1,8 +1,11 @@
 describe("app.views.ConversationsForm", function() {
+  beforeEach(function() {
+    spec.loadFixture("conversations_read");
+  });
+
   describe("keyDown", function() {
     beforeEach(function() {
       this.submitCallback = jasmine.createSpy().and.returnValue(false);
-      spec.loadFixture("conversations_read");
       new app.views.ConversationsForm();
     });
 
@@ -49,62 +52,49 @@ describe("app.views.ConversationsForm", function() {
     });
   });
 
-  describe("onSubmitNewConversation", function() {
-    beforeEach(function() {
-      spec.loadFixture("conversations_read");
-      $("#conversation-new").removeClass("hidden");
-      $("#conversation-show").addClass("hidden");
-      spyOn(app.views.ConversationsForm.prototype, "onSubmitNewConversation").and.callThrough();
-      this.target = new app.views.ConversationsForm();
+  describe("conversationCreateSuccess", function() {
+    it("is called when there was a successful ajax request for the conversation form", function() {
+      spyOn(app.views.ConversationsForm.prototype, "conversationCreateSuccess");
+      this.view = new app.views.ConversationsForm();
+
+      $("#conversation-show").trigger("ajax:success", [{id: 23}]);
+      expect(app.views.ConversationsForm.prototype.conversationCreateSuccess).not.toHaveBeenCalled();
+
+      $("#new-conversation").trigger("ajax:error", [{responseText: "error"}]);
+      expect(app.views.ConversationsForm.prototype.conversationCreateSuccess).not.toHaveBeenCalled();
+
+      $("#new-conversation").trigger("ajax:success", [{id: 23}]);
+      expect(app.views.ConversationsForm.prototype.conversationCreateSuccess).toHaveBeenCalled();
     });
 
-    it("onSubmitNewConversation is called when submitting the conversation form", function() {
-      spyOn(app.views.ConversationsForm.prototype, "getConversationParticipants").and.returnValue([]);
-      $("#conversation-new").trigger("submit");
+    it("redirects to the new conversation", function() {
+      spyOn(app, "_changeLocation");
+      this.view = new app.views.ConversationsForm();
+      $("#new-conversation").trigger("ajax:success", [{id: 23}]);
+      expect(app._changeLocation).toHaveBeenCalledWith(Routes.conversation(23));
+    });
+  });
 
-      expect(app.views.ConversationsForm.prototype.onSubmitNewConversation).toHaveBeenCalled();
+  describe("conversationCreateError", function() {
+    it("is called when an ajax request failed for the conversation form", function() {
+      spyOn(app.views.ConversationsForm.prototype, "conversationCreateError");
+      this.view = new app.views.ConversationsForm();
+
+      $("#conversation-show").trigger("ajax:error", [{responseText: "error"}]);
+      expect(app.views.ConversationsForm.prototype.conversationCreateError).not.toHaveBeenCalled();
+
+      $("#new-conversation").trigger("ajax:success", [{id: 23}]);
+      expect(app.views.ConversationsForm.prototype.conversationCreateError).not.toHaveBeenCalled();
+
+      $("#new-conversation").trigger("ajax:error", [{responseText: "error"}]);
+      expect(app.views.ConversationsForm.prototype.conversationCreateError).toHaveBeenCalled();
     });
 
-    it("does not submit a conversation with no recipient", function() {
-      spyOn(app.views.ConversationsForm.prototype, "getConversationParticipants").and.returnValue([]);
-      var event = jasmine.createSpyObj("event", ["preventDefault", "stopPropagation"]);
-
-      this.target.onSubmitNewConversation(event);
-
-      expect(event.preventDefault).toHaveBeenCalled();
-      expect(event.stopPropagation).toHaveBeenCalled();
-    });
-
-    it("submits a conversation with recipients", function() {
-      spyOn(app.views.ConversationsForm.prototype, "getConversationParticipants").and.returnValue([1]);
-      var event = jasmine.createSpyObj("event", ["preventDefault", "stopPropagation"]);
-
-      this.target.onSubmitNewConversation(event);
-
-      expect(event.preventDefault).toHaveBeenCalled();
-      expect(event.stopPropagation).not.toHaveBeenCalled();
-    });
-
-    it("flashes an error message when submitting a conversation with no recipient", function() {
-      spyOn(app.views.FlashMessages.prototype, "error");
-      spyOn(app.views.ConversationsForm.prototype, "getConversationParticipants").and.returnValue([]);
-      var event = jasmine.createSpyObj("event", ["preventDefault", "stopPropagation"]);
-
-      this.target.onSubmitNewConversation(event);
-
-      expect(app.views.FlashMessages.prototype.error)
-        .toHaveBeenCalledWith(Diaspora.I18n.t("conversation.create.no_recipient"));
-    });
-
-    it("does not flash an error message when submitting a conversation with recipients", function() {
-      spyOn(app.views.FlashMessages.prototype, "error");
-      spyOn(app.views.ConversationsForm.prototype, "getConversationParticipants").and.returnValue([1]);
-      var event = jasmine.createSpyObj("event", ["preventDefault", "stopPropagation"]);
-
-      this.target.onSubmitNewConversation(event);
-
-      expect(app.views.FlashMessages.prototype.error).not
-        .toHaveBeenCalledWith(Diaspora.I18n.t("conversation.create.no_recipient"));
+    it("shows a flash message", function() {
+      spyOn(app.flashMessages, "error");
+      this.view = new app.views.ConversationsForm();
+      $("#new-conversation").trigger("ajax:error", [{responseText: "Oh noez! Something went wrong!"}]);
+      expect(app.flashMessages.error).toHaveBeenCalledWith("Oh noez! Something went wrong!");
     });
   });
 });

spec/javascripts/jasmine_helpers/SpecHelper.js

@@ -54,6 +54,10 @@ beforeEach(function() {
   Diaspora.page = new Page();
   Diaspora.page.publish("page/ready", [$(document.body)]);
 
+  // don't change window.location in jasmine tests
+  app._changeLocation = function() { /* noop */ };
+  Diaspora.Mobile.changeLocation = function() { /* noop */ };
+
   // add custom matchers for flash messages
   jasmine.addMatchers(customMatchers);
 

spec/javascripts/mobile/mobile_conversations_spec.js

@@ -0,0 +1,52 @@
+describe("Diaspora.Mobile.Conversations", function() {
+  beforeEach(function() {
+    spec.loadFixture("conversations_new_mobile");
+    Diaspora.Page = "ConversationsNew";
+  });
+
+  describe("conversationCreateSuccess", function() {
+    it("is called when there was a successful ajax request for the conversation form", function() {
+      spyOn(Diaspora.Mobile.Conversations, "conversationCreateSuccess");
+      Diaspora.Mobile.Conversations.initialize();
+
+      $("#flash-messages").trigger("ajax:success", [{id: 23}]);
+      expect(Diaspora.Mobile.Conversations.conversationCreateSuccess).not.toHaveBeenCalled();
+
+      $("#new-conversation").trigger("ajax:error", [{responseText: "error"}]);
+      expect(Diaspora.Mobile.Conversations.conversationCreateSuccess).not.toHaveBeenCalled();
+
+      $("#new-conversation").trigger("ajax:success", [{id: 23}]);
+      expect(Diaspora.Mobile.Conversations.conversationCreateSuccess).toHaveBeenCalled();
+    });
+
+    it("redirects to the new conversation", function() {
+      spyOn(Diaspora.Mobile, "changeLocation");
+      Diaspora.Mobile.Conversations.initialize();
+      $("#new-conversation").trigger("ajax:success", [{id: 23}]);
+      expect(Diaspora.Mobile.changeLocation).toHaveBeenCalledWith(Routes.conversation(23));
+    });
+  });
+
+  describe("conversationCreateError", function() {
+    it("is called when an ajax request failed for the conversation form", function() {
+      spyOn(Diaspora.Mobile.Conversations, "conversationCreateError");
+      Diaspora.Mobile.Conversations.initialize();
+
+      $("#flash-messages").trigger("ajax:error", [{responseText: "error"}]);
+      expect(Diaspora.Mobile.Conversations.conversationCreateError).not.toHaveBeenCalled();
+
+      $("#new-conversation").trigger("ajax:success", [{id: 23}]);
+      expect(Diaspora.Mobile.Conversations.conversationCreateError).not.toHaveBeenCalled();
+
+      $("#new-conversation").trigger("ajax:error", [{responseText: "error"}]);
+      expect(Diaspora.Mobile.Conversations.conversationCreateError).toHaveBeenCalled();
+    });
+
+    it("shows a flash message", function() {
+      spyOn(Diaspora.Mobile.Alert, "error");
+      Diaspora.Mobile.Conversations.initialize();
+      $("#new-conversation").trigger("ajax:error", [{responseText: "Oh noez! Something went wrong!"}]);
+      expect(Diaspora.Mobile.Alert.error).toHaveBeenCalledWith("Oh noez! Something went wrong!");
+    });
+  });
+});