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

Issue:
Review `workflow edit` command



Discussion / hints:

Fix patch:
diff --git a/docs/source/cli.rst b/docs/source/cli.rst
index feb649722..6503621e0 100644
--- a/docs/source/cli.rst
+++ b/docs/source/cli.rst
@@ -1,5 +1,5 @@
-Fractal CLI
-===================
+Command-line interface for Fractal Client
+=========================================
 
 .. argparse::
    :filename: fractal/parser.py
diff --git a/fractal/cmd/_dataset.py b/fractal/cmd/_dataset.py
index 7e6bd4934..cf3b30201 100644
--- a/fractal/cmd/_dataset.py
+++ b/fractal/cmd/_dataset.py
@@ -110,7 +110,7 @@ async def dataset_show(
         return RichJsonInterface(retcode=0, data=dataset.dict())
     else:
         table = Table(title="Dataset")
-        table.add_column("Id", style="cyan", no_wrap=True)
+        table.add_column("ID", style="cyan", no_wrap=True)
         table.add_column("Name", justify="right", style="green")
         table.add_column("Type", style="white")
         table.add_column("Meta", justify="center")
diff --git a/fractal/cmd/_project.py b/fractal/cmd/_project.py
index 6ab242c3d..435a3e1ee 100644
--- a/fractal/cmd/_project.py
+++ b/fractal/cmd/_project.py
@@ -55,7 +55,7 @@ async def project_list(client: AuthClient, **kwargs) -> RichConsoleInterface:
     projects = [ProjectRead(**item) for item in res]
 
     table = Table(title="Project List")
-    table.add_column("Id", style="cyan", no_wrap=True)
+    table.add_column("ID", style="cyan", no_wrap=True)
     table.add_column("Name", style="magenta")
     table.add_column("Proj. Dir.", justify="right", style="green")
     table.add_column("Dataset list", style="white")
diff --git a/fractal/common b/fractal/common
index 52e5a22d6..129c170cf 160000
--- a/fractal/common
+++ b/fractal/common
@@ -1,1 +1,1 @@
-Subproject commit 52e5a22d61cd6cfff538277d90603d4f55c799f9
+Subproject commit 129c170cf2e1715964706ceb035f79869208ceb1
diff --git a/fractal/parser.py b/fractal/parser.py
index 566b6e76b..292d35df9 100644
--- a/fractal/parser.py
+++ b/fractal/parser.py
@@ -15,7 +15,9 @@
 import argparse as ap
 
 
-parser_main = ap.ArgumentParser(description="Client component of Fractal")
+parser_main = ap.ArgumentParser(
+    description="Command-line interface for Fractal Client"
+)
 
 parser_main.add_argument(
     "-u",
@@ -39,7 +41,7 @@
     action="store_true",
     help=(
         "Return output suitable for scripting, e.g., "
-        "only the id of items created instead of the full object."
+        "only the ID of items created instead of the full object."
     ),
 )
 parser_main.add_argument(
@@ -55,7 +57,7 @@
 
 # REGISTER GROUP
 register_parser = subparsers_main.add_parser(
-    "register", help="Register with the Fractal server"
+    "register", description="Register with the Fractal server"
 )
 register_parser.add_argument("email", help="Email to be used as username")
 register_parser.add_argument(
@@ -69,14 +71,16 @@
 
 
 # PROJECT GROUP
-project_parser = subparsers_main.add_parser("project", help="project commands")
+project_parser = subparsers_main.add_parser(
+    "project", description="project commands"
+)
 project_subparsers = project_parser.add_subparsers(
     title="Valid subcommands:", dest="subcmd", required=True
 )
 
 # project new
 project_new_parser = project_subparsers.add_parser(
-    "new", help="Create new project"
+    "new", description="Create new project"
 )
 project_new_parser.add_argument("name", help="Name of new project", type=str)
 project_new_parser.add_argument(
@@ -97,31 +101,31 @@
 
 # project list
 project_list_parser = project_subparsers.add_parser(
-    "list", help="List projects"
+    "list", description="List projects"
 )
 
 # project show
 project_show_parser = project_subparsers.add_parser(
-    "show", help="Show details of a single project"
+    "show", description="Show details of single project"
 )
 project_show_parser.add_argument(
-    "project_id", type=int, help="Id of project to show"
+    "project_id", type=int, help="ID of project to show"
 )
 
 # project delete
 project_delete_parser = project_subparsers.add_parser(
-    "delete", help="Delete project"
+    "delete", description="Delete project"
 )
 project_delete_parser.add_argument(
-    "project_id", type=int, help="Id of project to delete"
+    "project_id", type=int, help="ID of project to delete"
 )
 
 # project add-dataset
 project_add_dataset_parser = project_subparsers.add_parser(
-    "add-dataset", help="Add dataset to project"
+    "add-dataset", description="Add dataset to project"
 )
 project_add_dataset_parser.add_argument(
-    "project_id", type=int, help="Id of project to add the new dataset to"
+    "project_id", type=int, help="ID of project to add the new dataset to"
 )
 project_add_dataset_parser.add_argument(
     "dataset_name", help="Name of new dataset"
@@ -133,20 +137,22 @@
 
 
 # DATASET GROUP
-dataset_parser = subparsers_main.add_parser("dataset", help="dataset commands")
+dataset_parser = subparsers_main.add_parser(
+    "dataset", description="dataset commands"
+)
 dataset_subparsers = dataset_parser.add_subparsers(
     title="Valid subcommands:", dest="subcmd", required=True
 )
 
 # dataset add-resource
 dataset_add_resource_parser = dataset_subparsers.add_parser(
-    "add-resource", help="Add resource to existing dataset"
+    "add-resource", description="Add resource to existing dataset"
 )
 dataset_add_resource_parser.add_argument(
-    "project_id", type=int, help="Project id"
+    "project_id", type=int, help="Project ID"
 )
 dataset_add_resource_parser.add_argument(
-    "dataset_id", type=int, help="Dataset id"
+    "dataset_id", type=int, help="Dataset ID"
 )
 dataset_add_resource_parser.add_argument("path", help="Path to resource")
 dataset_add_resource_parser.add_argument(
@@ -155,24 +161,24 @@
 
 # dataset rm-resource
 dataset_rm_resource_parser = dataset_subparsers.add_parser(
-    "rm-resource", help="Remove resource to existing dataset"
+    "rm-resource", description="Remove resource to existing dataset"
 )
 dataset_rm_resource_parser.add_argument(
-    "project_id", type=int, help="Project id"
+    "project_id", type=int, help="Project ID"
 )
 dataset_rm_resource_parser.add_argument(
-    "dataset_id", type=int, help="Dataset id"
+    "dataset_id", type=int, help="Dataset ID"
 )
 dataset_rm_resource_parser.add_argument(
-    "resource_id", type=int, help="Resource id"
+    "resource_id", type=int, help="Resource ID"
 )
 
 # dataset edit
 dataset_edit_parser = dataset_subparsers.add_parser(
-    "edit", help="Edit dataset", argument_default=ap.SUPPRESS
+    "edit", description="Edit dataset", argument_default=ap.SUPPRESS
 )
-dataset_edit_parser.add_argument("project_id", type=int, help="Project id")
-dataset_edit_parser.add_argument("dataset_id", type=int, help="Dataset id")
+dataset_edit_parser.add_argument("project_id", type=int, help="Project ID")
+dataset_edit_parser.add_argument("dataset_id", type=int, help="Dataset ID")
 dataset_edit_parser.add_argument("--name", help="New name of dataset")
 dataset_edit_parser.add_argument("--path", help="New path of dataset")
 dataset_edit_parser.add_argument(
@@ -198,32 +204,32 @@
 
 # dataset show
 dataset_show_parser = dataset_subparsers.add_parser(
-    "show", help="Show dataset", argument_default=ap.SUPPRESS
+    "show", description="Show dataset", argument_default=ap.SUPPRESS
 )
-dataset_show_parser.add_argument("project_id", type=int, help="Project id")
-dataset_show_parser.add_argument("dataset_id", type=int, help="Dataset id")
+dataset_show_parser.add_argument("project_id", type=int, help="Project ID")
+dataset_show_parser.add_argument("dataset_id", type=int, help="Dataset ID")
 
 # dataset delete
 dataset_delete_parser = dataset_subparsers.add_parser(
-    "delete", help="Delete dataset", argument_default=ap.SUPPRESS
+    "delete", description="Delete dataset", argument_default=ap.SUPPRESS
 )
-dataset_delete_parser.add_argument("project_id", type=int, help="Project id")
-dataset_delete_parser.add_argument("dataset_id", type=int, help="Dataset id")
+dataset_delete_parser.add_argument("project_id", type=int, help="Project ID")
+dataset_delete_parser.add_argument("dataset_id", type=int, help="Dataset ID")
 
 
 # TASK GROUP
-task_parser = subparsers_main.add_parser("task", help="task commands")
+task_parser = subparsers_main.add_parser("task", description="task commands")
 task_subparsers = task_parser.add_subparsers(
     title="Valid subcommands:", dest="subcmd", required=True
 )
 
 # task list
-task_list_parser = task_subparsers.add_parser("list", help="List tasks")
+task_list_parser = task_subparsers.add_parser("list", description="List tasks")
 
 # task collect
 task_collect_parser = task_subparsers.add_parser(
     "collect",
-    help="Install and collect all tasks a pip installable package exposes",
+    description="Install and collect all tasks from a pip-installable package",
 )
 task_collect_parser.add_argument(
     "package",
@@ -256,7 +262,7 @@
 # task check-collection
 task_check_collection_parser = task_subparsers.add_parser(
     "check-collection",
-    help="Check status of background task collection processes",
+    description="Check status of background task collection processes",
 )
 task_check_collection_parser.add_argument(
     "state_id",
@@ -271,7 +277,7 @@
 
 # task edit
 task_edit_parser = task_subparsers.add_parser(
-    "edit", help="Edit task", argument_default=ap.SUPPRESS
+    "edit", description="Edit task", argument_default=ap.SUPPRESS
 )
 task_edit_parser.add_argument(
     "task_id_or_name", help="ID or name of task to edit", type=str
@@ -291,7 +297,7 @@
 
 # task delete
 task_delete_parser = task_subparsers.add_parser(
-    "delete", help="Delete task", argument_default=ap.SUPPRESS
+    "delete", description="Delete task", argument_default=ap.SUPPRESS
 )
 task_delete_parser.add_argument(
     "task_id_or_name", help="ID or name of task to delete", type=str
@@ -300,15 +306,25 @@
 
 # WORKFLOW GROUP
 
-# workflow new
 workflow_parser = subparsers_main.add_parser(
-    "workflow", help="workflow commands"
+    "workflow", description="workflow commands"
 )
 workflow_subparsers = workflow_parser.add_subparsers(
     title="Valid subcommand", dest="subcmd", required=True
 )
+
+# workflow list
+workflow_list_parser = workflow_subparsers.add_parser(
+    "list", description="List workflows for given project"
+)
+workflow_list_parser.add_argument(
+    "project_id",
+    help="Project ID",
+)
+
+# workflow new
 workflow_new_parser = workflow_subparsers.add_parser(
-    "new", help="Create new workflow"
+    "new", description="Create new workflow"
 )
 workflow_new_parser.add_argument(
     "name",
@@ -319,40 +335,48 @@
     help="Project ID",
 )
 
-# workflow list
-workflow_list_parser = workflow_subparsers.add_parser(
-    "list", help="List workflows for given project"
+# workflow show
+workflow_new_parser = workflow_subparsers.add_parser(
+    "show", description="Show workflow"
 )
-workflow_list_parser.add_argument(
-    "project_id",
-    help="Project ID",
+workflow_new_parser.add_argument(
+    "workflow_id",
+    help="Workflow ID",
 )
 
-# workflow delete
-workflow_delete_parser = workflow_subparsers.add_parser(
-    "delete", help="Delete workflow"
+# workflow edit
+workflow_edit_parser = workflow_subparsers.add_parser(
+    "edit", description="Edit workflow", argument_default=ap.SUPPRESS
 )
-workflow_delete_parser.add_argument(
+workflow_edit_parser.add_argument(
     "workflow_id",
-    help="Workflow id",
+    type=int,
+    help="Workflow ID",
+)
+workflow_edit_parser.add_argument("--name", type=str, help="New workflow name")
+workflow_edit_parser.add_argument(
+    "--project-id",
+    type=int,
+    help="ID of the project to be associated with the current workflow",
 )
 
-# workflow show
-workflow_new_parser = workflow_subparsers.add_parser(
-    "show", help="Show a workflow"
+# workflow delete
+workflow_delete_parser = workflow_subparsers.add_parser(
+    "delete", description="Delete workflow"
 )
-workflow_new_parser.add_argument(
+workflow_delete_parser.add_argument(
     "workflow_id",
-    help="Workflow id",
+    help="Workflow ID",
 )
 
+
 # workflow add task
 workflow_add_task_parser = workflow_subparsers.add_parser(
-    "add-task", help="Add a new task to a specific workflow"
+    "add-task", description="Add new task to specific workflow"
 )
 workflow_add_task_parser.add_argument(
     "workflow_id",
-    help="Workflow id",
+    help="Workflow ID",
 )
 workflow_add_task_parser.add_argument(
     "task_id_or_name", help="ID or name of the new task", type=str
@@ -363,81 +387,68 @@
 workflow_add_task_parser.add_argument(
     "--args-file",
     help=(
-        "Path to a json serialised file containing the arguments "
+        "Path to json serialised file containing the arguments "
         "ovverrides of the task"
     ),
 )
 workflow_add_task_parser.add_argument(
     "--meta-file",
     help=(
-        "Path to a json serialised file containing the meta"
+        "Path to json serialised file containing the meta"
         "ovverrides of the task"
     ),
 )
 
 # workflow edit task
 workflow_edit_task_parser = workflow_subparsers.add_parser(
-    "edit-task", help="Edit a task within a specific workflow"
+    "edit-task", description="Edit task within specific workflow"
 )
 workflow_edit_task_parser.add_argument(
     "workflow_id",
-    help="Workflow id",
+    help="Workflow ID",
 )
 workflow_edit_task_parser.add_argument(
     "workflow_task_id",
-    help="Workflow task Id, the Id of a task inside the list of tasks",
+    help="Workflow task ID, the ID of a task inside the list of tasks",
 )
 workflow_edit_task_parser.add_argument(
     "--args-file",
     help=(
-        "Path to a json serialised file containing the arguments "
+        "Path to json serialised file containing the arguments "
         "ovverrides of the task"
     ),
 )
 workflow_edit_task_parser.add_argument(
     "--meta-file",
     help=(
-        "Path to a json serialised file containing the meta"
+        "Path to json serialised file containing the meta "
         "ovverrides of the task"
     ),
 )
 
 # workflow remove task
 workflow_remove_task_parser = workflow_subparsers.add_parser(
-    "rm-task", help="Remove a task in a specific workflow"
+    "rm-task", description="Remove task from a specific workflow"
 )
 workflow_remove_task_parser.add_argument(
     "workflow_id",
-    help="Workflow id",
+    help="Workflow ID",
 )
 workflow_remove_task_parser.add_argument(
     "workflow_task_id",
-    help="Workflow task Id, the Id of a task inside the list of tasks",
-)
-
-# workflow edit
-workflow_edit_parser = workflow_subparsers.add_parser(
-    "edit", help="Edit workflow", argument_default=ap.SUPPRESS
-)
-workflow_edit_parser.add_argument(
-    "workflow_id",
-    help="Workflow id",
-)
-workflow_edit_parser.add_argument("--name", help="New workflow name")
-
-workflow_edit_parser.add_argument(
-    "--project-id",
-    help="Change the project associated with the current workflow",
+    help="Workflow task ID (the ID of a task inside the list of tasks)",
 )
 
 # workflow apply
 workflow_apply_parser = workflow_subparsers.add_parser(
-    "apply", help="Apply workflow to dataset", argument_default=ap.SUPPRESS
+    "apply",
+    description="Apply workflow to dataset",
+    argument_default=ap.SUPPRESS,
 )
 workflow_apply_parser.add_argument("workflow_id")
 workflow_apply_parser.add_argument("input_dataset_id")
 workflow_apply_parser.add_argument(
-    "-o", "--output_dataset_id", help="Output dataset id"
+    "-o", "--output_dataset_id", help="Output dataset ID"
 )
 workflow_apply_parser.add_argument(
     "--overwrite-input",
@@ -448,7 +459,7 @@
 workflow_apply_parser.add_argument(
     "-p",
     "--project-id",
-    help="Id of project the workflow and dataset belong to",
+    help="ID of project the workflow and dataset belong to",
 )
 workflow_apply_parser.add_argument(
     "-w",
@@ -459,14 +470,14 @@
 
 # JOB GROUP
 
-job_parser = subparsers_main.add_parser("job", help="job commands")
+job_parser = subparsers_main.add_parser("job", description="job commands")
 job_subparsers = job_parser.add_subparsers(
     title="Valid subcommand", dest="subcmd", required=True
 )
 
 # job list
 job_list_parser = job_subparsers.add_parser(
-    "list", help="List jobs for given project"
+    "list", description="List jobs for given project"
 )
 job_list_parser.add_argument(
     "project_id",
@@ -476,12 +487,12 @@
 # job status
 job_status_parser = job_subparsers.add_parser(
     "status",
-    help="Query status of workflow-execution job",
+    description="Query status of workflow-execution job",
     argument_default=ap.SUPPRESS,
 )
 job_status_parser.add_argument(
     "job_id",
-    help="Id of the job",
+    help="ID of the job",
 )
 job_status_parser.add_argument(
     "--do-not-separate-logs",
@@ -493,11 +504,11 @@
 # job download-logs
 job_download_logs_parser = job_subparsers.add_parser(
     "download-logs",
-    help="Download full folder of workflow-execution job",
+    description="Download full folder of workflow-execution job",
 )
 job_download_logs_parser.add_argument(
     "job_id",
-    help="Id of the job",
+    help="ID of the job",
 )
 job_download_logs_parser.add_argument(
     "--output",
@@ -509,5 +520,5 @@
 
 # VERSION GROUP
 version_parser = subparsers_main.add_parser(
-    "version", help="Print version and exit"
+    "version", description="Print version and exit"
 )


Test patch:
diff --git a/tests/test_workflow.py b/tests/test_workflow.py
index 8e625e050..4e390d35a 100644
--- a/tests/test_workflow.py
+++ b/tests/test_workflow.py
@@ -53,6 +53,51 @@ async def test_workflow_delete(register_user, invoke):
     assert len(res_list.data) == 0
 
 
+async def test_workflow_edit(register_user, invoke):
+    # Create two projects
+    res_pj_1 = await invoke("project new project_name_1 /some/path")
+    res_pj_2 = await invoke("project new project_name_2 /some/path")
+    assert res_pj_1.retcode == 0
+    assert res_pj_2.retcode == 0
+    project_id_1 = res_pj_1.data["id"]
+    project_id_2 = res_pj_2.data["id"]
+
+    # Add a workflow to project 1
+    res_wf = await invoke(f"workflow new MyWorkflow {project_id_1}")
+    workflow_id = res_wf.data["id"]
+    assert res_wf.retcode == 0
+
+    # List workflows
+    res_list_1 = await invoke(f"workflow list {project_id_1}")
+    res_list_2 = await invoke(f"workflow list {project_id_2}")
+    assert res_list_1.retcode == 0
+    assert res_list_2.retcode == 0
+    assert len(res_list_1.data) == 1
+    assert len(res_list_2.data) == 0
+    assert res_list_1.data[0]["id"] == workflow_id
+
+    # Edit workflow's name and project
+    NAME = "New workflow name"
+    cmd = (
+        f"workflow edit {workflow_id} "
+        f'--name "{NAME}" --project-id {project_id_2}'
+    )
+    debug(cmd)
+    res_edit = await invoke(cmd)
+    assert res_edit.retcode == 0
+    debug(res_edit.data)
+
+    # List workflows, and check edit
+    res_list_1 = await invoke(f"workflow list {project_id_1}")
+    res_list_2 = await invoke(f"workflow list {project_id_2}")
+    assert res_list_1.retcode == 0
+    assert res_list_2.retcode == 0
+    assert len(res_list_1.data) == 0
+    assert len(res_list_2.data) == 1
+    assert res_list_2.data[0]["id"] == workflow_id
+    assert res_list_2.data[0]["name"] == NAME
+
+
 async def test_workflow_list(register_user, invoke):
     PROJECT_NAME = "project_name"
     PROJECT_PATH = "project_path"
@@ -103,7 +148,7 @@ async def test_workflow_list_when_two_projects_exist(
     assert len(res_list_2.data) == NUM_WF_PROJECT_2
 
 
-async def test_add_task(
+async def test_workflow_add_task(
     invoke,
     register_user,
     task_factory,
@@ -150,34 +195,7 @@ async def test_add_task(
     assert res.data.startswith("Added")
 
 
-async def test_remove_task(
-    invoke,
-    register_user,
-    task_factory,
-    workflow_factory,
-    tmp_path: Path,
-):
-    # Create workflow and task
-    wf = await workflow_factory()
-    t = await task_factory()
-
-    # Add task to workflow, twice
-    cmd = f"workflow add-task {wf.id} {t.id}"
-    res = await invoke(cmd)
-    assert res.retcode == 0
-    res = await invoke(cmd)
-    assert res.retcode == 0
-    workflow_task_id_1 = res.data["task_list"][0]["id"]
-
-    # Remove task 1 from workflow
-    cmd = f"workflow rm-task {wf.id} {workflow_task_id_1}"
-    debug(cmd)
-    res = await invoke(cmd)
-    assert res.retcode == 0
-    debug(res.data)
-
-
-async def test_add_task_by_name(
+async def test_workflow_add_task_by_name(
     invoke,
     register_user,
     task_factory,
@@ -203,7 +221,7 @@ async def test_add_task_by_name(
     assert res.data["task_list"][0]["id"] == task.id
 
 
-async def test_task_cache_fails_for_non_unique_names(
+async def test_task_cache_with_non_unique_names(
     invoke,
     register_user,
     task_factory,
@@ -241,7 +259,34 @@ async def test_task_cache_fails_for_non_unique_names(
         res = await invoke(cmd)
 
 
-async def test_edit_workflow_task(
+async def test_workflow_rm_task(
+    invoke,
+    register_user,
+    task_factory,
+    workflow_factory,
+    tmp_path: Path,
+):
+    # Create workflow and task
+    wf = await workflow_factory()
+    t = await task_factory()
+
+    # Add task to workflow, twice
+    cmd = f"workflow add-task {wf.id} {t.id}"
+    res = await invoke(cmd)
+    assert res.retcode == 0
+    res = await invoke(cmd)
+    assert res.retcode == 0
+    workflow_task_id_1 = res.data["task_list"][0]["id"]
+
+    # Remove task 1 from workflow
+    cmd = f"workflow rm-task {wf.id} {workflow_task_id_1}"
+    debug(cmd)
+    res = await invoke(cmd)
+    assert res.retcode == 0
+    debug(res.data)
+
+
+async def test_workflow_edit_task(
     invoke,
     register_user,
     task_factory,