Merge branch 'develop' into move_packs

app/controllers/account_controller.rb

@@ -21,11 +21,9 @@ def signup
       return
     end
 
-    return render_user_errors if @user.errors.any?
-    #warn Rails.logger.warn("signup UA:#{user_args.inspect}, CA:#{card_args.inspect}")
+    return user_errors if @user.errors.any?
     @user, @card = User.create_with_card( user_args, card_args )
-    #warn Rails.logger.warn("signup UA:#{@user.inspect}, CA:#{@card.inspect}")
-    return render_user_errors if @user.errors.any?
+    return user_errors if @user.errors.any?
 
     tr_card = @card.trait_card :account
     #warn "check for account #{@card.name} #{tr_card.inspect}"
@@ -42,13 +40,6 @@ def signup
     end
   end
 
-  def render_user_errors
-    @card.errors += @user.errors
-    errors
-  end
-
-
-
   def accept
     card_key=params[:card][:key]
     #warn "accept #{card_key.inspect}, #{Card[card_key]}, #{params.inspect}"
@@ -125,9 +116,9 @@ def forgot_password
 
   protected
 
-  def render_user_errors
+  def user_errors
     @user.errors.each do |field, err|
-      @card.errors.add field, err unless @card.errors[field]
+      @card.errors.add field, err unless @card.errors[field].any?
       # needed to prevent duplicates because User adds them in the other direction in user.rb
     end
     errors

app/models/user.rb

@@ -19,7 +19,7 @@ class User < ActiveRecord::Base
   validates_presence_of     :password_confirmation,      :if => :password_required?
   validates_length_of       :password, :within => 5..40, :if => :password_required?
   validates_confirmation_of :password,                   :if => :password_required?
-  validates_presence_of     :invite_sender,              :if => :active?
+#  validates_presence_of     :invite_sender,              :if => :active?
 #  validates_uniqueness_of   :salt, :allow_nil => true
 
   before_validation :downcase_email!
@@ -34,14 +34,10 @@ def from_id(card_id) User.where(:card_id=>card_id).first         end
     def cache()          Wagn::Cache[User]                           end
 
     # FIXME: args=params.  should be less coupled..
-    def create_with_card(user_args, card_args, email_args={})
-      #warn  "create with(#{user_args.inspect}, #{card_args.inspect}, #{email_args.inspect})"
+    def create_with_card user_args, card_args, email_args={}
       card_args[:type_id] ||= Card::UserID
       @card = Card.fetch_or_new(card_args[:name], card_args)
-      #warn "create with >>#{Session.user_card.name}"
-      #warn "create with args= #{({:invite_sender=>Session.user_card, :status=>'active'}.merge(user_args)).inspect}"
       Session.as_bot do
-        #warn "cwa #{user_args.inspect}, #{card_args.inspect}"
         @user = User.new({:invite_sender=>Session.user_card, :status=>'active'}.merge(user_args))
         #warn "user is #{@user.inspect}" unless @user.email
         @user.generate_password if @user.password.blank?
@@ -94,28 +90,21 @@ def reset_instance_cache
     self.class.cache.write(login, nil) if login
   end
 
-  def save_with_card(card)
-    #warn(Rails.logger.info "save with card #{card.inspect}, #{self.inspect}")
+  def save_with_card card
     User.transaction do
       card = card.refresh
-      newcard = card.new_card?
-      card.save
-      #warn "save with_card #{User.count}, #{card.id}, #{card.inspect}"
+      if card.save
+        self.card_id = card.id
+        save
+      else
+        valid?
+      end
       card.errors.each do |key,err|
         self.errors.add key,err
       end
-      self.card_id = card.id
-      save
-      if newcard && errors.any?
-        card.delete #won't the rollback take care of this?  if not, should Wagn Bot do it?
-        self.card_id=nil
-        save
-        raise ActiveRecord::Rollback
-      end
+      raise ActiveRecord::Rollback if errors.any?
       true
     end
-  rescue Exception => e
-    warn (Rails.logger.info "save with card failed. #{e.inspect},  #{card.inspect} Bt:#{e.backtrace*"\n"}")
   end
 
   def accept(card, email_args)

spec/controllers/account_controller_spec.rb

@@ -46,30 +46,40 @@
   describe "#signup" do
     before do
       @msgs=[]
-      mock.proxy(Mailer).account_info.with_any_args.times(any_times) { |m|
+      mock.proxy(Mailer).account_info.with_any_args.times(any_times) do |m|
         @msgs << m
-        mock(m).deliver }
-
-      post :signup, :user=>{:email=>'joe@new.com'}, :card=>{:name=>'Joe New'}
-
-      @new_user = User.where(:email=>'joe@new.com').first
-      @user_card = Card['Joe New']
-
+        mock(m).deliver
+      end
     end
 
+    #FIXME: tests needed : signup without approval, signup alert emails
+
+
     it 'should create a user' do
-      @new_user.should be
-      @new_user.card_id.should == @user_card.id
-      @user_card.type_id.should == Card::AccountRequestID
+      post :signup, :user=>{:email=>'joe@new.com'}, :card=>{:name=>'Joe New'}
+      new_user = User.where(:email=>'joe@new.com').first
+      user_card = Card['Joe New']
+      new_user.should be
+      new_user.card_id.should == user_card.id
+      user_card.type_id.should == Card::AccountRequestID
     end
 
     it 'should send email' do
+      post :signup, :user=>{:email=>'joe@new.com'}, :card=>{:name=>'Joe New'}
       login_as :joe_admin
 
       post :accept, :card=>{:key=>'joe_new'}, :email=>{:subject=>'Hey Joe!', :message=>'Can I Come on in?'}
 
       @msgs.size.should == 1
       @msgs[0].should be_a Mail::Message
+      #puts "msg looks like #{@msgs[0].inspect}"
+    end
+
+    it 'should detect duplicates' do
+      post :signup, :user=>{:email=>'joe@user.com'}, :card=>{:name=>'Joe Scope'}
+      post :signup, :user=>{:email=>'joe@user.com'}, :card=>{:name=>'Joe Duplicate'}
+
+      Card['Joe Duplicate'].should be_nil
     end
   end