From fbf903d1e710261a72ea16fb922ee0e44ce9f200 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Thu, 13 Jun 2024 18:24:18 +0200
Subject: [PATCH] Refactor API test for updating a group

---
 lib/galaxy_test/api/test_groups.py | 72 +++++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 16 deletions(-)

diff --git a/lib/galaxy_test/api/test_groups.py b/lib/galaxy_test/api/test_groups.py
index 7441c3740663..7e0d40943868 100644
--- a/lib/galaxy_test/api/test_groups.py
+++ b/lib/galaxy_test/api/test_groups.py
@@ -1,4 +1,7 @@
-from typing import Optional
+from typing import (
+    List,
+    Optional,
+)
 
 from galaxy_test.base.populators import DatasetPopulator
 from ._framework import ApiTestCase
@@ -72,37 +75,62 @@ def test_show_unknown_raises_400(self):
         self._assert_status_code_is(response, 400)
 
     def test_update(self):
-        group = self.test_create_valid(group_name=f"group-test-{self.dataset_populator.get_random_name()}")
+        user_id = self.dataset_populator.user_id()
+        user_private_role_id = self.dataset_populator.user_private_role_id()
+        original_name = f"group-test-{self.dataset_populator.get_random_name()}"
+        group = self.test_create_valid(group_name=original_name)
+
+        self._assert_group_has_expected_values(
+            group["id"],
+            name=original_name,
+            user_ids=[user_id],
+            role_ids=[user_private_role_id],
+        )
 
         group_id = group["id"]
         updated_name = f"group-test-updated-{self.dataset_populator.get_random_name()}"
         update_response = self._put(f"groups/{group_id}", data={"name": updated_name}, admin=True, json=True)
         self._assert_status_code_is_ok(update_response)
 
-        # Update replace with another user
+        # Only the name should be updated
+        self._assert_group_has_expected_values(
+            group_id,
+            name=updated_name,
+            user_ids=[user_id],
+            role_ids=[user_private_role_id],
+        )
+
+        # Add another user to the group
         another_user_id = None
         with self._different_user():
             another_user_id = self.dataset_populator.user_id()
+            another_role_id = self.dataset_populator.user_private_role_id()
         assert another_user_id is not None
         update_response = self._put(f"groups/{group_id}", data={"user_ids": [another_user_id]}, admin=True, json=True)
         self._assert_status_code_is_ok(update_response)
 
-        # get group and check if it was updated
-        response = self._get(f"groups/{group_id}", admin=True)
-        self._assert_status_code_is_ok(response)
-        users = self._get(f"groups/{group_id}/users", admin=True).json()
-        assert len(users) == 1
-        assert users[0]["id"] == another_user_id
+        # Check if the user was added
+        self._assert_group_has_expected_values(
+            group_id,
+            name=updated_name,
+            user_ids=[user_id, another_user_id],
+            role_ids=[user_private_role_id],
+        )
 
-        # Remove private role from group
-        update_response = self._put(f"groups/{group_id}", data={"role_ids": []}, admin=True, json=True)
+        # Add another role to the group
+        update_response = self._put(f"groups/{group_id}", data={"role_ids": [another_role_id]}, admin=True, json=True)
         self._assert_status_code_is_ok(update_response)
 
-        # get group and check if it was updated
-        response = self._get(f"groups/{group_id}", admin=True)
-        self._assert_status_code_is_ok(response)
-        roles = self._get(f"groups/{group_id}/roles", admin=True).json()
-        assert len(roles) == 0
+        # Check if the role was added
+        self._assert_group_has_expected_values(
+            group_id,
+            name=updated_name,
+            user_ids=[user_id, another_user_id],
+            role_ids=[user_private_role_id, another_role_id],
+        )
+
+        # TODO: Test removing users and roles
+        # Currently not possible because the API can only add users and roles
 
     def test_update_only_admin(self):
         group = self.test_create_valid()
@@ -177,6 +205,18 @@ def _assert_valid_group(self, group, assert_id=None):
         if assert_id is not None:
             assert group["id"] == assert_id
 
+    def _assert_group_has_expected_values(self, group_id: str, name: str, user_ids: List[str], role_ids: List[str]):
+        group = self._get(f"groups/{group_id}", admin=True).json()
+        assert group["name"] == name
+        users = self._get(f"groups/{group_id}/users", admin=True).json()
+        assert len(users) == len(user_ids)
+        for user in users:
+            assert user["id"] in user_ids
+        roles = self._get(f"groups/{group_id}/roles", admin=True).json()
+        assert len(roles) == len(role_ids)
+        for role in roles:
+            assert role["id"] in role_ids
+
     def _build_valid_group_payload(self, name: Optional[str] = None):
         name = name or self.dataset_populator.get_random_name()
         user_id = self.dataset_populator.user_id()