From 104fe1e80d6edf7bec9dd6b283f33ee47dadef82 Mon Sep 17 00:00:00 2001
From: Keith James <kdj@sanger.ac.uk>
Date: Mon, 18 Sep 2023 13:48:54 +0100
Subject: [PATCH] Fix inverted login in ACL user_type filter

The logic for filtering ACLs by user_type was inverted and the use of
thew new user_type keyword option was not covered by tests. This
change adds tests (which failed, initially) and also fixes the bug.
---
 src/partisan/irods.py |  2 +-
 tests/test_irods.py   | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/partisan/irods.py b/src/partisan/irods.py
index 68d4e43..cdbf920 100644
--- a/src/partisan/irods.py
+++ b/src/partisan/irods.py
@@ -1772,7 +1772,7 @@ def acl(self, user_type: str = None, timeout=None, tries=1) -> List[AC]:
 
         if user_type is not None:
             by_name = {user.name: user for user in rods_users(user_type=user_type)}
-            acl = [ac for ac in acl if ac.user not in by_name]
+            acl = [ac for ac in acl if ac.user in by_name]
 
         return sorted(acl)
 
diff --git a/tests/test_irods.py b/tests/test_irods.py
index eeb3ea1..b23237b 100644
--- a/tests/test_irods.py
+++ b/tests/test_irods.py
@@ -568,6 +568,18 @@ def test_add_ac_collection(self, full_collection):
         for item in coll.contents(recurse=True):
             assert item.acl() == [irods_own], "Collection content ACL unchanged"
 
+    @m.it("Can have its permissions listed")
+    def test_list_ac_collection(self, full_collection):
+        zone = "testZone"
+        coll = Collection(full_collection)
+        irods_own = AC("irods", Permission.OWN, zone=zone)
+        public_read = AC("public", Permission.READ, zone=zone)
+        assert coll.add_permissions(public_read) == 1
+
+        assert coll.acl() == [irods_own, public_read]
+        assert coll.acl(user_type="rodsadmin") == [irods_own]
+        assert coll.acl(user_type="rodsgroup") == [public_read]
+
     @m.it("Can have access controls added, recursively")
     def test_add_ac_collection_recurse(self, full_collection):
         zone = "testZone"
@@ -995,6 +1007,18 @@ def test_add_ac_data_object(self, simple_data_object):
         assert obj.add_permissions(public_read) == 1
         assert obj.acl() == [irods_own, public_read]
 
+    @m.it("Can have its permissions listed")
+    def test_list_ac_data_object(self, simple_data_object):
+        zone = "testZone"
+        obj = DataObject(simple_data_object)
+        irods_own = AC("irods", Permission.OWN, zone=zone)
+        public_read = AC("public", Permission.READ, zone=zone)
+        assert obj.add_permissions(public_read) == 1
+
+        assert obj.acl() == [irods_own, public_read]
+        assert obj.acl(user_type="rodsadmin") == [irods_own]
+        assert obj.acl(user_type="rodsgroup") == [public_read]
+
     @m.it("Can have access controls removed")
     def test_rem_ac_data_object(self, simple_data_object):
         zone = "testZone"