Merge pull request #391 from projecthydra/avoid_send

Use find_by instead of a dynamic finder

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

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('[email protected]')).to eq ['archivist']
     end
 
-    it "doesn't change its response when it's called repeatedly" do
-      u = User.new(:uid=>'[email protected]')
-      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: '[email protected]') }
+      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