Add #agent_key & .from_agent_key to Group & User

These methods help reduce the amount of knowledge that ACLs require
regarding agents.

For Users, the default implementation is just to use `#user_key` and
`User.find_by_user_key`.

(An alternate approach would be just to implement `#user_key` and
`.find_by_user_key` directly on `Hyrax::Group`, but I’m not sure we
want Groups to be treatable as Users in all contexts where `#user_key`
might be used.)

app/models/concerns/hyrax/user.rb

@@ -53,6 +53,13 @@ def user_key
     public_send(self.class.user_key_field)
   end
 
+  ##
+  # @return [String] a local identifier for this user; for use (e.g.) in ACL
+  #   data
+  def agent_key
+    user_key
+  end
+
   # Look for, in order:
   #   A cached version of the agent
   #   A non-cached version (direct read of the database)
@@ -177,6 +184,10 @@ def find_or_create_system_user(user_key)
       User.find_by_user_key(user_key) || User.create!(user_key_field => user_key, password: Devise.friendly_token[0, 20])
     end
 
+    def from_agent_key(key)
+      User.find_by_user_key(key)
+    end
+
     def from_url_component(component)
       User.find_by_user_key(component.gsub(/-dot-/, '.'))
     end

app/models/hyrax/group.rb

@@ -9,7 +9,7 @@ def self.name_prefix
 
     ##
     # @return [Hyrax::Group]
-    def self.from_key(key)
+    def self.from_agent_key(key)
       new(key.delete_prefix(name_prefix))
     end
 
@@ -22,17 +22,15 @@ def initialize(name)
     ##
     # @return [Boolean]
     def ==(other)
-      other.class == self.class &&
-        other.name == self.name
+      other.class == self.class && other.name == name
     end
 
     ##
     # @return [String] a local identifier for this group; for use (e.g.) in ACL
     #   data
-    def group_key
+    def agent_key
       self.class.name_prefix + name
     end
-    alias user_key group_key
 
     def to_sipity_agent
       sipity_agent || create_sipity_agent!

app/services/hyrax/access_control_list.rb

@@ -200,8 +200,14 @@ def initialize(acl, mode)
 
       private
 
+      ##
+      # Returns the identifier used by ACLs to identify agents.
+      #
+      # This defaults to the `:agent_key`, but if that method doesn’t exist,
+      # `:user_key` will be used as a fallback.
       def id_for(agent:)
-        agent.user_key.to_s
+        key = agent.try(:agent_key) || agent.user_key
+        key.to_s
       end
     end
 

spec/models/hyrax/group_spec.rb

@@ -4,14 +4,14 @@
   let(:name) { 'etaoin' }
   let(:group) { described_class.new(name) }
 
-  describe '.from_key' do
+  describe '.from_agent_key' do
     it 'returns an equivalent group' do
-      expect(Hyrax::Group.from_key(Hyrax::Group.name_prefix + group.name)).to eq group
+      expect(described_class.from_agent_key(group.agent_key)).to eq group
     end
   end
 
   describe '#==' do
-    let (:other_group) { described_class.new(group.name) }
+    let(:other_group) { described_class.new(group.name) }
 
     it 'correctly determines equality for equivalent groups' do
       expect(other_group).to eq group
@@ -24,6 +24,12 @@
     end
   end
 
+  describe '#agent_key' do
+    it 'returns the name prefixed with the name prefix' do
+      expect(group.agent_key).to eq described_class.name_prefix + group.name
+    end
+  end
+
   describe '#to_sipity_agent' do
     subject { group.to_sipity_agent }
 

spec/models/user_spec.rb

@@ -61,6 +61,20 @@
     end
   end
 
+  describe '#agent_key' do
+    let(:key) { user.agent_key }
+
+    it 'is the same as the user key' do
+      expect(key).to eq user.user_key
+    end
+
+    it 'is findable by agent_key' do
+      user.save!
+
+      expect(described_class.from_agent_key(key)).to eq user
+    end
+  end
+
   it "has an email" do
     expect(user.email).to be_kind_of String
   end