Repository: fractal-analytics-platform/fractal-client (language: python)

Issue:
Fix help messages
https://github.com/fractal-analytics-platform/fractal-client/blob/main/fractal_client/parser.py#L970-L993
results into
![image](https://github.com/user-attachments/assets/a38662ee-4b22-4b43-b61f-fc8fdf7d545b)

Align with new viewer-path endpoint
```
fractal user whoami --viewer-paths

ERROR:root:Server returned 404
ERROR:root:Original request: GET http://localhost:8000/auth/current-user/viewer-paths/
ERROR:root:Original payload: 
ERROR:root:Server error message: {'detail': 'Not Found'}

ERROR:root:Terminating.
```

Ref:
* https://github.com/fractal-analytics-platform/fractal-server/pull/2096
Align with new viewer-path endpoint
```
fractal user whoami --viewer-paths

ERROR:root:Server returned 404
ERROR:root:Original request: GET http://localhost:8000/auth/current-user/viewer-paths/
ERROR:root:Original payload: 
ERROR:root:Server error message: {'detail': 'Not Found'}

ERROR:root:Terminating.
```

Ref:
* https://github.com/fractal-analytics-platform/fractal-server/pull/2096


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a2ad9959..605d8195 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 * Update task-collection commands, to align with [fractal-server 2.9.0](https://github.com/fractal-analytics-platform/fractal-server/blob/main/CHANGELOG.md#290) (\#738).
 * Remove (internal) obsolete `do_not_separate_logs` argument (\#738).
+* Add `group {add|remove}-user` commands, and deprecate `--new-user-ids` argument from `group update` (\#748).
+* Update `user whoami --viewer-paths` to call the new dedicated [server endpoint](https://github.com/fractal-analytics-platform/fractal-server/pull/2096) (\#748).
 * Testing:
     * Align with fractal-server 2.9.0 removal of `DB_ENGINE` variable (\#743).
 
diff --git a/fractal_client/cmd/__init__.py b/fractal_client/cmd/__init__.py
index e6330ee2..35d2be2a 100644
--- a/fractal_client/cmd/__init__.py
+++ b/fractal_client/cmd/__init__.py
@@ -8,9 +8,11 @@
 from ._dataset import get_dataset
 from ._dataset import patch_dataset
 from ._dataset import post_dataset
+from ._group import group_add_user
 from ._group import group_get
 from ._group import group_list
 from ._group import group_new
+from ._group import group_remove_user
 from ._group import group_update
 from ._job import get_job
 from ._job import get_job_list
@@ -385,9 +387,17 @@ def group(
         function_kwargs = get_kwargs(parameters, kwargs)
         iface = group_new(client, batch=batch, **function_kwargs)
     elif subcmd == "update":
-        parameters = ["group_id", "new_user_ids", "new_viewer_paths"]
+        parameters = ["group_id", "new_viewer_paths"]
         function_kwargs = get_kwargs(parameters, kwargs)
         iface = group_update(client, **function_kwargs)
+    elif subcmd == "add-user":
+        parameters = ["group_id", "user_id"]
+        function_kwargs = get_kwargs(parameters, kwargs)
+        iface = group_add_user(client, **function_kwargs)
+    elif subcmd == "remove-user":
+        parameters = ["group_id", "user_id"]
+        function_kwargs = get_kwargs(parameters, kwargs)
+        iface = group_remove_user(client, **function_kwargs)
     else:
         raise NoCommandError(f"Command 'group {subcmd}' not found")
 
diff --git a/fractal_client/cmd/_group.py b/fractal_client/cmd/_group.py
index b44e4986..a05dfe53 100644
--- a/fractal_client/cmd/_group.py
+++ b/fractal_client/cmd/_group.py
@@ -50,19 +50,41 @@ def group_update(
     client: AuthClient,
     *,
     group_id: int,
-    new_user_ids: list[int] | None = None,
-    new_viewer_paths: list[str] | None = None,
+    new_viewer_paths: list[str],
 ):
 
-    request_body = dict()
-    if new_viewer_paths is not None:
-        request_body["viewer_paths"] = new_viewer_paths
-    if new_user_ids is not None:
-        request_body["new_user_ids"] = new_user_ids
-
     res = client.patch(
         f"{settings.FRACTAL_SERVER}/auth/group/{group_id}/",
-        json=request_body,
+        json=dict(viewer_paths=new_viewer_paths),
+    )
+    data = check_response(res, expected_status_code=200)
+    return Interface(retcode=0, data=data)
+
+
+def group_add_user(
+    client: AuthClient,
+    *,
+    group_id: int,
+    user_id: int,
+):
+
+    res = client.post(
+        f"{settings.FRACTAL_SERVER}/auth/group/{group_id}/add-user/{user_id}/"
+    )
+    data = check_response(res, expected_status_code=200)
+    return Interface(retcode=0, data=data)
+
+
+def group_remove_user(
+    client: AuthClient,
+    *,
+    group_id: int,
+    user_id: int,
+):
+
+    res = client.post(
+        f"{settings.FRACTAL_SERVER}/auth/group/{group_id}/remove-user/"
+        f"{user_id}/"
     )
     data = check_response(res, expected_status_code=200)
     return Interface(retcode=0, data=data)
diff --git a/fractal_client/cmd/_user.py b/fractal_client/cmd/_user.py
index 064a125e..2a6f57bc 100644
--- a/fractal_client/cmd/_user.py
+++ b/fractal_client/cmd/_user.py
@@ -220,7 +220,8 @@ def user_whoami(
 
     if viewer_paths:
         res = client.get(
-            f"{settings.FRACTAL_SERVER}/auth/current-user/viewer-paths/"
+            f"{settings.FRACTAL_SERVER}/auth/current-user/"
+            "allowed-viewer-paths/"
         )
         returned_viewer_paths = check_response(res, expected_status_code=200)
         return Interface(
diff --git a/fractal_client/parser.py b/fractal_client/parser.py
index 6d366616..55501fe3 100644
--- a/fractal_client/parser.py
+++ b/fractal_client/parser.py
@@ -967,27 +967,39 @@
 group_update_parser.add_argument(
     "group_id", help="ID of the group to update.", type=int
 )
-group_update_parser.add_argument(
-    "--new-user-ids",
-    help=(
-        "New list of users in the group "
-        "(e.g. `--new-user-ids 4 8 15 16 23 42`); "
-        "note that this replaces the existing one.",
-    ),
-    required=False,
-    type=int,
-    nargs="+",
-    default=None,
-)
 group_update_parser.add_argument(
     "--new-viewer-paths",
     help=(
         "New list of group `viewer_paths` (e.g "
-        "`--new-viewer-paths /something /else`);"
-        "note that this replaces the existing one.",
+        "`--new-viewer-paths /something /else`); "
+        "note that this replaces the existing one."
     ),
-    required=False,
+    required=True,
     type=str,
     nargs="+",
     default=None,
 )
+
+# group add-user
+group_add_user_parser = group_subparsers.add_parser(
+    "add-user", description="Add a single user to group.", allow_abbrev=False
+)
+group_add_user_parser.add_argument(
+    "group_id", help="ID of the group to which to add the user.", type=int
+)
+group_add_user_parser.add_argument(
+    "user_id", help="ID of the user to add.", type=int
+)
+
+# group remove-user
+group_remove_user_parser = group_subparsers.add_parser(
+    "remove-user",
+    description="Remove a single user from group.",
+    allow_abbrev=False,
+)
+group_remove_user_parser.add_argument(
+    "group_id", help="ID of the group to which to remove the user.", type=int
+)
+group_remove_user_parser.add_argument(
+    "user_id", help="ID of the user to remove.", type=int
+)
diff --git a/poetry.lock b/poetry.lock
index 85c88991..7520f898 100644
--- a/poetry.lock
+++ b/poetry.lock
@@ -1,4 +1,4 @@
-# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand.
+# This file is automatically @generated by Poetry 1.8.4 and should not be changed by hand.
 
 [[package]]
 name = "alembic"
@@ -696,7 +696,7 @@ typing = ["typing-extensions (>=4.12.2)"]
 
 [[package]]
 name = "fractal-server"
-version = "2.9.0a4"
+version = "2.9.0a10"
 description = "Server component of the Fractal analytics platform"
 optional = false
 python-versions = "^3.10"
@@ -726,7 +726,7 @@ uvicorn-worker = "^0.2.0"
 type = "git"
 url = "https://github.com/fractal-analytics-platform/fractal-server.git"
 reference = "main"
-resolved_reference = "87c2868d73e458273beca7895d51763366ed5847"
+resolved_reference = "8bb57912219c0b87997249d730c7f0413dcf8478"
 
 [[package]]
 name = "ghp-import"


Test patch:
diff --git a/tests/fixtures_testserver.py b/tests/fixtures_testserver.py
index b37fcdfd..0b80db4b 100644
--- a/tests/fixtures_testserver.py
+++ b/tests/fixtures_testserver.py
@@ -63,6 +63,7 @@ def testserver(tester, tmpdir_factory, request):
             "FRACTAL_RUNNER_WORKING_BASE_DIR="
             f"{FRACTAL_RUNNER_WORKING_BASE_DIR}\n"
             "FRACTAL_LOGGING_LEVEL=0\n"
+            "FRACTAL_VIEWER_AUTHORIZATION_SCHEME=viewer-paths\n"
         )
     _run_command(
         f"dropdb --username=postgres --host localhost --if-exists {DB_NAME}"
diff --git a/tests/test_group.py b/tests/test_group.py
index d5068455..4ba36d7a 100644
--- a/tests/test_group.py
+++ b/tests/test_group.py
@@ -16,7 +16,7 @@ def _assert_403(cmd):
     _assert_403(cmd="group list")
     _assert_403(cmd="group get 1")
     _assert_403(cmd="group new foo")
-    _assert_403(cmd="group update 1 --new-user-ids 1")
+    _assert_403(cmd="group update 1 --new-viewer-paths foo")
 
 
 def test_group_commands(
@@ -72,50 +72,40 @@ def test_group_commands(
     group2_viewer_paths = res.data["viewer_paths"]
     assert group2_viewer_paths == []
 
-    # Add users to groups (`group update`)
-
-    # empty update
-    res = invoke_as_superuser(f"group update {default_group_id}")
-    assert res.retcode == 0
-    assert res.data["id"] == default_group_id
+    # Add users to groups (`group add-user/remove-user`)
 
     with pytest.raises(SystemExit):
-        # missing 'group_id' and 'new_user_ids'
-        invoke_as_superuser("group update")
+        # missing both arguments
+        invoke_as_superuser("group add-user")
     with pytest.raises(SystemExit):
-        # missing 'group_id'
-        invoke_as_superuser(f"group update --new-user-ids {superuser_id}")
+        # missing one argument
+        invoke_as_superuser("group add-user 1")
+
     with pytest.raises(SystemExit):
         # user already in group
         invoke_as_superuser(
-            f"group update {default_group_id} --new-user-ids {superuser_id}"
+            f"group add-user {default_group_id} {superuser_id}"
         )
     with pytest.raises(SystemExit):
         # non existing user
-        invoke_as_superuser(
-            f"group update {default_group_id} --new-user-ids 9999"
-        )
+        invoke_as_superuser(f"group add-user {default_group_id} 9999")
 
     # add `user1` and `user2` to `group1`
-    res = invoke_as_superuser(
-        f"group update {group1_id} --new-user-ids {user1_id} {user2_id}"
-    )
+    invoke_as_superuser(f"group add-user {group1_id} {user1_id}")
+    res = invoke_as_superuser(f"group add-user {group1_id} {user2_id}")
     assert res.retcode == 0
     assert res.data["id"] == group1_id
     assert res.data["user_ids"] == [user1_id, user2_id]
     assert res.data["viewer_paths"] == group1_viewer_paths
 
     # add `user3` and `user2` to `group2`
-    res = invoke_as_superuser(
-        f"group update {group2_id} --new-user-ids {user3_id} {user2_id}"
-    )
+    invoke_as_superuser(f"group add-user {group2_id} {user3_id}")
+    res = invoke_as_superuser(f"group add-user {group2_id} {user2_id}")
     assert res.retcode == 0
     assert res.data["id"] == group2_id
     assert set(res.data["user_ids"]) == set([user3_id, user2_id])
     # add also `superuser` to `group2`
-    res = invoke_as_superuser(
-        f"group update {group2_id} --new-user-ids {superuser_id}"
-    )
+    res = invoke_as_superuser(f"group add-user {group2_id} {superuser_id}")
     assert set(res.data["user_ids"]) == set([user3_id, user2_id, superuser_id])
     assert res.data["viewer_paths"] == group2_viewer_paths
 
@@ -132,6 +122,14 @@ def test_group_commands(
         [user3_id, user2_id, superuser_id]
     )
 
+    # Remove users from group
+    res = invoke_as_superuser(f"group remove-user {group2_id} {user3_id}")
+    assert set(res.data["user_ids"]) == set([user2_id, superuser_id])
+    res = invoke_as_superuser(f"group remove-user {group2_id} {user2_id}")
+    assert set(res.data["user_ids"]) == set([superuser_id])
+    res = invoke_as_superuser(f"group remove-user {group2_id} {superuser_id}")
+    assert set(res.data["user_ids"]) == set()
+
     # Test `group get` command
 
     with pytest.raises(SystemExit):
@@ -171,10 +169,7 @@ def test_group_commands(
     assert res_post_patch.data == res_pre_patch.data
 
     # Test `whoami --viewer-paths`
-
-    invoke_as_superuser(
-        f"group update {group1_id} --new-user-ids {superuser_id}"
-    )
+    invoke_as_superuser(f"group add-user {group1_id} {superuser_id}")
     assert "viewer_paths" not in superuser
     res = invoke_as_superuser("user whoami --viewer-paths")
     assert set(res.data.get("viewer_paths")) == {"/a/b", "/c/d"}