fixed up update controller action and building users #scary #gladsofaerisstillhere

app/controllers/invitations_controller.rb

@@ -21,7 +21,7 @@ class InvitationsController < Devise::InvitationsController
     message = params[:user].delete(:invite_messages)
     emails = params[:user][:email].to_s.gsub(/\s/, '').split(/, */)
     #NOTE should we try and find users by email here? probs
-    aspect = Aspect.find(aspect_id)
+    aspect = current_user.aspects.find(aspect_id)
     invites = Invitation.batch_build(:sender => current_user, :aspect => aspect, :emails => emails, :service => 'email')
 
     flash[:notice] = extract_messages(invites)
@@ -30,29 +30,22 @@ class InvitationsController < Devise::InvitationsController
   end
 
   def update
-    begin
       invitation_token = params[:user][:invitation_token]
+
       if invitation_token.nil? || invitation_token.blank?
         raise I18n.t('invitations.check_token.not_found')
       end
-      user = User.find_by_invitation_token(params[:user][:invitation_token])
+
+      user = User.find_by_invitation_token!(invitation_token)
+      
       user.accept_invitation!(params[:user])
-      user.seed_aspects
-    rescue Exception => e #What exception is this trying to rescue?  If it is ActiveRecord::NotFound, we should say so.
-      raise e 
-      user = nil
-      record = e.record
-      record.errors.delete(:person)
 
-      flash[:error] = record.errors.full_messages.join(", ")
+      if user.persisted? && user.person && user.person.persisted?
-    end
+        user.seed_aspects
-
+        flash[:notice] = I18n.t 'registrations.create.success'
-    if user
+        sign_in_and_redirect(:user, user)
-      flash[:notice] = I18n.t 'registrations.create.success'
-      sign_in_and_redirect(:user, user)
     else
-      redirect_to accept_user_invitation_path(
+      redirect_to accept_user_invitation_path(:invitation_token => params[:user][:invitation_token]), :error => user.errors.full_messages.join(", ")
-        :invitation_token => params[:user][:invitation_token])
     end
   end
 

app/models/invitation.rb

@@ -1,4 +1,4 @@
-#   Copyright (c) 2010, Diaspora Inc.  This file is
+#   Copyright (c) 2011, Diaspora Inc.  This file is
 #   licensed under the Affero General Public License version 3 or later.  See
 #   the COPYRIGHT file.
 
@@ -14,10 +14,10 @@ class Invitation < ActiveRecord::Base
   attr_accessible :sender, :recipient, :aspect, :service, :identifier, :admin
 
   before_validation :set_email_as_default_service
-  validate :ensure_not_inviting_self, :on => :create
+  validate :ensure_not_inviting_self, :on => :create, :unless => :admin?
 
   validate :valid_identifier?
-  validate :sender_owns_aspect?
+  validate :sender_owns_aspect?, :unless => :admin?
   validates_uniqueness_of :sender_id, :scope => [:identifier, :service], :unless => :admin?
 
   after_create :queue_send! #TODO make this after_commit :queue_saved!, :on => :create
@@ -117,15 +117,15 @@ class Invitation < ActiveRecord::Base
 
   # @note Validation
   def ensure_not_inviting_self
-    if !self.admin? && self.identifier == self.sender.email
+    if self.identifier == self.sender.email
-      errors[:base] << 'You can not invite yourself'
+      errors[:base] << 'You can not invite yourself.'
     end
   end  
 
   # @note Validation
   def sender_owns_aspect?
-    unless(self.sender && (self.sender_id == self.aspect.user_id))
+    if self.sender_id != self.aspect.user_id
-      errors[:base] << 'You do not own that aspect'
+      errors[:base] << 'You do not own that aspect.'
     end
   end
 

app/models/person.rb

@@ -19,6 +19,7 @@ class Person < ActiveRecord::Base
 
   has_one :profile, :dependent => :destroy
   delegate :last_name, :to => :profile
+  accepts_nested_attributes_for :profile
 
   before_validation :downcase_diaspora_handle
   def downcase_diaspora_handle
