From 2d54d9a97e408de3f64be8e180e396dc1a4973a2 Mon Sep 17 00:00:00 2001
From: Gonzalo Rodriguez <gonzalo@wyeworks.com>
Date: Wed, 17 Aug 2011 23:52:31 -0300
Subject: [PATCH] Validate presence of message text on conversations. Closes
#1329.
---
app/controllers/conversations_controller.rb | 16 +--
app/controllers/messages_controller.rb | 5 +-
app/models/message.rb | 2 +
config/locales/diaspora/en.yml | 3 +
features/conversations.feature | 4 +
.../conversations_controller_spec.rb | 100 +++++++++++-------
spec/controllers/messages_controller_spec.rb | 33 ++++--
7 files changed, 110 insertions(+), 53 deletions(-)
diff --git a/app/controllers/conversations_controller.rb b/app/controllers/conversations_controller.rb
index 80d963caa4..93342958fb 100644
--- a/app/controllers/conversations_controller.rb
+++ b/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]
- redirect_to person_path(params[:profile])
- else
- redirect_to conversations_path(:conversation_id => @conversation.id)
- end
+ else
+ flash[:error] = I18n.t('conversations.create.fail')
+ end
+ if params[:profile]
+ redirect_to person_path(params[:profile])
+ else
+ redirect_to conversations_path(:conversation_id => @conversation.id)
end
end
diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index 97f21556c0..c6f6561660 100644
--- a/app/controllers/messages_controller.rb
+++ b/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
diff --git a/app/models/message.rb b/app/models/message.rb
index fb46d91e4e..4d13d78076 100644
--- a/app/models/message.rb
+++ b/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
diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml
index 43eceebb22..93efc0d8ba 100644
--- a/config/locales/diaspora/en.yml
+++ b/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"
diff --git a/features/conversations.feature b/features/conversations.feature
index 55626aa03e..9bf3b6566a 100644
--- a/features/conversations.feature
+++ b/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"
diff --git a/spec/controllers/conversations_controller_spec.rb b/spec/controllers/conversations_controller_spec.rb
index 6ee8b1d595..09c93b4031 100644
--- a/spec/controllers/conversations_controller_spec.rb
+++ b/spec/controllers/conversations_controller_spec.rb
@@ -54,47 +54,75 @@ describe ConversationsController do
end
describe '#create' do
- before do
- @hash = {
- :conversation => {
- :subject => "secret stuff",
- :text => 'text'},
- :contact_ids => [alice.contacts.first.id]
- }
- end
-
- it 'creates a conversation' do
- lambda {
+ context 'with a valid conversation' do
+ before do
+ @hash = {
+ :conversation => {
+ :subject => "secret stuff",
+ :text => 'text debug'
+ },
+ :contact_ids => [alice.contacts.first.id]
+ }
+ end
+
+ 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
- }.should change(Conversation, :count).by(1)
- end
-
- it 'creates a message' do
- lambda {
+ 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
- }.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
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'} ]
+ context 'with empty text' do
+ before do
+ @hash = {
+ :conversation => {
+ :subject => 'secret stuff',
+ :text => ' '
+ },
+ :contact_ids => [alice.contacts.first.id]
}
- )
- p = Postzord::Dispatch.new(alice, cnv)
- Postzord::Dispatch.stub!(:new).and_return(p)
- p.should_receive(:post)
- post :create, @hash
+ 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
diff --git a/spec/controllers/messages_controller_spec.rb b/spec/controllers/messages_controller_spec.rb
index 6a120effd5..36a899b997 100644
--- a/spec/controllers/messages_controller_spec.rb
+++ b/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
- lambda{
- post :create, @message_hash
- }.should change(Message, :count).by(1)
- response.code.should == '302'
- response.should redirect_to(conversations_path(:conversation_id => @cnv))
+ context "with a valid message" do
+ before do
+ @message_hash = {:conversation_id => @cnv.id, :message => {:text => "here is something else"}}
+ end
+
+ 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
--
GitLab