Merge pull request #3748 from rschaden/3271-invitation-email-validation

3271 invitation email validation

app/controllers/invitations_controller.rb

@@ -10,6 +10,10 @@ class InvitationsController < ApplicationController
 
   def new
     @invite_code = current_user.invitation_code
+
+    @invalid_emails = html_safe_string_from_session_array(:invalid_email_invites)
+    @valid_emails   = html_safe_string_from_session_array(:valid_email_invites)
+
     respond_to do |format|
       format.html do
         render :layout => false
@@ -48,10 +52,32 @@ class InvitationsController < ApplicationController
   end
 
   def create
-    inviter = EmailInviter.new(params[:email_inviter][:emails], current_user, params[:email_inviter])
+    emails = params[:email_inviter][:emails].split(',').map(&:strip).uniq
-    inviter.send!
 
-    redirect_to :back, :notice => t('invitations.create.sent', :emails => inviter.emails.join(', '))
+    valid_emails, invalid_emails = emails.partition { |email| valid_email?(email) }
+
+    session[:valid_email_invites] = valid_emails
+    session[:invalid_email_invites] = invalid_emails
+
+    unless valid_emails.empty?
+      inviter = EmailInviter.new(valid_emails.join(','), current_user,
+                                 params[:email_inviter])
+      inviter.send!
+    end
+
+    if emails.empty?
+      flash[:error] = t('invitations.create.empty')
+    elsif invalid_emails.empty?
+      flash[:notice] =  t('invitations.create.sent', :emails => valid_emails.join(', '))
+    elsif valid_emails.empty?
+      flash[:error] = t('invitations.create.rejected') +  invalid_emails.join(', ')
+    else
+      flash[:error] = t('invitations.create.sent', :emails => valid_emails.join(', '))
+      flash[:error] << '. '
+      flash[:error] << t('invitations.create.rejected') +  invalid_emails.join(', ')
+    end
+
+    redirect_to :back
   end
 
   def check_if_invites_open
@@ -61,4 +87,17 @@ class InvitationsController < ApplicationController
       redirect_to :back
     end
   end
+
+  private
+  def valid_email?(email)
+    User.email_regexp.match(email).present?
+  end
+
+  def html_safe_string_from_session_array(key)
+    return "" unless session[key].present?
+    return "" unless session[key].respond_to?(:join)
+    value = session[key].join(', ').html_safe
+    session[key] = nil
+    return value
+  end
 end

app/views/invitations/new.html.haml

@@ -19,7 +19,9 @@
       = form_tag new_user_invitation_path  do
         %h4
           = t('email')
-        = text_field_tag 'email_inviter[emails]' ,nil, :title => t('.comma_separated_plz'), :placeholder => 'foo@bar.com, max@foo.com...'
+        = text_field_tag 'email_inviter[emails]', @invalid_emails, :title => t('.comma_separated_plz'), :placeholder => 'foo@bar.com, max@foo.com...'
+        %p
+          = t('invitations.create.note_already_sent', :emails => @valid_emails) unless @valid_emails.empty?
         %br
 
         %h4

config/locales/diaspora/en.yml

@@ -362,6 +362,8 @@ en:
       already_sent: "You already invited this person."
       already_contacts: "You are already connected with this person"
       own_address: "You can't send an invitation to your own address."
+      empty: "Please enter at least one email address."
+      note_already_sent: "Invitations have already been sent to: %{emails}"
     new:
       language: "Language"
       invite_someone_to_join: "Invite someone to join Diaspora!"

spec/controllers/invitations_controller_spec.rb

@@ -16,17 +16,111 @@ describe InvitationsController do
     before do
       sign_in :user, @user
       @controller.stub!(:current_user).and_return(@user)
-      request.env["HTTP_REFERER"]= 'http://test.host/cats/foo'
+      @referer = 'http://test.host/cats/foo'
+      request.env["HTTP_REFERER"] = @referer
     end
 
-    it 'creates an EmailInviter'  do
+    context "no emails" do
-      inviter = stub(:emails => ['mbs@gmail.com'], :send! => true)
+      before do
-      EmailInviter.should_receive(:new).with(@invite['email_inviter']['emails'], @user, @invite['email_inviter']).
+        @invite = {'email_inviter' => {'message' => "test", 'emails' => ""}}
-        and_return(inviter)
+      end
-      post :create,  @invite
+
+      it 'does not create an EmailInviter' do
+        EmailInviter.should_not_receive(:new)
+        post :create,  @invite
+      end
+
+      it 'returns to the previous page' do
+        post :create, @invite
+        response.should redirect_to @referer
+      end
+
+      it 'flashes an error' do
+        post :create, @invite
+        flash[:error].should == I18n.t("invitations.create.empty")
+      end
     end
 
