Validate presence of message text on conversations. Closes #1329.

app/controllers/conversations_controller.rb

@@ -31,15 +31,17 @@ class ConversationsController < ApplicationController
     message_text = params[:conversation].delete(:text)
     params[:conversation][:messages_attributes] = [ {:author => current_user.person, :text => message_text }]
 
-    if @conversation = Conversation.create(params[:conversation])
+    @conversation = Conversation.new(params[:conversation])
+    if @conversation.save
       Postzord::Dispatch.new(current_user, @conversation).post
-
       flash[:notice] = I18n.t('conversations.create.sent')
-      if params[:profile]
+    else
-        redirect_to person_path(params[:profile])
+      flash[:error] = I18n.t('conversations.create.fail')
-      else
+    end
-        redirect_to conversations_path(:conversation_id => @conversation.id)
+    if params[:profile]
-      end
+      redirect_to person_path(params[:profile])
+    else
+      redirect_to conversations_path(:conversation_id => @conversation.id)
     end
   end
 

app/controllers/messages_controller.rb

@@ -19,11 +19,10 @@ class MessagesController < ApplicationController
       if message.save
         Rails.logger.info("event=create type=comment user=#{current_user.diaspora_handle} status=success message=#{message.id} chars=#{params[:message][:text].length}")
         Postzord::Dispatch.new(current_user, message).post
-
-        redirect_to conversations_path(:conversation_id => cnv.id)
       else
-        render :nothing => true, :status => 422
+        flash[:error] = I18n.t('conversations.new_message.fail')
       end
+      redirect_to conversations_path(:conversation_id => cnv.id)
     else
       render :nothing => true, :status => 422
     end

app/models/message.rb

@@ -14,6 +14,8 @@ class Message < ActiveRecord::Base
   belongs_to :author, :class_name => 'Person'
   belongs_to :conversation, :touch => true
 
+  validates_presence_of :text
+
   after_create do
     #sign comment as commenter
     self.author_signature = self.sign_with_key(self.author.owner.encryption_key) if self.author.owner

config/locales/diaspora/en.yml

@@ -257,6 +257,9 @@ en:
         other: "%{count} new messages"
     create:
       sent: "Message sent"
+      fail: "Invalid message"
+    new_message:
+      fail: "Invalid message"
     destroy:
       success: "Conversation successfully removed"
 

features/conversations.feature

@@ -16,3 +16,7 @@ Feature: private messages
     And I should see "Greetings" within "#conversation_show"
     And "Alice Awesome" should be part of active conversation
     And I should see "hello, alice!" within ".stream"
+
+  Scenario: send an empty message
+    Given I send a message with subject "Greetings" and text " " to "Alice Awesome"
+    Then I should not see "Greetings" within "#conversation_inbox"

spec/controllers/conversations_controller_spec.rb

@@ -54,47 +54,75 @@ describe ConversationsController do
   end
 
   describe '#create' do
-    before do
+    context 'with a valid conversation' do
-      @hash = {
+      before do
-        :conversation => {
+        @hash = {
-          :subject => "secret stuff",
+          :conversation => {
-          :text => 'text'},
+            :subject => "secret stuff",
-        :contact_ids => [alice.contacts.first.id]
+            :text => 'text debug'
-      }
+          },
-    end
+          :contact_ids => [alice.contacts.first.id]
-
-    it 'creates a conversation' do
-      lambda {
-        post :create, @hash
-      }.should change(Conversation, :count).by(1)
-    end
-
-    it 'creates a message' do
-      lambda {
-        post :create, @hash
-      }.should change(Message, :count).by(1)
-    end
-
-    it 'sets the author to the current_user' do
-      @hash[:author] = Factory.create(:user)
-      post :create, @hash
-      Message.first.author.should == alice.person
-      Conversation.first.author.should == alice.person
-    end
-
-    it 'dispatches the conversation' do
-      cnv = Conversation.create(
-        {
-          :author => alice.person,
-          :participant_ids => [alice.contacts.first.person.id, alice.person.id],
-          :subject => 'not spam',
-          :messages_attributes => [ {:author => alice.person, :text => 'cool stuff'} ]
         }
-      )
+      end
-      p = Postzord::Dispatch.new(alice, cnv)
+
-      Postzord::Dispatch.stub!(:new).and_return(p)
+      it 'creates a conversation' do
-      p.should_receive(:post)
+        lambda {
-      post :create, @hash
+          post :create, @hash
+        }.should change(Conversation, :count).by(1)
+      end
+
+      it 'creates a message' do
+        lambda {
+          post :create, @hash
+        }.should change(Message, :count).by(1)
+      end
+
+      it 'sets the author to the current_user' do
+        @hash[:author] = Factory.create(:user)
+        post :create, @hash
+        Message.first.author.should == alice.person
+        Conversation.first.author.should == alice.person
+      end
+
+      it 'dispatches the conversation' do
+        cnv = Conversation.create(
+          {
+            :author => alice.person,
+            :participant_ids => [alice.contacts.first.person.id, alice.person.id],
+            :subject => 'not spam',
+            :messages_attributes => [ {:author => alice.person, :text => 'cool stuff'} ]
+          }
+        )
+
+        p = Postzord::Dispatch.new(alice, cnv)
+        Postzord::Dispatch.stub!(:new).and_return(p)
+        p.should_receive(:post)
+        post :create, @hash
+      end
+    end
+
+    context 'with empty text' do
+      before do
+        @hash = {
+          :conversation => {
+            :subject => 'secret stuff',
+            :text => '  '
+          },
+          :contact_ids => [alice.contacts.first.id]
+        }
+      end
+
+      it 'does not create a conversation' do
+        lambda {
+          post :create, @hash
+        }.should_not change(Conversation, :count).by(1)
+      end
+
+      it 'does not create a message' do
+        lambda {
+          post :create, @hash
+        }.should_not change(Message, :count).by(1)
+      end
     end
   end
 

spec/controllers/messages_controller_spec.rb

@@ -28,15 +28,34 @@ describe MessagesController do
     context "on my own post" do
       before do
         @cnv = Conversation.create(@create_hash)
-        @message_hash = {:conversation_id => @cnv.id, :message => {:text => "here is something else"}}
       end
 
-      it 'redirects to conversation' do
+      context "with a valid message" do
-        lambda{
+        before do
-          post :create, @message_hash
+          @message_hash = {:conversation_id => @cnv.id, :message => {:text => "here is something else"}}
-        }.should change(Message, :count).by(1)
+        end
-        response.code.should == '302'
+
-        response.should redirect_to(conversations_path(:conversation_id => @cnv))
+        it 'redirects to conversation' do
+          lambda{
+            post :create, @message_hash
+          }.should change(Message, :count).by(1)
+          response.code.should == '302'
+          response.should redirect_to(conversations_path(:conversation_id => @cnv))
+        end
+      end
+
+      context "with an empty message" do
+        before do
+          @message_hash = {:conversation_id => @cnv.id, :message => {:text => " "}}
+        end
+
+        it 'redirects to conversation' do
+          lambda{
+            post :create, @message_hash
+          }.should_not change(Message, :count).by(1)
+          response.code.should == '302'
+          response.should redirect_to(conversations_path(:conversation_id => @cnv))
+        end
       end
     end