@@ -51,6 +52,20 @@ class Person < ActiveRecord::Base
     AppConfig[:featured_users].present? ? Person.where(:diaspora_handle => AppConfig[:featured_users]) : []
   end
 
+  # Set a default of an empty profile when a new Person record is instantiated.
+  # Passing :profile => nil to Person.new will instantiate a person with no profile.
+  # Calling Person.new with a block:
+  #   Person.new do |p|
+  #     p.profile = nil
+  #   end
+  # will not work!  The nil profile will be overriden with an empty one.
+  def initialize(params={})
+    profile_set = params.has_key?(:profile) || params.has_key?("profile")
+    params[:profile_attributes] = params.delete(:profile) if params.has_key?(:profile) && params[:profile].is_a?(Hash)
+    super
+    self.profile ||= Profile.new unless profile_set
+  end
+  
   
   def self.find_from_id_or_username(params)
     p =   if params[:id].present?

app/models/user.rb

@@ -328,7 +328,9 @@ class User < ActiveRecord::Base
       self.invitation_token = nil
       self.password              = opts[:password]
       self.password_confirmation = opts[:password_confirmation]
-      self.save!
+      
+      self.save
+      return unless self.errors.empty?
 
       # moved old Invitation#share_with! logic into here,
       # but i don't think we want to destroy the invitation
@@ -341,8 +343,6 @@ class User < ActiveRecord::Base
 
       log_hash[:status] = "success"
       Rails.logger.info(log_hash)
-
-      self.reload # Because to_request adds a request and saves elsewhere
       self
     end
   end
@@ -362,23 +362,17 @@ class User < ActiveRecord::Base
     errors = self.errors
     errors.delete :person
     return if errors.size > 0
-
+    self.set_person(Person.new(opts[:person] || {} ))
-    opts[:person] ||= {}
-    unless opts[:person][:profile].is_a?(Profile)
-      opts[:person][:profile] ||= Profile.new
-      opts[:person][:profile] = Profile.new(opts[:person][:profile])
-    end
-
-    #TODO make this User#person=
-    self.person = Person.new(opts[:person])
-    self.person.diaspora_handle = "#{opts[:username]}@#{AppConfig[:pod_uri].authority}"
-    self.person.url = AppConfig[:pod_url]
-
     self.generate_keys
-
     self
   end
 
+  def set_person(person)
+    person.url = AppConfig[:pod_url]
+    person.diaspora_handle = "#{self.username}@#{AppConfig[:pod_uri].authority}"
+    self.person = person
+  end
+
   def seed_aspects
     self.aspects.create(:name => I18n.t('aspects.seed.family'))
     self.aspects.create(:name => I18n.t('aspects.seed.friends'))

spec/factories.rb

@@ -21,7 +21,7 @@ Factory.define :person do |p|
   p.sequence(:url)  { |n| AppConfig[:pod_url] }
   p.serialized_public_key OpenSSL::PKey::RSA.generate(1024).public_key.export
   p.after_build do |person|
-    person.profile ||= Factory.build(:profile, :person => person)
+    person.profile = Factory.build(:profile, :person => person) unless person.profile.first_name.present?
   end
   p.after_create do |person|
     person.profile.save

spec/models/invitation_spec.rb

@@ -1,4 +1,4 @@
-#   Copyright (c) 2010, Diaspora Inc.  This file is
+#   Copyright (c) 2011, Diaspora Inc.  This file is
 #   licensed under the Affero General Public License version 3 or later.  See
 #   the COPYRIGHT file.
 

spec/models/person_spec.rb

@@ -11,6 +11,15 @@ describe Person do
     @person = Factory.create(:person)
   end
 
+  it 'always has a profile' do
+    Person.new.profile.should_not be_nil
+  end
+
+  it 'does not save automatically' do
+    Person.new.persisted?.should be_false
+    Person.new.profile.persisted?.should be_false
+  end
+
   context 'scopes' do
     describe '.for_json' do
       it 'does not select public keys' do
@@ -144,7 +153,7 @@ describe Person do
     end
   end
 
-  describe 'xml' do
+  describe 'XML' do
     before do
       @xml = @person.to_xml.to_s
     end