Fix group-member-missing-check (#582)

* Fix group-member-missing-check * Fix type during test * Apply review feedback * Fix allow_all_case --------- Co-authored-by: Michael Franklin <[email protected]>
populationgenomics · Oct 23, 2023 · dd1ba37 · dd1ba37
1 parent 8dcd99e
commit dd1ba37
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 11 deletions.
diff --git a/db/python/tables/project.py b/db/python/tables/project.py
@@ -198,10 +198,11 @@ async def get_and_check_access_to_projects_for_ids(
             group_id_project_map = {p.write_group_id: p for p in projects}
 
         group_ids = set(gid for gid in group_id_project_map.keys() if gid)
-        # awkward double negative to find missing projects
-        missing_group_ids = await self.gtable.check_which_groups_member_is_missing(
+
+        present_group_ids = await self.gtable.check_which_groups_member_has(
             group_ids=group_ids, member=user
         )
+        missing_group_ids = group_ids - present_group_ids
 
         if missing_group_ids:
             # so we can directly return the project names they're missing
@@ -248,10 +249,10 @@ async def check_access_to_project_ids(
             group_id_project_map = {p.write_group_id: p for p in projects}
 
         group_ids = set(gid for gid in group_id_project_map.keys() if gid)
-        # awkward double negative to find missing projects
-        missing_group_ids = await self.gtable.check_which_groups_member_is_missing(
+        present_group_ids = await self.gtable.check_which_groups_member_has(
             group_ids=group_ids, member=user
         )
+        missing_group_ids = group_ids - present_group_ids
 
         if missing_group_ids:
             # so we can directly return the project names they're missing
@@ -543,22 +544,24 @@ async def check_if_member_in_group_name(self, group_name: str, member: str) -> b
 
         return bool(value)
 
-    async def check_which_groups_member_is_missing(
+    async def check_which_groups_member_has(
         self, group_ids: set[int], member: str
     ) -> set[int]:
-        """Check if a member is in a group"""
+        """
+        Check which groups a member has
+        """
         if self.allow_full_access:
-            return set()
+            return group_ids
 
         _query = """
-            SELECT g.group_id
+            SELECT gm.group_id as gid
             FROM group_member gm
-            WHERE gm.member = member AND gm.group_id IN :group_ids
+            WHERE gm.member = :member AND gm.group_id IN :group_ids
         """
-        membered_group_ids = await self.connection.fetch_all(
+        results = await self.connection.fetch_all(
             _query, {'group_ids': group_ids, 'member': member}
         )
-        return set(group_ids) - set(membered_group_ids)
+        return set(r['gid'] for r in results)
 
     async def create_group(self, name: str) -> int:
         """Create a new group"""

diff --git a/test/test_project_groups.py b/test/test_project_groups.py
@@ -93,6 +93,35 @@ async def test_group_set_members_succeeded(self):
             await self.pttable.gtable.check_if_member_in_group_name(g, 'user2')
         )
 
+    @run_as_sync
+    async def test_check_which_groups_member_is_missing(self):
+        """Test the check_which_groups_member_has function"""
+        await self._add_group_member_direct(GROUP_NAME_MEMBERS_ADMIN)
+
+        group = str(uuid.uuid4())
+        gid = await self.pttable.gtable.create_group(group)
+        present_gids = await self.pttable.gtable.check_which_groups_member_has(
+            {gid}, self.author
+        )
+        missing_gids = {gid} - present_gids
+        self.assertEqual(1, len(missing_gids))
+        self.assertEqual(gid, missing_gids.pop())
+
+    @run_as_sync
+    async def test_check_which_groups_member_is_missing_none(self):
+        """Test the check_which_groups_member_has function"""
+        await self._add_group_member_direct(GROUP_NAME_MEMBERS_ADMIN)
+
+        group = str(uuid.uuid4())
+        gid = await self.pttable.gtable.create_group(group)
+        await self.pttable.gtable.set_group_members(gid, [self.author], self.author)
+        present_gids = await self.pttable.gtable.check_which_groups_member_has(
+            {gid}, self.author
+        )
+        missing_gids = {gid} - present_gids
+
+        self.assertEqual(0, len(missing_gids))
+
     @run_as_sync
     async def test_project_creators_failed(self):
         """