diff options
| -rw-r--r-- | users/app/controllers/v1/users_controller.rb | 13 | ||||
| -rw-r--r-- | users/app/models/account_settings.rb | 36 | ||||
| -rw-r--r-- | users/app/models/identity.rb | 27 | ||||
| -rw-r--r-- | users/app/models/signup_service.rb | 9 | ||||
| -rw-r--r-- | users/app/models/user.rb | 57 | ||||
| -rw-r--r-- | users/test/functional/v1/users_controller_test.rb | 8 | ||||
| -rw-r--r-- | users/test/integration/api/account_flow_test.rb | 13 | ||||
| -rw-r--r-- | users/test/unit/identity_test.rb | 20 | ||||
| -rw-r--r-- | users/test/unit/user_test.rb | 2 | 
9 files changed, 105 insertions, 80 deletions
| diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb index 467c9b4..f380c19 100644 --- a/users/app/controllers/v1/users_controller.rb +++ b/users/app/controllers/v1/users_controller.rb @@ -18,14 +18,23 @@ module V1      end      def create -      @user = User.create(params[:user]) +      @user = signup_service.register(params[:user])        respond_with @user # return ID instead?      end      def update -      @user.update_attributes params[:user] +      account_settings.update params[:user]        respond_with @user      end +    protected + +    def account_settings +      AccountSettings.new(@user) +    end + +    def signup_service +      SignupService.new +    end    end  end diff --git a/users/app/models/account_settings.rb b/users/app/models/account_settings.rb new file mode 100644 index 0000000..a73a95a --- /dev/null +++ b/users/app/models/account_settings.rb @@ -0,0 +1,36 @@ +class AccountSettings + +  def initialize(user) +    @user = user +  end + +  def update(attrs) +    if attrs[:password_verifier].present? +      update_login(attrs[:login]) +      @user.update_attributes attrs.slice(:password_verifier, :password_salt) +    end +    # TODO: move into identity controller +    update_pgp_key(attrs[:public_key]) if attrs.has_key? :public_key +    @user.save && save_identities +  end + +  protected + +  def update_login(login, verifier) +    return unless login.present? +    @old_identity = Identity.for(@user) +    @user.login = login +    @new_identity = Identity.for(@user) # based on the new login +    @old_identity.destination = @user.email_address # alias old -> new +  end + +  def update_pgp_key(key) +    @new_identity ||= Identity.for(@user) +    @new_identity.set_key(:pgp, key) +  end + +  def save_identities +    @new_identity.try(:save) && @old_identity.try(:save) +  end + +end diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb index 73531ec..355f67a 100644 --- a/users/app/models/identity.rb +++ b/users/app/models/identity.rb @@ -27,6 +27,33 @@ class Identity < CouchRest::Model::Base    end +  def self.for(user, attributes = {}) +    find_for(user, attributes) || build_for(user, attributes) +  end + +  def self.find_for(user, attributes = {}) +    attributes.reverse_merge! attributes_from_user(user) +    find_by_address_and_destination [attributes[:address], attributes[:destination]] +  end + +  def self.build_for(user, attributes = {}) +    attributes.reverse_merge! attributes_from_user(user) +    Identity.new(attributes) +  end + +  def self.create_for(user, attributes = {}) +    identity = build_for(user, attributes) +    identity.save +    identity +  end + +  def self.attributes_from_user(user) +    { user_id: user.id, +      address: user.email_address, +      destination: user.email_address +    } +  end +    def keys      read_attribute('keys') || HashWithIndifferentAccess.new    end diff --git a/users/app/models/signup_service.rb b/users/app/models/signup_service.rb new file mode 100644 index 0000000..f316ca9 --- /dev/null +++ b/users/app/models/signup_service.rb @@ -0,0 +1,9 @@ +class SignupService + +  def register(attrs) +    User.create(attrs).tap do |user| +      Identity.create_for user +    end +  end + +end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index c791069..f78f290 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -47,63 +47,10 @@ class User < CouchRest::Model::Base      view :by_created_at    end # end of design -  # We proxy access to the pgp_key. So we need to make sure -  # the updated identity actually gets saved. -  def save(*args) -    super -    identity.user_id ||= self.id -    identity.save if identity.changed? -  end - -  # So far this only works for creating a new user. -  # TODO: Create an alias for the old login when changing the login -  def login=(value) -    write_attribute 'login', value -    if @identity -      @identity.address = email_address -      @identity.destination = email_address -    else -      build_identity -    end -  end - -  # DEPRECATED -  # -  # Please set the key on the identity directly -  def public_key=(value) -    identity.set_key(:pgp, value) -  end - -  # DEPRECATED -  # -  # Please access identity.keys[:pgp] directly -  def public_key -    identity.keys[:pgp] -  end -    class << self      alias_method :find_by_param, :find    end -  # this is the main identity. login@domain.tld -  # aliases and forwards are represented in other identities. -  def identity -    @identity ||= find_identity || build_identity -  end - -  def create_identity(attribs = {}, &block) -    new_identity = build_identity(attribs, &block) -    new_identity.save -    new_identity -  end - -  def build_identity(attribs = {}, &block) -    attribs.reverse_merge! user_id: self.id, -      address: self.email_address, -      destination: self.email_address -    Identity.new(attribs, &block) -  end -    def to_param      self.id    end @@ -142,10 +89,6 @@ 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/functional/v1/users_controller_test.rb b/users/test/functional/v1/users_controller_test.rb index 0d44e50..a330bf3 100644 --- a/users/test/functional/v1/users_controller_test.rb +++ b/users/test/functional/v1/users_controller_test.rb @@ -5,7 +5,9 @@ class V1::UsersControllerTest < ActionController::TestCase    test "user can change settings" do      user = find_record :user      changed_attribs = record_attributes_for :user_with_settings -    user.expects(:update_attributes).with(changed_attribs) +    account_settings = stub +    account_settings.expects(:update).with(changed_attribs) +    AccountSettings.expects(:new).with(user).returns(account_settings)      login user      put :update, :user => changed_attribs, :id => user.id, :format => :json @@ -18,7 +20,9 @@ class V1::UsersControllerTest < ActionController::TestCase    test "admin can update user" do      user = find_record :user      changed_attribs = record_attributes_for :user_with_settings -    user.expects(:update_attributes).with(changed_attribs) +    account_settings = stub +    account_settings.expects(:update).with(changed_attribs) +    AccountSettings.expects(:new).with(user).returns(account_settings)      login :is_admin? => true      put :update, :user => changed_attribs, :id => user.id, :format => :json diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb index 93b6507..c09dcb6 100644 --- a/users/test/integration/api/account_flow_test.rb +++ b/users/test/integration/api/account_flow_test.rb @@ -84,20 +84,17 @@ class AccountFlowTest < RackTest        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 +    assert_equal test_public_key, Identity.for(@user).keys[:pgp] +    # does not change login if no password_verifier is present +    assert_equal original_login, @user.login      # eventually probably want to remove most of this into a non-integration functional test      # should not overwrite public key:      put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:blee => :blah}, :format => :json -    @user.reload -    assert_equal test_public_key, @user.public_key +    assert_equal test_public_key, Identity.for(@user).keys[:pgp]      # should overwrite public key:      put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => nil}, :format => :json -    # TODO: not sure why i need this, but when public key is removed, the DB is updated but @user.reload doesn't seem to actually reload. -    @user = User.find(@user.id) # @user.reload -    assert_nil @user.public_key +    assert_nil Identity.for(@user).keys[:pgp]    end  end diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb index 40b1296..bf24f02 100644 --- a/users/test/unit/identity_test.rb +++ b/users/test/unit/identity_test.rb @@ -11,28 +11,28 @@ class IdentityTest < ActiveSupport::TestCase    end    test "initial identity for a user" do -    id = @user.identity +    id = Identity.for(@user)      assert_equal @user.email_address, id.address      assert_equal @user.email_address, id.destination      assert_equal @user, id.user    end    test "add alias" do -    id = @user.build_identity address: alias_name +    id = Identity.for @user, address: alias_name      assert_equal LocalEmail.new(alias_name), id.address      assert_equal @user.email_address, id.destination      assert_equal @user, id.user    end    test "add forward" do -    id = @user.build_identity destination: forward_address +    id = Identity.for @user, destination: forward_address      assert_equal @user.email_address, id.address      assert_equal Email.new(forward_address), id.destination      assert_equal @user, id.user    end    test "forward alias" do -    id = @user.build_identity address: alias_name, destination: forward_address +    id = Identity.for @user, address: alias_name, destination: forward_address      assert_equal LocalEmail.new(alias_name), id.address      assert_equal Email.new(forward_address), id.destination      assert_equal @user, id.user @@ -40,29 +40,29 @@ class IdentityTest < ActiveSupport::TestCase    end    test "prevents duplicates" do -    id = @user.create_identity address: alias_name, destination: forward_address -    dup = @user.build_identity address: alias_name, destination: forward_address +    id = Identity.create_for @user, address: alias_name, destination: forward_address +    dup = Identity.build_for @user, address: alias_name, destination: forward_address      assert !dup.valid?      assert_equal ["This alias already exists"], dup.errors[:base]    end    test "validates availability" do      other_user = FactoryGirl.create(:user) -    id = @user.create_identity address: alias_name, destination: forward_address -    taken = other_user.build_identity address: alias_name +    id = Identity.create_for @user, address: alias_name, destination: forward_address +    taken = Identity.build_for other_user, address: alias_name      assert !taken.valid?      assert_equal ["This email has already been taken"], taken.errors[:base]      other_user.destroy    end    test "setting and getting pgp key" do -    id = @user.identity +    id = Identity.for(@user)      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 = Identity.for(@user)      id.set_key(:pgp, pgp_key_string)      id.save      view = Identity.pgp_key_by_email.key(id.address) diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb index f303c8d..477c727 100644 --- a/users/test/unit/user_test.rb +++ b/users/test/unit/user_test.rb @@ -58,7 +58,7 @@ class UserTest < ActiveSupport::TestCase    test "login needs to be unique amongst aliases" do      other_user = FactoryGirl.create :user -    other_user.create_identity address: @user.login +    Identity.create_for other_user, address: @user.login      assert !@user.valid?      other_user.destroy    end | 
