Merge pull request #475 from fractal-analytics-platform/fix-optional-task-args-version

Fixed issue in task version update with optional arguments

Author: zonia3000

CHANGELOG.md

@@ -1,5 +1,9 @@
 *Note: Numbers like (\#123) point to closed Pull Requests on the fractal-web repository.*
 
+# Unreleased
+
+* fixed issue in task version update with optional arguments (\#475).
+
 # 1.0.1
 
 * used payload containing all fields in meta properties PATCH endpoint (\#473).

src/lib/components/v2/workflow/ArgumentsSchema.svelte

@@ -111,11 +111,13 @@
 
 		const result = await response.json();
 		if (response.ok) {
+			const argsNonParallel = deepCopyArgs(result.args_non_parallel);
+			const argsParallel = deepCopyArgs(result.args_parallel);
 			if (result.args_non_parallel) {
-				nonParallelSchemaComponent?.setArguments(result.args_non_parallel);
+				nonParallelSchemaComponent?.setArguments(argsNonParallel);
 			}
 			if (result.args_parallel) {
-				parallelSchemaComponent?.setArguments(result.args_parallel);
+				parallelSchemaComponent?.setArguments(argsParallel);
 			}
 			onWorkflowTaskUpdated(result);
 		} else {
@@ -124,6 +126,17 @@
 		savingChanges = false;
 	}
 
+	/**
+	 * Returns a deep copy of the arguments object to avoid side effects from the JSchema component.
+	 * @param {object|null} args
+	 */
+	function deepCopyArgs(args) {
+		if (typeof args === 'object') {
+			return JSON.parse(JSON.stringify(args));
+		}
+		return null;
+	}
+
 	/**
 	 * @param {string} argsSchemaVersion
 	 */

tests/v2/task_version_update.spec.js

@@ -1,4 +1,4 @@
-import { test } from './workflow_fixture.js';
+import { expect, test } from './workflow_fixture.js';
 import { waitPageLoading } from '../utils.js';
 import { createFakeTask, deleteTask } from './task_utils.js';
 
@@ -16,7 +16,10 @@ test('Task version update [v2]', async ({ page, workflow }) => {
 			version: '0.0.1',
 			args_schema_non_parallel: {
 				properties: {
-					p1: { type: 'string' }
+					p1: { type: 'string' },
+					// Optional arguments added to test bug #474
+					p2: { type: 'string', title: 'p2' },
+					p3: { type: 'string', title: 'p3' }
 				},
 				type: 'object'
 			}
@@ -27,7 +30,9 @@ test('Task version update [v2]', async ({ page, workflow }) => {
 			version: '0.0.2',
 			args_schema_non_parallel: {
 				properties: {
-					p1: { type: 'string' }
+					p1: { type: 'string' },
+					p2: { type: 'string', title: 'p2' },
+					p3: { type: 'string', title: 'p3' }
 				},
 				type: 'object',
 				required: ['p1']
@@ -101,12 +106,19 @@ test('Task version update [v2]', async ({ page, workflow }) => {
 		await workflow.selectTask(nonParallelTask);
 	});
 
+	await test.step('Fill optional argument p2', async () => {
+		await page.getByRole('textbox', { name: 'p2' }).fill('foo');
+		await page.getByRole('button', { name: 'Save changes' }).click();
+		await page.getByText('Arguments changes saved successfully').waitFor();
+	});
+
 	await test.step('Update non parallel task to v2', async () => {
 		await page.getByRole('button', { name: 'Version' }).click();
 		await page
 			.getByRole('combobox', { name: 'New versions of this task exist:' })
 			.selectOption('0.0.2');
 		await page.getByText('Following errors must be fixed before performing the update').waitFor();
+		await expect(page.locator('.alert-danger li')).toHaveCount(1);
 		await page
 			.getByRole('textbox', { name: 'Fix the non parallel arguments:' })
 			.fill('{"p1": "test"}');

tests/v2/workflow_task_meta_properties.spec.js

@@ -26,23 +26,23 @@ test('Workflow task meta properties [v2]', async ({ page, workflow }) => {
 
 	await test.step('Add first property', async () => {
 		await page.getByRole('button', { name: 'Add property' }).click();
-		await page.getByPlaceholder('Arg name').fill('k1');
-		await page.getByPlaceholder('Argument default value').fill('v1');
+		await page.getByPlaceholder('Arg name').fill('arg-k1');
+		await page.getByPlaceholder('Argument default value').fill('arg-v1');
 		await page.getByLabel('Save argument').click();
 	});
 
 	await test.step('Add second property', async () => {
 		await page.getByRole('button', { name: 'Add property' }).click();
-		await page.getByPlaceholder('Arg name').fill('k2');
-		await page.getByPlaceholder('Argument default value').fill('v2');
+		await page.getByPlaceholder('Arg name').fill('arg-k2');
+		await page.getByPlaceholder('Argument default value').fill('arg-v2');
 		await page.getByLabel('Save argument').click();
 	});
 
 	await test.step('Verify that meta contains the 2 properties', async () => {
-		await page.getByText('k1').waitFor();
-		await page.getByText('k2').waitFor();
-		await page.getByText('v1').waitFor();
-		await page.getByText('v2').waitFor();
+		await page.getByText('arg-k1').waitFor();
+		await page.getByText('arg-k2').waitFor();
+		await page.getByText('arg-v1').waitFor();
+		await page.getByText('arg-v2').waitFor();
 	});
 
 	await test.step('Cleanup', async () => {