-    it "redirects if invitations are closed" do
+    context 'only valid emails' do
+      before do
+        @emails = 'mbs@gmail.com'
+        @invite = {'email_inviter' => {'message' => "test", 'emails' => @emails}}
+      end
+
+      it 'creates an EmailInviter'  do
+        inviter = stub(:emails => [@emails], :send! => true)
+        EmailInviter.should_receive(:new).with(@invite['email_inviter']['emails'], @user, @invite['email_inviter']).
+          and_return(inviter)
+        post :create,  @invite
+      end
+
+      it 'returns to the previous page on success' do
+        post :create, @invite
+        response.should redirect_to @referer
+      end
+
+      it 'flashes a notice' do
+        post :create, @invite
+        expected =  I18n.t('invitations.create.sent', :emails => @emails.split(',').join(', '))
+        flash[:notice].should == expected
+      end
+    end
+
+    context 'only invalid emails' do
+      before do
+        @emails = 'invalid_email'
+        @invite = {'email_inviter' => {'message' => "test", 'emails' => @emails}}
+      end
+
+      it 'does not create an EmailInviter' do
+        EmailInviter.should_not_receive(:new)
+        post :create,  @invite
+      end
+
+      it 'returns to the previous page' do
+        post :create, @invite
+        response.should redirect_to @referer
+      end
+
+      it 'flashes an error' do
+        post :create, @invite
+
+        expected =  I18n.t('invitations.create.rejected') + @emails.split(',').join(', ')
+        flash[:error].should == expected
+      end
+    end
+
+    context 'mixed valid and invalid emails' do
+      before do
+        @valid_emails = 'foo@bar.com,mbs@gmail.com'
+        @invalid_emails = 'invalid'
+        @invite = {'email_inviter' => {'message' => "test", 'emails' =>
+                                       @valid_emails + ',' + @invalid_emails}}
+      end
+
+      it 'creates an EmailInviter'  do
+        inviter = stub(:emails => [@emails], :send! => true)
+        EmailInviter.should_receive(:new).with(@valid_emails, @user, @invite['email_inviter']).
+          and_return(inviter)
+        post :create,  @invite
+      end
+
+      it 'returns to the previous page' do
+        post :create, @invite
+        response.should redirect_to @referer
+      end
+
+      it 'flashes a notice' do
+        post :create, @invite
+        expected =  I18n.t('invitations.create.sent', :emails =>
+                          @valid_emails.split(',').join(', ')) +
+                          '. ' + I18n.t('invitations.create.rejected') +
+                          @invalid_emails.split(',').join(', ')
+        flash[:error].should == expected
+      end
+    end
+
+    it 'redirects if invitations are closed' do
       open_bit = AppConfig.settings.invitations.open?
       AppConfig.settings.invitations.open =  false
 
@@ -34,11 +128,6 @@ describe InvitationsController do
       response.should be_redirect
       AppConfig.settings.invitations.open = open_bit
     end
-
-    it 'returns to the previous page on success' do
-      post :create, @invite
-      response.should redirect_to("http://test.host/cats/foo")
-    end
   end
 
   describe '#email' do
@@ -92,4 +181,40 @@ describe InvitationsController do
       response.should redirect_to new_user_session_path
     end
   end
+
+  describe '.valid_email?' do
+    it 'returns false for empty email' do
+      subject.send(:valid_email?, '').should be false
+    end
+
+    it 'returns false for email without @-sign' do
+      subject.send(:valid_email?, 'foo').should be false
+    end
+
+    it 'returns true for valid email' do
+      subject.send(:valid_email?, 'foo@bar.com').should be true
+    end
+  end
+
+  describe '.html_safe_string_from_session_array' do
+    it 'returns "" for blank session[key]' do
+      subject.send(:html_safe_string_from_session_array, :blank).should eq ""
+    end
+
+    it 'returns "" if session[key] is not an array' do
+      session[:test_key] = "test"
+      subject.send(:html_safe_string_from_session_array, :test_key).should eq ""
+    end
+
+    it 'returns the correct value' do
+      session[:test_key] = ["test", "foo"]
+      subject.send(:html_safe_string_from_session_array, :test_key).should eq "test, foo"
+    end
+
+    it 'sets session[key] to nil' do
+      session[:test_key] = ["test"]
+      subject.send(:html_safe_string_from_session_array, :test_key)
+      session[:test_key].should be nil
+    end
+  end
 end