diff --git a/hydra-access-controls/lib/hydra/user.rb b/hydra-access-controls/lib/hydra/user.rb index 60f34cbb9..6a9e8ac3e 100644 --- a/hydra-access-controls/lib/hydra/user.rb +++ b/hydra-access-controls/lib/hydra/user.rb @@ -15,11 +15,11 @@ def groups end module ClassMethods - # This method should find User objects using the user_key you've chosen. - # By default, uses the unique identifier specified in by devise authentication_keys (ie. find_by_id, or find_by_email). - # You must have that find method implemented on your user class, or must override find_by_user_key + # This method finds User objects using the user_key as specified by the + # Devise authentication_keys configuration variable. This method encapsulates + # whether we use email or username (or something else) as the identifing user attribute. def find_by_user_key(key) - self.send("find_by_#{Devise.authentication_keys.first}".to_sym, key) + find_by(Devise.authentication_keys.first.to_sym => key) end end end diff --git a/hydra-access-controls/spec/unit/role_mapper_spec.rb b/hydra-access-controls/spec/unit/role_mapper_spec.rb index 9e88473ae..b10ae929b 100644 --- a/hydra-access-controls/spec/unit/role_mapper_spec.rb +++ b/hydra-access-controls/spec/unit/role_mapper_spec.rb @@ -1,10 +1,6 @@ require 'spec_helper' describe RoleMapper do - before do - allow(Devise).to receive(:authentication_keys).and_return(['uid']) - end - it "defines the 4 roles" do expect(RoleMapper.role_names.sort).to eq %w(admin_policy_object_editor archivist donor patron researcher) end @@ -51,11 +47,16 @@ expect(RoleMapper.roles('archivist2@example.com')).to eq ['archivist'] end - it "doesn't change its response when it's called repeatedly" do - u = User.new(:uid=>'leland_himself@example.com') - allow(u).to receive(:new_record?).and_return(false) - expect(RoleMapper.roles(u).sort).to eq ['archivist', 'donor', 'patron'] - expect(RoleMapper.roles(u).sort).to eq ['archivist', 'donor', 'patron'] + context "when called with a user instance" do + let(:user) { User.new(email: 'leland_himself@example.com') } + before do + allow(user).to receive(:new_record?).and_return(false) + end + + it "doesn't change its response when it's called repeatedly" do + expect(RoleMapper.roles(user).sort).to eq ['archivist', 'donor', 'patron'] + expect(RoleMapper.roles(user).sort).to eq ['archivist', 'donor', 'patron'] + end end it "returns an empty array if there are no roles" do