fix double render bug when attempting to signup and email sending fails

git-svn-id: http://svn.techno-weenie.net/projects/beast/trunk@2949 567b1171-46fb-0310-a4c9-b4bef9110e78

app/controllers/users_controller.rb

@@ -35,19 +35,22 @@ def create
         flash[:error] = "I could not find an account with the email address '{email}'. Did you type it correctly?"[:could_not_find_account_message, params[:email]] if params[:email] and not @user
         redirect_to login_path and return unless @user
         @user.login = params[:user][:login] unless params[:user].blank?
-        @user.reset_login_key! 
+        @user.reset_login_key
+        @user.save! unless @user.valid? # kinda backwards, but trigger an ActiveRecord::RecordInvalid error if its not valid before attempting to send email
         begin
           UserMailer.deliver_signup(@user, request.host_with_port, params[:to])
         rescue Net::SMTPFatalError => e
           flash[:notice] = "A permanent error occured while sending the signup message to '{email}'. Please check the e-mail address."[:signup_permanent_error_message, @user.email]
-          redirect_to :action => "new"
+          render :action => "new"
         rescue Net::SMTPServerBusy, Net::SMTPUnknownError, \
           Net::SMTPSyntaxError, TimeoutError => e
           flash[:notice] = "The signup message cannot be sent to '{email}' at this moment. Please, try again later."[:signup_cannot_sent_message, @user.email]
-          redirect_to :action => "new"
+          render :action => "new"
+        else
+          @user.save(false)
+          flash[:notice] = params[:email] ? "A temporary login email has been sent to '{email}'."[:temporary_login_message, @user.email] : "An account activation email has been sent to '{email}'."[:account_activation_message, @user.email]
+          redirect_to CGI.unescape(login_path)
         end
-        flash[:notice] = params[:email] ? "A temporary login email has been sent to '{email}'."[:temporary_login_message, @user.email] : "An account activation email has been sent to '{email}'."[:account_activation_message, @user.email]
-        redirect_to CGI.unescape(login_path)
       end
     end
   end

app/models/user.rb

@@ -67,10 +67,14 @@ def openid_url=(value)
     write_attribute :openid_url, value.blank? ? nil : OpenIdAuthentication.normalize_url(value)
   end
 
-  def reset_login_key!
-    self.login_key = Digest::SHA1.hexdigest(Time.now.to_s + password_hash.to_s + rand(123456789).to_s).to_s
+  def reset_login_key
     # this is not currently honored
     self.login_key_expires_at = Time.now.utc+1.year
+    self.login_key = Digest::SHA1.hexdigest(Time.now.to_s + password_hash.to_s + rand(123456789).to_s).to_s
+  end
+
+  def reset_login_key!
+    reset_login_key
     save!
     login_key
   end

test/functional/users_controller_test.rb

@@ -53,6 +53,16 @@ def test_should_create_user
 #    assert_redirected_to user_path(assigns(:user))
   end
 
+  [Net::SMTPFatalError, Net::SMTPServerBusy, Net::SMTPUnknownError, Net::SMTPSyntaxError].each do |exc|
+    define_method "test_should_not_save_user_when_email_raising_#{exc.name.demodulize.underscore}" do
+      UserMailer.expects(:deliver_signup).raises(exc)
+      assert_difference User, :count, 0 do 
+        post :create, :user => { :login => 'nico', :email => 'nico@email.com', :password => 'fooey', :password_confirmation => 'fooey' }
+      end
+      assert_template 'new'
+    end
+  end
+
   def test_should_reset_login_key_for_forgotten_password
     old_key = users(:sam).login_key
     assert_difference User, :count, 0 do

test/test_helper.rb

@@ -2,6 +2,7 @@
 PASSWORD_SALT = 'beast'
 require File.expand_path(File.dirname(__FILE__) + "/../config/environment")
 require 'test_help'
+require 'mocha'
 
 class Test::Unit::TestCase
   self.use_transactional_fixtures = true