Merge pull request #422 from fractal-analytics-platform/fix-meta-update-bug

Fixed bug in workflow task meta properties update

Author: zonia3000

CHANGELOG.md

@@ -1,5 +1,9 @@
 *Note: Numbers like (\#123) point to closed Pull Requests on the fractal-web repository.*
 
+# Unreleased
+
+* Fixed bug in workflow task meta properties update (\#422).
+
 # 0.10.0
 
 This release requires fractal-server 1.4.6.

src/lib/components/workflow/MetaPropertiesForm.svelte

@@ -14,17 +14,15 @@
 
 	// Workflow id
 	export let workflowId;
-	// Workflow task id
-	export let taskId;
-	// Workflow task meta properties
-	export let metaProperties;
-	export let originalMetaProperties;
+	/** @type {import('$lib/types').WorkflowTask} */
+	export let workflowTask;
 
-	if (metaProperties === null || metaProperties === undefined) {
-		metaProperties = {};
-	}
-	if (originalMetaProperties === null || originalMetaProperties === undefined) {
-		originalMetaProperties = {};
+	let metaProperties = {};
+	let originalMetaProperties = {};
+
+	$: {
+		metaProperties = workflowTask.meta || {};
+		updateOriginalMetaProperties();
 	}
 
 	async function handleEntryUpdate() {
@@ -34,20 +32,25 @@
 			const updatedMetaProperties = await updateFormEntry(
 				projectId,
 				workflowId,
-				taskId,
+				workflowTask.id,
 				modifiedProperties,
 				'meta'
 			);
+			workflowTask.meta = updatedMetaProperties.meta;
 			metaProperties = updatedMetaProperties.meta;
 			// Updating original properties again
-			for (let key in metaProperties) {
-				originalMetaProperties[key] = metaProperties[key];
-			}
+			updateOriginalMetaProperties();
 		} catch (error) {
 			console.log(error);
 			displayStandardErrorAlert(error, 'metaPropertiesFormError');
 		}
 	}
+
+	function updateOriginalMetaProperties() {
+		for (let key in metaProperties) {
+			originalMetaProperties[key] = metaProperties[key];
+		}
+	}
 </script>
 
 <div>

src/routes/projects/[projectId]/workflows/[workflowId]/+page.svelte

@@ -1,6 +1,5 @@
 <script>
-	import { onDestroy, onMount } from 'svelte';
-	import { writable } from 'svelte/store';
+	import { onMount } from 'svelte';
 	import { goto, beforeNavigate } from '$app/navigation';
 	import { page } from '$app/stores';
 	import ArgumentForm from '$lib/components/workflow/ArgumentForm.svelte';
@@ -29,13 +28,10 @@
 	 */
 	let availableTasks = [];
 
-	/** @type {import('svelte/store').Writable<import('$lib/types').WorkflowTask|undefined>} */
-	let workflowTaskContext = writable(undefined);
 	let workflowTabContextId = 0;
 	let workflowSuccessMessage = '';
 	/** @type {import('$lib/types').WorkflowTask|undefined} */
 	let selectedWorkflowTask = undefined;
-	let originalMetaProperties = {};
 	let checkingConfiguration = false;
 	let inputDatasetControl = '';
 	let outputDatasetControl = '';
@@ -77,16 +73,6 @@
 
 	$: updatableWorkflowList = workflow?.task_list || [];
 
-	const unsubscribe = workflowTaskContext.subscribe((value) => {
-		selectedWorkflowTask = value;
-		originalMetaProperties = {};
-		if (value && value.meta) {
-			for (let key in value.meta) {
-				originalMetaProperties[key] = value.meta[key];
-			}
-		}
-	});
-
 	onMount(async () => {
 		workflow = /** @type {import('$lib/types').Workflow} */ ($page.data.workflow);
 		project = workflow.project;
@@ -332,7 +318,7 @@
 
 		// Successfully deleted task
 		workflow = workflowResult;
-		workflowTaskContext.set(undefined);
+		selectedWorkflowTask = undefined;
 	}
 
 	async function setActiveWorkflowTaskContext(event) {
@@ -360,7 +346,7 @@
 	 * @param {import('$lib/types').WorkflowTask} wft
 	 */
 	function setWorkflowTaskContext(wft) {
-		workflowTaskContext.set(wft);
+		selectedWorkflowTask = wft;
 		// Check if args schema is available
 		argsSchemaAvailable =
 			wft.task.args_schema === undefined || wft.task.args_schema === null ? false : true;
@@ -570,8 +556,6 @@
 	async function updateNewVersionsCount(count) {
 		newVersionsCount = count;
 	}
-
-	onDestroy(unsubscribe);
 </script>
 
 <div class="d-flex justify-content-between align-items-center mb-4">
@@ -660,8 +644,8 @@
 							{#each workflow.task_list as workflowTask}
 								<button
 									style="cursor: pointer"
-									class="list-group-item list-group-item-action {$workflowTaskContext !==
-										undefined && $workflowTaskContext.id == workflowTask.id
+									class="list-group-item list-group-item-action {selectedWorkflowTask !==
+										undefined && selectedWorkflowTask.id == workflowTask.id
 										? 'active'
 										: ''}"
 									data-fs-target={workflowTask.id}
@@ -785,9 +769,7 @@
 										{#key selectedWorkflowTask}
 											<MetaPropertiesForm
 												workflowId={workflow.id}
-												taskId={selectedWorkflowTask.id}
-												metaProperties={selectedWorkflowTask.meta}
-												{originalMetaProperties}
+												workflowTask={selectedWorkflowTask}
 											/>
 										{/key}
 									{/if}

src/routes/projects/[projectId]/workflows/experimental/[workflowId]/+page.svelte

@@ -1,7 +1,6 @@
 <script>
 	import { env } from '$env/dynamic/public';
 	import { onDestroy, onMount } from 'svelte';
-	import { writable } from 'svelte/store';
 	import { beforeNavigate } from '$app/navigation';
 	import { page } from '$app/stores';
 	import ArgumentForm from '$lib/components/workflow/ArgumentForm.svelte';
@@ -42,13 +41,10 @@
 	/** @type {import('$lib/components/common/StandardErrorAlert.svelte').default|undefined} */
 	let workflowErrorAlert = undefined;
 
-	/** @type {import('svelte/store').Writable<import('$lib/types').WorkflowTask|undefined>} */
-	let workflowTaskContext = writable(undefined);
 	let workflowTabContextId = 0;
 	let workflowSuccessMessage = '';
 	/** @type {import('$lib/types').WorkflowTask|undefined} */
 	let selectedWorkflowTask = undefined;
-	let originalMetaProperties = {};
 	let checkingConfiguration = false;
 	let setSlurmAccount = true;
 	let slurmAccount =
@@ -91,16 +87,6 @@
 
 	$: updatableWorkflowList = workflow.task_list || [];
 
-	const unsubscribe = workflowTaskContext.subscribe((value) => {
-		selectedWorkflowTask = value;
-		originalMetaProperties = {};
-		if (value && value.meta) {
-			for (let key in value.meta) {
-				originalMetaProperties[key] = value.meta[key];
-			}
-		}
-	});
-
 	const updateJobsInterval = env.PUBLIC_UPDATE_JOBS_INTERVAL
 		? parseInt(env.PUBLIC_UPDATE_JOBS_INTERVAL)
 		: 3000;
@@ -345,7 +331,7 @@
 
 		// Successfully deleted task
 		workflow = workflowResult;
-		workflowTaskContext.set(undefined);
+		selectedWorkflowTask = undefined;
 	}
 
 	async function setActiveWorkflowTaskContext(event) {
@@ -369,12 +355,11 @@
 		unsavedChangesModal.toggle();
 	}
 
-
 	/**
 	 * @param {import('$lib/types').WorkflowTask} wft
 	 */
 	function setWorkflowTaskContext(wft) {
-		workflowTaskContext.set(wft);
+		selectedWorkflowTask = wft;
 		// Check if args schema is available
 		argsSchemaAvailable =
 			wft.task.args_schema === undefined || wft.task.args_schema === null ? false : true;
@@ -651,7 +636,6 @@
 
 	onDestroy(() => {
 		clearTimeout(statusWatcherTimer);
-		unsubscribe();
 	});
 </script>
 
@@ -805,8 +789,8 @@
 							{#each workflow.task_list as workflowTask}
 								<button
 									style="cursor: pointer"
-									class="list-group-item list-group-item-action {$workflowTaskContext !==
-										undefined && $workflowTaskContext.id == workflowTask.id
+									class="list-group-item list-group-item-action {selectedWorkflowTask !==
+										undefined && selectedWorkflowTask.id == workflowTask.id
 										? 'active'
 										: ''}"
 									data-fs-target={workflowTask.id}
@@ -933,9 +917,7 @@
 										{#key selectedWorkflowTask}
 											<MetaPropertiesForm
 												workflowId={workflow.id}
-												taskId={selectedWorkflowTask.id}
-												metaProperties={selectedWorkflowTask.meta}
-												{originalMetaProperties}
+												workflowTask={selectedWorkflowTask}
 											/>
 										{/key}
 									{/if}

tests/workflow_meta.spec.js

@@ -0,0 +1,46 @@
+import { waitPageLoading } from './utils.js';
+import { expect, test } from './workflow_fixture.js';
+
+test('Workflow task meta', async ({ page, workflow }) => {
+	await page.waitForURL(workflow.url);
+	await waitPageLoading(page);
+
+	await test.step('Add 2 tasks to workflow', async () => {
+		await workflow.addFakeTask();
+		await workflow.addFakeTask();
+	});
+
+	await test.step('Select first task meta', async () => {
+		await page.getByText('Fake Task').first().click();
+		await page.getByText('Meta', { exact: true }).click();
+	});
+
+	await test.step('Add meta property to first task', async () => {
+		await page.getByRole('button', { name: 'Add property' }).click();
+		await page.getByRole('textbox').nth(0).fill('key1');
+		await page.getByRole('textbox').nth(1).fill('value1');
+		await page.getByLabel('Save argument').click();
+		await page.waitForFunction(
+			() => document.querySelectorAll('#meta-tab input[type="text"]').length === 0
+		);
+	});
+
+	await test.step('Select second task and then go back to first task', async () => {
+		await page.getByText('Fake Task').nth(1).click();
+		await page.getByText('Fake Task').nth(0).click();
+	});
+
+	await test.step('Verify that the inserted values are still there', async () => {
+		await expect(page.getByText('key1')).toBeVisible();
+		await expect(page.getByText('value1')).toBeVisible();
+	});
+
+	await test.step('Verify that the inserted values have been saved', async () => {
+		await page.reload();
+		await waitPageLoading(page);
+		await page.getByText('Fake Task').first().click();
+		await page.getByText('Meta', { exact: true }).click();
+		await expect(page.getByText('key1')).toBeVisible();
+		await expect(page.getByText('value1')).toBeVisible();
+	});
+});