diff --git a/app/assets/javascripts/app/views/conversations_form_view.js b/app/assets/javascripts/app/views/conversations_form_view.js index ebd59f1b4..bfdfe62fb 100644 --- a/app/assets/javascripts/app/views/conversations_form_view.js +++ b/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 - diff --git a/app/assets/javascripts/mobile/mobile.js b/app/assets/javascripts/mobile/mobile.js index f7f7a2a45..5ae8717e3 100644 --- a/app/assets/javascripts/mobile/mobile.js +++ b/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 diff --git a/app/assets/javascripts/mobile/mobile_alert.js b/app/assets/javascripts/mobile/mobile_alert.js new file mode 100644 index 000000000..67e23a58a --- /dev/null +++ b/app/assets/javascripts/mobile/mobile_alert.js @@ -0,0 +1,19 @@ +(function() { + Diaspora.Mobile.Alert = { + _flash: function(message, type) { + var html = ""; + $("#flash-messages").append(html); + }, + + success: function(message) { this._flash(message, "success"); }, + + error: function(message) { this._flash(message, "danger"); } + }; +})(); diff --git a/app/assets/javascripts/mobile/mobile_application.js b/app/assets/javascripts/mobile/mobile_application.js index e1d3c64d1..2723e8bef 100644 --- a/app/assets/javascripts/mobile/mobile_application.js +++ b/app/assets/javascripts/mobile/mobile_application.js @@ -8,6 +8,10 @@ // init autosize plugin autosize($("textarea")); + }, + + changeLocation: function(href) { + window.location.assign(href); } }; })(); diff --git a/app/assets/javascripts/mobile/mobile_conversations.js b/app/assets/javascripts/mobile/mobile_conversations.js new file mode 100644 index 000000000..4fef1b5b3 --- /dev/null +++ b/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(); +}); diff --git a/app/controllers/conversations_controller.rb b/app/controllers/conversations_controller.rb index 1a09e9d9d..ba99b6b6a 100644 --- a/app/controllers/conversations_controller.rb +++ b/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 diff --git a/app/views/conversations/_new.haml b/app/views/conversations/_new.haml index e8ab11d37..41fb777aa 100644 --- a/app/views/conversations/_new.haml +++ b/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") diff --git a/app/views/conversations/create.js.erb b/app/views/conversations/create.js.erb deleted file mode 100644 index 3310fcb5b..000000000 --- a/app/views/conversations/create.js.erb +++ /dev/null @@ -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 %> diff --git a/app/views/conversations/index.mobile.haml b/app/views/conversations/index.mobile.haml index fe6eefa32..6bb7804db 100644 --- a/app/views/conversations/index.mobile.haml +++ b/app/views/conversations/index.mobile.haml @@ -7,10 +7,8 @@ = link_to t(".new_conversation"), new_conversation_path, class: "btn btn-default pull-right" - flash.each do |name, msg| - %div{ id: "flash_#{name}", class: "expose" } - .message= msg - .stream - %p{ class: "conversation_#{name}" }= msg + .alert{class: "alert-#{flash_class name}", role: "alert"} + = msg .conversation-inbox#conversation-inbox .stream.conversations diff --git a/app/views/conversations/new.mobile.haml b/app/views/conversations/new.mobile.haml index 6a7a1c0da..ba78c558f 100644 --- a/app/views/conversations/new.mobile.haml +++ b/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') diff --git a/features/mobile/conversations.feature b/features/mobile/conversations.feature index e88842f91..9fe2e23e0 100644 --- a/features/mobile/conversations.feature +++ b/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 diff --git a/spec/controllers/conversations_controller_spec.rb b/spec/controllers/conversations_controller_spec.rb index 5ba05c98f..0002468de 100644 --- a/spec/controllers/conversations_controller_spec.rb +++ b/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 diff --git a/spec/controllers/jasmine_fixtures/conversations_spec.rb b/spec/controllers/jasmine_fixtures/conversations_spec.rb index 4c4d317b7..c07360d69 100644 --- a/spec/controllers/jasmine_fixtures/conversations_spec.rb +++ b/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 diff --git a/spec/javascripts/app/views/conversations_form_view_spec.js b/spec/javascripts/app/views/conversations_form_view_spec.js index 79305115a..989469670 100644 --- a/spec/javascripts/app/views/conversations_form_view_spec.js +++ b/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!"); }); }); }); diff --git a/spec/javascripts/jasmine_helpers/SpecHelper.js b/spec/javascripts/jasmine_helpers/SpecHelper.js index 531675210..7fa076858 100644 --- a/spec/javascripts/jasmine_helpers/SpecHelper.js +++ b/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); diff --git a/spec/javascripts/mobile/mobile_alert_spec.js b/spec/javascripts/mobile/mobile_alert_spec.js new file mode 100644 index 000000000..ffb789ef7 --- /dev/null +++ b/spec/javascripts/mobile/mobile_alert_spec.js @@ -0,0 +1,29 @@ +describe("Diaspora.Mobile.Alert", function() { + describe("_flash", function() { + beforeEach(function() { + spec.content().html("
"); + }); + + it("appends an alert to the #flash-messages div", function() { + Diaspora.Mobile.Alert._flash("Oh snap! You got an error!", "error-class"); + expect($("#flash-messages .alert")).toHaveClass("alert-error-class"); + expect($("#flash-messages .alert").text()).toBe("Oh snap! You got an error!"); + }); + }); + + describe("success", function() { + it("calls _flash", function() { + spyOn(Diaspora.Mobile.Alert, "_flash"); + Diaspora.Mobile.Alert.success("Awesome!"); + expect(Diaspora.Mobile.Alert._flash).toHaveBeenCalledWith("Awesome!", "success"); + }); + }); + + describe("error", function() { + it("calls _flash", function() { + spyOn(Diaspora.Mobile.Alert, "_flash"); + Diaspora.Mobile.Alert.error("Oh noez!"); + expect(Diaspora.Mobile.Alert._flash).toHaveBeenCalledWith("Oh noez!", "danger"); + }); + }); +}); diff --git a/spec/javascripts/mobile/mobile_conversations_spec.js b/spec/javascripts/mobile/mobile_conversations_spec.js new file mode 100644 index 000000000..34281e27f --- /dev/null +++ b/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!"); + }); + }); +});