From 49d4079065098e4c8c2d436bf8f08f83ec0517b0 Mon Sep 17 00:00:00 2001 From: rschaden Date: Sun, 18 Nov 2012 13:22:18 +0100 Subject: [PATCH] add email validation to invitations --- app/controllers/invitations_controller.rb | 45 +++++- app/views/invitations/new.html.haml | 6 +- config/locales/diaspora/en.yml | 16 +- .../invitations_controller_spec.rb | 149 ++++++++++++++++-- 4 files changed, 192 insertions(+), 24 deletions(-) diff --git a/app/controllers/invitations_controller.rb b/app/controllers/invitations_controller.rb index a73d7c5b1..984c6ff5e 100644 --- a/app/controllers/invitations_controller.rb +++ b/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]) - inviter.send! + emails = params[:email_inviter][:emails].split(',').map(&:strip).uniq - 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 diff --git a/app/views/invitations/new.html.haml b/app/views/invitations/new.html.haml index 4f25dbf68..422689f1d 100644 --- a/app/views/invitations/new.html.haml +++ b/app/views/invitations/new.html.haml @@ -19,11 +19,13 @@ = 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 - = t('.language') + = t('.language') = select_tag('email_inviter[locale]', options_from_collection_for_select(available_language_options, "second", "first", :selected => current_user.language)) %br diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 8bb171971..1c206b0f0 100644 --- a/config/locales/diaspora/en.yml +++ b/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!" @@ -519,18 +521,18 @@ en: invite: message: |- Hello! - + You have been invited to join Diaspora*! - + Click this link to get started - + [%{invite_url}][1] - - + + Love, - + The Diaspora* email robot! - + [1]: %{invite_url} people: zero: "no people" diff --git a/spec/controllers/invitations_controller_spec.rb b/spec/controllers/invitations_controller_spec.rb index 413bbe822..5d3cc62ed 100644 --- a/spec/controllers/invitations_controller_spec.rb +++ b/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 - inviter = stub(:emails => ['mbs@gmail.com'], :send! => true) - EmailInviter.should_receive(:new).with(@invite['email_inviter']['emails'], @user, @invite['email_inviter']). - and_return(inviter) - post :create, @invite + context "no emails" do + before do + @invite = {'email_inviter' => {'message' => "test", '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 + 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