setter for keys for dirty tracking, more robust tests

Just altering identity.keys did not mark identities as changed. Also we now have a sane default for keys.

4 files changed, 31 insertions, 10 deletions

diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index b862590..73531ec 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -6,7 +6,7 @@ class Identity < CouchRest::Model::Base
 
   property :address, LocalEmail
   property :destination, Email
-  property :keys, Hash
+  property :keys, HashWithIndifferentAccess
 
   validate :unique_forward
   validate :alias_available
@@ -27,6 +27,15 @@ class Identity < CouchRest::Model::Base
 
   end
 
+  def keys
+    read_attribute('keys') || HashWithIndifferentAccess.new
+  end
+
+  def set_key(type, value)
+    return if keys[type] == value
+    write_attribute('keys', keys.merge(type => value))
+  end
+
   protected
 
   def unique_forward
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 546d571..c791069 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -59,14 +59,19 @@ class User < CouchRest::Model::Base
   # TODO: Create an alias for the old login when changing the login
   def login=(value)
     write_attribute 'login', value
-    @identity = build_identity
+    if @identity
+      @identity.address = email_address
+      @identity.destination = email_address
+    else
+      build_identity
+    end
   end
 
   # DEPRECATED
   #
-  # Please access identity.keys[:pgp] directly
+  # Please set the key on the identity directly
   def public_key=(value)
-    identity.keys[:pgp] = value
+    identity.set_key(:pgp, value)
   end
 
   # DEPRECATED
@@ -83,8 +88,7 @@ class User < CouchRest::Model::Base
   # this is the main identity. login@domain.tld
   # aliases and forwards are represented in other identities.
   def identity
-    @identity ||=
-      Identity.find_by_address_and_destination([email_address, email_address])
+    @identity ||= find_identity || build_identity
   end
 
   def create_identity(attribs = {}, &block)
@@ -96,8 +100,7 @@ class User < CouchRest::Model::Base
   def build_identity(attribs = {}, &block)
     attribs.reverse_merge! user_id: self.id,
       address: self.email_address,
-      destination: self.email_address,
-      keys: {}
+      destination: self.email_address
     Identity.new(attribs, &block)
   end
 
@@ -139,6 +142,10 @@ class User < CouchRest::Model::Base
 
   protected
 
+  def find_identity
+    Identity.find_by_address_and_destination([email_address, email_address])
+  end
+
   ##
   #  Validation Functions
   ##
diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index 4c94389..93b6507 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -79,8 +79,13 @@ class AccountFlowTest < RackTest
     test_public_key = 'asdlfkjslfdkjasd'
     original_login = @user.login
     new_login = 'zaph'
+    User.find_by_login(new_login).try(:destroy)
+    Identity.by_address.key(new_login + '@' + APP_CONFIG[:domain]).each do |identity|
+      identity.destroy
+    end
     put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => test_public_key, :login => new_login}, :format => :json
     @user.reload
+    assert last_response.successful?
     assert_equal test_public_key, @user.public_key
     assert_equal new_login, @user.login
     # eventually probably want to remove most of this into a non-integration functional test
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index d20ad93..40b1296 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -57,13 +57,13 @@ class IdentityTest < ActiveSupport::TestCase
 
   test "setting and getting pgp key" do
     id = @user.identity
-    id.keys[:pgp] = pgp_key_string
+    id.set_key(:pgp, pgp_key_string)
     assert_equal pgp_key_string, id.keys[:pgp]
   end
 
   test "querying pgp key via couch" do
     id = @user.identity
-    id.keys[:pgp] = pgp_key_string
+    id.set_key(:pgp, pgp_key_string)
     id.save
     view = Identity.pgp_key_by_email.key(id.address)
     assert_equal 1, view.rows.count