From 6a6f141f967463da683c18e6a89125a26197eee6 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Tue, 5 Nov 2024 11:03:48 +0100
Subject: [PATCH] Move to optional `--zarr-dir` argument for `project
 add-dataset`

---
 CHANGELOG.md                   |  5 ++++-
 fractal_client/cmd/_dataset.py | 13 ++++++-------
 fractal_client/parser.py       |  3 ++-
 tests/test_dataset.py          | 20 +++++++++++++++++---
 4 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7670724e..d114096a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,7 +1,10 @@
 **Note**: Numbers like (\#123) point to closed Pull Requests on the fractal repository.
 
-# 2.3.1 (unreleased)
+# 2.3.1
 
+> WARNING: This release has a breaking change in the `project add-dataset` command.
+
+* Move to from positional `zarr_dir` argument to optional `--zarr-dir` argument, for `project add-dataset` (\#736).
 * Add support for user-settings `project_dir`, introduced in fractal-server 2.8.0 (\#736).
 * Internal:
     * Update effect of `include_logs` for task-collection check command (\#730).
diff --git a/fractal_client/cmd/_dataset.py b/fractal_client/cmd/_dataset.py
index 0a564168..fd7d9bb9 100644
--- a/fractal_client/cmd/_dataset.py
+++ b/fractal_client/cmd/_dataset.py
@@ -11,7 +11,7 @@ def post_dataset(
     *,
     project_id: int,
     dataset_name: str,
-    zarr_dir: str,
+    zarr_dir: str | None = None,
     filters: str | None = None,
     batch: bool = False,
 ) -> Interface:
@@ -22,14 +22,13 @@ def post_dataset(
         filters: Path to file containing dataset filters in JSON format.
         batch: Dataset filters.
     """
-    if filters is None:
-        dataset = dict(name=dataset_name, zarr_dir=zarr_dir)
-    else:
+    dataset = dict(name=dataset_name)
+    if zarr_dir is not None:
+        dataset["zarr_dir"] = zarr_dir
+    if filters is not None:
         with open(filters, "r") as f:
             filters_dict = json.load(f)
-        dataset = dict(
-            name=dataset_name, filters=filters_dict, zarr_dir=zarr_dir
-        )
+        dataset["filters"] = filters_dict
 
     res = client.post(
         f"{settings.BASE_URL}/project/{project_id}/dataset/",
diff --git a/fractal_client/parser.py b/fractal_client/parser.py
index 034e0fc0..736f2a75 100644
--- a/fractal_client/parser.py
+++ b/fractal_client/parser.py
@@ -109,8 +109,9 @@
     help="Name of new dataset.",
 )
 project_add_dataset_parser.add_argument(
-    "zarr_dir",
+    "--zarr-dir",
     help="Path to zarr dir.",
+    required=False,
 )
 project_add_dataset_parser.add_argument(
     "--filters",
diff --git a/tests/test_dataset.py b/tests/test_dataset.py
index 1943a4e6..5487c2d8 100644
--- a/tests/test_dataset.py
+++ b/tests/test_dataset.py
@@ -3,7 +3,12 @@
 from devtools import debug
 
 
-def test_create_dataset(invoke, tmp_path, new_name):
+def test_create_dataset(
+    invoke,
+    tmp_path,
+    new_name,
+    invoke_as_superuser,
+):
     """
     Test some specific branches of the post_dataset function and parser.
     """
@@ -19,7 +24,8 @@ def test_create_dataset(invoke, tmp_path, new_name):
 
     res = invoke(
         (
-            f"project add-dataset {project_id} {new_name()} /tmp "
+            f"project add-dataset {project_id} {new_name()} "
+            "--zarr-dir /tmp "
             f"--filters {file_filters}"
         )
     )
@@ -28,7 +34,15 @@ def test_create_dataset(invoke, tmp_path, new_name):
     assert res.retcode == 0
     assert res.data["filters"] == FILTERS
 
-    res = invoke(f"--batch project add-dataset {project_id} {new_name()} /tmp")
+    # Add a project_dir to user-settings
+    res = invoke("--batch user whoami")
+    assert res.retcode == 0
+    user_id = res.data
+    res = invoke_as_superuser(
+        f"user edit {user_id} --new-project-dir /something"
+    )
+    assert res.retcode == 0
+    res = invoke(f"--batch project add-dataset {project_id} {new_name()}")
     debug(res.data)
     assert res.retcode == 0