From 9af9158a4d4844bff2bf4f96000f45d7fba7dc28 Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Mon, 7 Oct 2024 15:45:35 +0200 Subject: [PATCH 01/16] Used new group_ids_names field --- __tests__/v2/UserEditor.test.js | 2 +- src/lib/components/v2/admin/UserEditor.svelte | 10 +++++----- src/lib/server/api/auth_api.js | 12 ++++++------ src/lib/types.d.ts | 3 +-- src/routes/profile/+page.svelte | 7 ++++--- src/routes/v2/admin/users/[userId]/+page.svelte | 6 +++--- src/routes/v2/admin/users/[userId]/edit/+page.svelte | 2 +- src/routes/v2/admin/users/register/+page.svelte | 4 ++-- 8 files changed, 23 insertions(+), 23 deletions(-) diff --git a/__tests__/v2/UserEditor.test.js b/__tests__/v2/UserEditor.test.js index 0f7849d0..9ffe64d0 100644 --- a/__tests__/v2/UserEditor.test.js +++ b/__tests__/v2/UserEditor.test.js @@ -29,7 +29,7 @@ describe('UserEditor', () => { const selectedUser = { id: 1, - group_ids: [] + group_ids_names: [] }; const initialSettings = { diff --git a/src/lib/components/v2/admin/UserEditor.svelte b/src/lib/components/v2/admin/UserEditor.svelte index 955aed0e..a152f38d 100644 --- a/src/lib/components/v2/admin/UserEditor.svelte +++ b/src/lib/components/v2/admin/UserEditor.svelte @@ -9,7 +9,7 @@ import SlimSelect from 'slim-select'; import StandardDismissableAlert from '$lib/components/common/StandardDismissableAlert.svelte'; - /** @type {import('$lib/types').User & {group_ids: number[]}} */ + /** @type {import('$lib/types').User & {group_ids_names: Array<[number, string]>}} */ export let user; /** @type {Array} */ export let groups = []; @@ -26,12 +26,12 @@ /** @type {number[]} */ let groupIdsToAdd = []; - $: userGroups = user.group_ids - .map((id) => groups.filter((g) => g.id === id)[0]) + $: userGroups = user.group_ids_names + .map((ni) => groups.filter((g) => g.id === ni[0])[0]) .sort(sortGroupByNameComparator); $: availableGroups = groups - .filter((g) => !user.group_ids.includes(g.id)) + .filter((g) => !user.group_ids_names.map((ni) => ni[0]).includes(g.id)) .filter((g) => !groupIdsToAdd.includes(g.id)) .sort(sortGroupByNameComparator); @@ -225,7 +225,7 @@ const result = await response.json(); if (response.ok) { - user = { ...user, group_ids: result.group_ids }; + user = { ...user, group_ids_names: result.group_ids_names }; groupIdsToAdd = []; return true; } diff --git a/src/lib/server/api/auth_api.js b/src/lib/server/api/auth_api.js index 19ccd739..7e95362b 100644 --- a/src/lib/server/api/auth_api.js +++ b/src/lib/server/api/auth_api.js @@ -29,12 +29,12 @@ export async function userAuthentication(fetch, data) { /** * Fetches user identity * @param {typeof fetch} fetch - * @param {boolean} groupNames indicates whether to load the list of group names. + * @param {boolean} groupIdsNames indicates whether to load the list of group IDs and names. * @returns {Promise} */ -export async function getCurrentUser(fetch, groupNames = false) { +export async function getCurrentUser(fetch, groupIdsNames = false) { logger.debug('Retrieving current user'); - const url = `${env.FRACTAL_SERVER_HOST}/auth/current-user/?group_names=${groupNames}`; + const url = `${env.FRACTAL_SERVER_HOST}/auth/current-user/?group_ids_names=${groupIdsNames}`; const response = await fetch(url, { method: 'GET', credentials: 'include' @@ -135,13 +135,13 @@ export async function listUsers(fetch) { * Fetches a user from the server * @param {typeof fetch} fetch * @param {number|string} userId - * @param {boolean} groupIds indicates whether to load the list of group IDs. + * @param {boolean} groupIdsNames indicates whether to load the list of group IDs and names. * @returns {Promise} */ -export async function getUser(fetch, userId, groupIds = true) { +export async function getUser(fetch, userId, groupIdsNames = true) { logger.debug('Fetching user [user_id=%d]', userId); const response = await fetch( - `${env.FRACTAL_SERVER_HOST}/auth/users/${userId}/?group_ids=${groupIds}`, + `${env.FRACTAL_SERVER_HOST}/auth/users/${userId}/?group_ids_names=${groupIdsNames}`, { method: 'GET', credentials: 'include' diff --git a/src/lib/types.d.ts b/src/lib/types.d.ts index c48f22fe..138f33e0 100644 --- a/src/lib/types.d.ts +++ b/src/lib/types.d.ts @@ -112,8 +112,7 @@ export type User = { is_verified: boolean username: string | null password?: string - group_names?: string[] - group_ids?: number[] + group_ids_names : Array<[number, string]> | null oauth_accounts: Array<{ id: number account_email: string diff --git a/src/routes/profile/+page.svelte b/src/routes/profile/+page.svelte index 8346cb11..1fe54c6e 100644 --- a/src/routes/profile/+page.svelte +++ b/src/routes/profile/+page.svelte @@ -2,7 +2,7 @@ import { page } from '$app/stores'; import BooleanIcon from '$lib/components/common/BooleanIcon.svelte'; - /** @type {import('$lib/types').User & {group_names: string[]}} */ + /** @type {import('$lib/types').User & {group_ids_names: Array<[number, string]>}} */ $: user = $page.data.user; @@ -43,8 +43,9 @@ Groups - {#each user.group_names as group} - {group} + + {#each user.group_ids_names as [_, group_name]} + {group_name} {/each} diff --git a/src/routes/v2/admin/users/[userId]/+page.svelte b/src/routes/v2/admin/users/[userId]/+page.svelte index 85b3d4fb..04f4106a 100644 --- a/src/routes/v2/admin/users/[userId]/+page.svelte +++ b/src/routes/v2/admin/users/[userId]/+page.svelte @@ -3,7 +3,7 @@ import { sortGroupByNameComparator } from '$lib/common/user_utilities'; import BooleanIcon from '$lib/components/common/BooleanIcon.svelte'; - /** @type {import('$lib/types').User & {group_ids: number[]}} */ + /** @type {import('$lib/types').User & {group_ids_names: Array<[number, string]>}} */ const user = $page.data.user; /** @type {import('$lib/types').UserSettings} */ const settings = $page.data.settings; @@ -12,8 +12,8 @@ /** @type {string} */ const runnerBackend = $page.data.runnerBackend; - $: userGroups = user.group_ids - .map((id) => groups.filter((g) => g.id === id)[0]) + $: userGroups = user.group_ids_names + .map((ni) => groups.filter((g) => g.id === ni[0])[0]) .sort(sortGroupByNameComparator); diff --git a/src/routes/v2/admin/users/[userId]/edit/+page.svelte b/src/routes/v2/admin/users/[userId]/edit/+page.svelte index d53cbf92..61454867 100644 --- a/src/routes/v2/admin/users/[userId]/edit/+page.svelte +++ b/src/routes/v2/admin/users/[userId]/edit/+page.svelte @@ -2,7 +2,7 @@ import { page } from '$app/stores'; import UserEditor from '$lib/components/v2/admin/UserEditor.svelte'; - /** @type {import('$lib/types').User & {group_ids: number[]}} */ + /** @type {import('$lib/types').User & {group_ids_names: Array<[number, string]>}} */ let user = $page.data.user; /** @type {import('$lib/types').UserSettings} */ diff --git a/src/routes/v2/admin/users/register/+page.svelte b/src/routes/v2/admin/users/register/+page.svelte index 4cb15d8c..60ab60ff 100644 --- a/src/routes/v2/admin/users/register/+page.svelte +++ b/src/routes/v2/admin/users/register/+page.svelte @@ -3,7 +3,7 @@ import UserEditor from '$lib/components/v2/admin/UserEditor.svelte'; import { onMount } from 'svelte'; - /** @type {import('$lib/types').User & {group_ids: number[]}} */ + /** @type {import('$lib/types').User & {group_ids_names: Array<[number, string]>}} */ let user = { email: '', is_active: true, @@ -11,7 +11,7 @@ is_verified: false, username: '', password: '', - group_ids: [], + group_ids_names: [], oauth_accounts: [] }; From fca9b7c09c13f9574b22e82a2a95e4f35bce995d Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Mon, 7 Oct 2024 15:47:51 +0200 Subject: [PATCH 02/16] Removed owner from v2 tasks --- src/lib/common/component_utilities.js | 24 +++++-- .../v2/projects/CreateWorkflowModal.svelte | 10 +-- .../v2/projects/WorkflowsList.svelte | 15 +---- .../components/v2/tasks/TaskEditModal.svelte | 7 -- .../components/v2/tasks/TaskInfoModal.svelte | 2 - .../components/v2/workflow/TaskInfoTab.svelte | 2 - .../v2/workflow/WorkflowTaskSelection.svelte | 66 +++++++------------ .../components/v2/workflow/version-checker.js | 2 +- src/lib/types-v2.d.ts | 3 +- src/routes/v2/admin/tasks/+page.svelte | 4 -- .../workflows/[workflowId]/+page.svelte | 7 +- src/routes/v2/tasks/+page.server.js | 4 +- src/routes/v2/tasks/+page.svelte | 7 +- tests/v2/add_single_task.spec.js | 45 ++++++------- tests/v2/task_utils.js | 4 ++ 15 files changed, 82 insertions(+), 120 deletions(-) diff --git a/src/lib/common/component_utilities.js b/src/lib/common/component_utilities.js index 5c22f330..ca7541a3 100644 --- a/src/lib/common/component_utilities.js +++ b/src/lib/common/component_utilities.js @@ -77,11 +77,9 @@ export function compareTaskNameAscAndVersionDesc(t1, t2) { } /** - * @template {import('$lib/types').Task|import('$lib/types-v2').TaskV2} T - * @param {Array} tasks - * @param {string|null} ownerName + * @param {Array} tasks * @param {'asc'|'desc'} order - * @returns {Array} + * @returns {Array} */ export function orderTasksByOwnerThenByNameThenByVersion(tasks, ownerName = null, order = 'asc') { const sortingFunction = getVersionSortingFunction(order); @@ -121,6 +119,24 @@ export function orderTasksByOwnerThenByNameThenByVersion(tasks, ownerName = null }); } +/** + * @param {Array} tasks + * @param {'asc'|'desc'} order + * @returns {Array} + */ +export function orderTasksByGroupThenByNameThenByVersion(tasks, order = 'asc') { + const sortingFunction = getVersionSortingFunction(order); + return tasks.sort((t1, t2) => { + if (t1.taskgroupv2_id < t2.taskgroupv2_id) { + return -1; + } + if (t1.taskgroupv2_id > t2.taskgroupv2_id) { + return 1; + } + return sortingFunction(t1, t2); + }); +} + /** * @param {'asc'|'desc'} order */ diff --git a/src/lib/components/v2/projects/CreateWorkflowModal.svelte b/src/lib/components/v2/projects/CreateWorkflowModal.svelte index c6e5c306..8f0ae6fc 100644 --- a/src/lib/components/v2/projects/CreateWorkflowModal.svelte +++ b/src/lib/components/v2/projects/CreateWorkflowModal.svelte @@ -5,7 +5,7 @@ import { goto } from '$app/navigation'; import { tick } from 'svelte'; - /** @type {(workflow: import('$lib/types-v2').WorkflowV2, warning: string) => void} */ + /** @type {(workflow: import('$lib/types-v2').WorkflowV2) => void} */ export let handleWorkflowImported; // Component properties @@ -96,15 +96,9 @@ /** @type {import('$lib/types-v2').WorkflowV2} */ const workflow = result; - let customTaskWarning = ''; await tick(); - const customTasks = workflow.task_list.map((w) => w.task).filter((t) => t.owner); - if (customTasks.length > 0) { - customTaskWarning = `Custom tasks (e.g. "${customTasks[0].name}") are not meant to be portable; workflow "${workflow.name}" was imported, but it may not work as expected.`; - } - - handleWorkflowImported(workflow, customTaskWarning); + handleWorkflowImported(workflow); } else { console.error('Import workflow failed', result); throw new AlertError(result, response.status); diff --git a/src/lib/components/v2/projects/WorkflowsList.svelte b/src/lib/components/v2/projects/WorkflowsList.svelte index 1202b687..ab16f60a 100644 --- a/src/lib/components/v2/projects/WorkflowsList.svelte +++ b/src/lib/components/v2/projects/WorkflowsList.svelte @@ -5,7 +5,6 @@ import CreateWorkflowModal from './CreateWorkflowModal.svelte'; import { onMount } from 'svelte'; import { saveSelectedDataset } from '$lib/common/workflow_utilities'; - import StandardDismissableAlert from '$lib/components/common/StandardDismissableAlert.svelte'; // The list of workflows /** @type {import('$lib/types-v2').WorkflowV2[]} */ @@ -15,8 +14,6 @@ let workflowSearch = ''; - let customTaskWarning = ''; - $: filteredWorkflows = workflows.filter((p) => p.name.toLowerCase().includes(workflowSearch.toLowerCase()) ); @@ -49,16 +46,11 @@ /** * @param {import('$lib/types-v2').WorkflowV2} importedWorkflow - * @param {string} warningMessage */ - function handleWorkflowImported(importedWorkflow, warningMessage) { + function handleWorkflowImported(importedWorkflow) { workflows.push(importedWorkflow); workflows = workflows; - if (warningMessage) { - customTaskWarning = warningMessage; - } else { - goto(`/v2/projects/${projectId}/workflows/${importedWorkflow.id}`); - } + goto(`/v2/projects/${projectId}/workflows/${importedWorkflow.id}`); } onMount(() => { @@ -91,7 +83,6 @@ class="btn btn-primary float-end" type="submit" on:click={() => { - customTaskWarning = ''; createWorkflowModal.show(); }} > @@ -102,8 +93,6 @@ - - diff --git a/src/lib/components/v2/tasks/TaskEditModal.svelte b/src/lib/components/v2/tasks/TaskEditModal.svelte index 599359e2..7b5abc5d 100644 --- a/src/lib/components/v2/tasks/TaskEditModal.svelte +++ b/src/lib/components/v2/tasks/TaskEditModal.svelte @@ -153,13 +153,6 @@ -
- -
- -
-
- {#if task.command_non_parallel !== null}
+ + + + + + + + {#each rows as row} + + + + {#each row.tasks as task} + + + + + + {/each} + {/each} + +
{groupByLabels[groupBy]}Version
{row.groupTitle}
{task.taskVersions[task.selectedVersion]['name']} + {#if Object.keys(task.taskVersions).length > 1} + + {:else} + {task.taskVersions[task.selectedVersion]['version']} + {/if} + + +
+ + + {/if} + + +
+
+ + +
+ + + + diff --git a/src/lib/components/v2/workflow/WorkflowTaskSelection.svelte b/src/lib/components/v2/workflow/WorkflowTaskSelection.svelte deleted file mode 100644 index 4e871d52..00000000 --- a/src/lib/components/v2/workflow/WorkflowTaskSelection.svelte +++ /dev/null @@ -1,275 +0,0 @@ - - -{#if tasks} -
-
-
-
-
- -
-
- -
-
-
-
- - {#if selectedTypeOfTask} - - {#each selectedMapTaskVersions as task} - - {/each} - - {/if} - {/if} -
-
-
-{/if} diff --git a/src/lib/components/v2/workflow/version-checker.js b/src/lib/components/v2/workflow/version-checker.js index 4adf1619..8da87f80 100644 --- a/src/lib/components/v2/workflow/version-checker.js +++ b/src/lib/components/v2/workflow/version-checker.js @@ -31,7 +31,6 @@ export async function getAllNewVersions(tasks) { (task.args_schema_non_parallel !== null || task.args_schema_parallel !== null) && task.version !== null && t.name === task.name && - t.taskgroupv2_id === task.taskgroupv2_id && t.type === task.type && t.version && (t.args_schema_non_parallel || t.args_schema_parallel) && diff --git a/src/lib/types-v2.d.ts b/src/lib/types-v2.d.ts index 74826a87..a6cfe2ac 100644 --- a/src/lib/types-v2.d.ts +++ b/src/lib/types-v2.d.ts @@ -150,7 +150,7 @@ export type TaskV2Info = { export type TaskGroupV2 = { id: number - task_list: TaskV2 + task_list: TaskV2[] user_id: number user_group_id: number origin: string @@ -163,3 +163,13 @@ export type TaskGroupV2 = { active: boolean timestamp_created: string } + +export type TasksTableRow = { + groupTitle: string + tasks: Array<{ + selectedVersion: string + taskVersions: { + [version: string]: { [key: string]: string } + } + }> +} diff --git a/src/routes/v2/projects/[projectId]/workflows/[workflowId]/+page.svelte b/src/routes/v2/projects/[projectId]/workflows/[workflowId]/+page.svelte index b8f9e701..4a760128 100644 --- a/src/routes/v2/projects/[projectId]/workflows/[workflowId]/+page.svelte +++ b/src/routes/v2/projects/[projectId]/workflows/[workflowId]/+page.svelte @@ -6,7 +6,6 @@ import ConfirmActionButton from '$lib/components/common/ConfirmActionButton.svelte'; import MetaPropertiesForm from '$lib/components/v2/workflow/MetaPropertiesForm.svelte'; import ArgumentsSchema from '$lib/components/v2/workflow/ArgumentsSchema.svelte'; - import WorkflowTaskSelection from '$lib/components/v2/workflow/WorkflowTaskSelection.svelte'; import { AlertError, displayStandardErrorAlert } from '$lib/common/errors'; import Modal from '$lib/components/common/Modal.svelte'; import StandardDismissableAlert from '$lib/components/common/StandardDismissableAlert.svelte'; @@ -20,6 +19,7 @@ import InputFiltersTab from '$lib/components/v2/workflow/InputFiltersTab.svelte'; import RunWorkflowModal from '$lib/components/v2/workflow/RunWorkflowModal.svelte'; import { getSelectedWorkflowDataset, saveSelectedDataset } from '$lib/common/workflow_utilities'; + import AddWorkflowTaskModal from '$lib/components/v2/workflow/AddWorkflowTaskModal.svelte'; /** @type {import('$lib/types-v2').WorkflowV2} */ let workflow = $page.data.workflow; @@ -28,11 +28,6 @@ $: project = workflow.project; /** @type {import('$lib/types-v2').DatasetV2[]} */ let datasets = $page.data.datasets; - /** - * List of available tasks to be inserted into workflow - * @type {Array} - */ - let availableTasks = []; /** @type {number|undefined} */ let selectedDatasetId = undefined; @@ -59,12 +54,6 @@ let argsChangesSaved = false; - // Create workflow task modal - /** @type {number|undefined} */ - let taskOrder = undefined; - /** @type {WorkflowTaskSelection|undefined} */ - let workflowTaskSelectionComponent = undefined; - /** @type {InputFiltersTab|undefined} */ let inputFiltersTab = undefined; @@ -82,8 +71,8 @@ let runWorkflowModal; /** @type {import('$lib/components/v2/workflow/TasksOrderModal.svelte').default} */ let editTasksOrderModal; - /** @type {Modal} */ - let insertTaskModal; + /** @type {AddWorkflowTaskModal} */ + let addWorkflowTaskModal; /** @type {Modal} */ let editWorkflowModal; @@ -93,8 +82,6 @@ /** @type {import('$lib/types-v2').ApplyWorkflowV2|undefined} */ let selectedSubmittedJob; - let customTaskWarning = ''; - $: updatableWorkflowList = workflow.task_list || []; $: sortedDatasets = datasets.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); @@ -157,14 +144,6 @@ return; } - customTaskWarning = ''; - await tick(); - const customTasks = workflow.task_list.map((w) => w.task); - - if (customTasks.length > 0) { - customTaskWarning = `Custom tasks (e.g. "${customTasks[0].name}") are not meant to be portable; re-importing this workflow may not work as expected.`; - } - const response = await fetch(`/api/v2/project/${project.id}/workflow/${workflow.id}/export`, { method: 'GET', credentials: 'include' @@ -195,30 +174,6 @@ } } - async function getAvailableTasks() { - resetCreateWorkflowTaskModal(); - await loadAvailableTasks(); - } - - async function loadAvailableTasks() { - workflowTaskSelectionComponent?.setLoadingTasks(true); - const response = await fetch( - `/api/v2/task?args_schema_parallel=false&args_schema_non_parallel=false`, - { - method: 'GET', - credentials: 'include' - } - ); - - if (response.ok) { - availableTasks = await response.json(); - } else { - console.error(response); - availableTasks = []; - } - workflowTaskSelectionComponent?.setLoadingTasks(false); - } - function resetWorkflowUpdateModal() { if (!workflow) { return; @@ -268,84 +223,13 @@ ); } - function resetCreateWorkflowTaskModal() { - taskOrder = undefined; - insertTaskModal.hideErrorAlert(); - workflowTaskSelectionComponent?.reset(); - } - - let creatingWorkflowTask = false; - /** - * Creates a new project's workflow task in the server. - * @returns {Promise} + * @param {import('$lib/types-v2').WorkflowV2} updatedWorkflow */ - async function handleCreateWorkflowTask() { - insertTaskModal.confirmAndHide( - async () => { - if (!workflow) { - return; - } - workflowSuccessMessage = ''; - creatingWorkflowTask = true; - - const taskId = workflowTaskSelectionComponent?.getSelectedTaskId(); - if (taskId === undefined) { - console.error('Missing selected task id'); - return; - } - - const headers = new Headers(); - headers.set('Content-Type', 'application/json'); - - // Creating workflow task - const workflowTaskResponse = await fetch( - `/api/v2/project/${project.id}/workflow/${workflow.id}/wftask?task_id=${taskId}`, - { - method: 'POST', - credentials: 'include', - headers, - body: JSON.stringify({ - order: taskOrder - }) - } - ); - - const workflowTaskResult = await workflowTaskResponse.json(); - - if (!workflowTaskResponse.ok) { - console.error('Error while creating workflow task', workflowTaskResult); - throw new AlertError(workflowTaskResult); - } - console.log('Workflow task created'); - - // Get updated workflow with created task - const workflowResponse = await fetch( - `/api/v2/project/${project.id}/workflow/${workflow.id}`, - { - method: 'GET', - credentials: 'include' - } - ); - - const workflowResult = await workflowResponse.json(); - - if (!workflowResponse.ok) { - console.error('Error while retrieving workflow', workflowResult); - throw new AlertError(workflowResult); - } - - // Update workflow - workflow = workflowResult; - // UI Feedback - workflowSuccessMessage = 'Workflow task created'; - resetCreateWorkflowTaskModal(); - await checkNewVersions(); - }, - () => { - creatingWorkflowTask = false; - } - ); + async function onWorkflowTaskAdded(updatedWorkflow) { + workflow = updatedWorkflow; + workflowSuccessMessage = 'Workflow task created'; + await checkNewVersions(); } /** @@ -781,8 +665,6 @@
- - {#if workflow} @@ -814,10 +696,11 @@
@@ -1009,40 +892,7 @@
{/if} - - -
New workflow task
- - -
-
-
- -
- -
- - -
- - -
- - + From 6b6a0203556fae9838fdf1a5162c6c8dc58afbbd Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Thu, 10 Oct 2024 12:22:24 +0200 Subject: [PATCH 06/16] Updated tests --- __tests__/v2/VersionUpdate.test.js | 5 -- playwright.config.js | 2 +- tests/v2/admin_jobs.spec.js | 4 +- tests/v2/custom_env_task.spec.js | 12 +-- ...export_import_workflow_custom_task.spec.js | 76 ------------------- tests/v2/import_export_args.spec.js | 12 +-- tests/v2/jschema.spec.js | 35 +-------- tests/v2/run_mock_tasks.spec.js | 4 +- tests/v2/slurm_account.spec.js | 2 +- tests/v2/stop_workflow.spec.js | 2 +- tests/v2/task_version_update.spec.js | 18 ++--- tests/v2/tasks_admin.spec.js | 2 +- tests/v2/workflow_fixture.js | 51 +++++-------- tests/v2/workflow_task_input_filters.spec.js | 4 +- .../v2/workflow_task_meta_properties.spec.js | 2 +- tests/v2/workflow_task_without_schema.spec.js | 2 +- 16 files changed, 57 insertions(+), 176 deletions(-) delete mode 100644 tests/v2/export_import_workflow_custom_task.spec.js diff --git a/__tests__/v2/VersionUpdate.test.js b/__tests__/v2/VersionUpdate.test.js index 373a9edb..f95b234c 100644 --- a/__tests__/v2/VersionUpdate.test.js +++ b/__tests__/v2/VersionUpdate.test.js @@ -210,11 +210,6 @@ describe('VersionUpdate', () => { expect(screen.getByText('Invalid JSON')).toBeDefined(); }); - it('no new versions available for null owner', async () => { - const task = getTask('My Other Task', '1.2.3'); - await checkVersions(task, 0); - }); - it('no new versions available for admin owner', async () => { const task = getTask('My Other Task', '1.3.0'); await checkVersions(task, 0); diff --git a/playwright.config.js b/playwright.config.js index 717580a8..5be49395 100644 --- a/playwright.config.js +++ b/playwright.config.js @@ -107,7 +107,7 @@ export default defineConfig({ webServer: [ { - command: './tests/start-test-server.sh 2.6.3', + command: './tests/start-test-server.sh 2.7.0a3', port: 8000, waitForPort: true, stdout: 'pipe', diff --git a/tests/v2/admin_jobs.spec.js b/tests/v2/admin_jobs.spec.js index f17ad3e5..0bb940bb 100644 --- a/tests/v2/admin_jobs.spec.js +++ b/tests/v2/admin_jobs.spec.js @@ -20,7 +20,7 @@ test('Execute a job and show it on the job tables [v2]', async ({ page, request let dataset1; await test.step('Create first job and wait its failure', async () => { const job = await createJob(page, request, async function (workflow) { - await workflow.addCollectedTask('generic_task'); + await workflow.addTask('generic_task'); await workflow.selectTask('generic_task'); await page.getByRole('switch').check(); await page.getByRole('button', { name: 'Save changes' }).click(); @@ -41,7 +41,7 @@ test('Execute a job and show it on the job tables [v2]', async ({ page, request let jobId2; await test.step('Create second job', async () => { const job = await createJob(page, request, async function (workflow) { - await workflow.addUserTask(taskName); + await workflow.addTask(taskName); }); workflow2 = job.workflow; dataset2 = job.dataset; diff --git a/tests/v2/custom_env_task.spec.js b/tests/v2/custom_env_task.spec.js index 0f660268..5223ba7e 100644 --- a/tests/v2/custom_env_task.spec.js +++ b/tests/v2/custom_env_task.spec.js @@ -1,8 +1,9 @@ import { expect, test } from '@playwright/test'; -import { uploadFile, waitModalClosed, waitPageLoading } from '../utils.js'; +import { uploadFile, waitPageLoading } from '../utils.js'; import path from 'path'; import fs from 'fs'; import os from 'os'; +import { deleteTask } from './task_utils.js'; test('Custom Python env task [v2]', async ({ page }) => { await page.goto('/v2/tasks'); @@ -81,11 +82,10 @@ test('Custom Python env task [v2]', async ({ page }) => { await test.step('Check task in the table and delete it', async () => { const taskRow = page.getByRole('row', { name: randomName }); await expect(taskRow).toBeVisible(); - await taskRow.getByRole('button', { name: 'Delete' }).click(); - const modal = page.locator('.modal.show'); - await modal.waitFor(); - await modal.getByRole('button', { name: 'Confirm' }).click(); - await waitModalClosed(page); + }); + + await test.step('Delete task', async () => { + await deleteTask(page, randomName); }); await test.step('Remove temporary file and folder', async () => { diff --git a/tests/v2/export_import_workflow_custom_task.spec.js b/tests/v2/export_import_workflow_custom_task.spec.js deleted file mode 100644 index fb5829ba..00000000 --- a/tests/v2/export_import_workflow_custom_task.spec.js +++ /dev/null @@ -1,76 +0,0 @@ -import { expect, test } from './workflow_fixture.js'; -import { waitModalClosed, waitPageLoading } from '../utils.js'; -import path from 'path'; -import { createFakeTask, deleteTask } from './task_utils.js'; -import os from 'os'; -import fs from 'fs'; - -test('Export and re-import a workflow with a custom task', async ({ page, workflow }) => { - await page.waitForURL(workflow.url); - await waitPageLoading(page); - - let taskName; - await test.step('Create test task', async () => { - taskName = await createFakeTask(page, { - type: 'non_parallel' - }); - }); - - await test.step('Open workflow page', async () => { - await workflow.openWorkflowPage(); - }); - - await test.step('Add task to workflow', async () => { - await workflow.addUserTask(taskName); - }); - - let downloadedFile; - await test.step('Export workflow and verify warning message', async () => { - const downloadPromise = page.waitForEvent('download'); - await page.getByLabel('Export workflow').click(); - const download = await downloadPromise; - downloadedFile = path.join(os.tmpdir(), download.suggestedFilename()); - await download.saveAs(downloadedFile); - await expect(page.getByText('are not meant to be portable')).toBeVisible(); - }); - - await test.step('Open project page', async () => { - await page.goto(`/v2/projects/${workflow.projectId}`); - }); - - await test.step('Open "Create new workflow" modal', async () => { - const createWorkflowBtn = page.getByRole('button', { name: 'Create new workflow' }); - await createWorkflowBtn.waitFor(); - await createWorkflowBtn.click(); - const modalTitle = page.locator('.modal.show .modal-title'); - await modalTitle.waitFor(); - await expect(modalTitle).toHaveText('Create new workflow'); - }); - - await test.step('Import the workflow and verify warning message', async () => { - await page - .getByRole('textbox', { name: 'Workflow name' }) - .fill(`${workflow.workflowName}-imported`); - const fileChooserPromise = page.waitForEvent('filechooser'); - await page.getByText('Import workflow from file').click(); - const fileChooser = await fileChooserPromise; - await fileChooser.setFiles(downloadedFile); - await page.getByRole('button', { name: 'Import workflow' }).click(); - await expect(page.getByText('are not meant to be portable')).toBeVisible(); - }); - - await test.step('Cleanup', async () => { - await deleteWorkflow(); - await deleteWorkflow(); - await deleteTask(page, taskName); - fs.rmSync(downloadedFile); - }); - - async function deleteWorkflow() { - await page.getByRole('button', { name: 'Delete' }).first().click(); - const modalTitle = page.locator('.modal.show .modal-title'); - await modalTitle.waitFor(); - await page.getByRole('button', { name: 'Confirm' }).click(); - await waitModalClosed(page); - } -}); diff --git a/tests/v2/import_export_args.spec.js b/tests/v2/import_export_args.spec.js index 97d99fc9..beb2b146 100644 --- a/tests/v2/import_export_args.spec.js +++ b/tests/v2/import_export_args.spec.js @@ -78,7 +78,7 @@ test('Import/export arguments [v2]', async ({ page, workflow }) => { }); await test.step('Non parallel task without args schema', async () => { - await workflow.addUserTask(nonParallelTaskWithoutArgsSchema); + await workflow.addTask(nonParallelTaskWithoutArgsSchema); await workflow.selectTask(nonParallelTaskWithoutArgsSchema); await page.getByRole('button', { name: 'Add property' }).click(); await page.getByPlaceholder('Argument name').click(); @@ -96,7 +96,7 @@ test('Import/export arguments [v2]', async ({ page, workflow }) => { }); await test.step('Parallel task without args schema', async () => { - await workflow.addUserTask(parallelTaskWithoutArgsSchema); + await workflow.addTask(parallelTaskWithoutArgsSchema); await workflow.selectTask(parallelTaskWithoutArgsSchema); await page.getByRole('button', { name: 'Add property' }).click(); await page.getByPlaceholder('Argument name').click(); @@ -114,7 +114,7 @@ test('Import/export arguments [v2]', async ({ page, workflow }) => { }); await test.step('Compound task without args schema', async () => { - await workflow.addUserTask(compoundTaskWithoutArgsSchema); + await workflow.addTask(compoundTaskWithoutArgsSchema); await workflow.selectTask(compoundTaskWithoutArgsSchema); await page.getByRole('button', { name: 'Add property' }).first().click(); await page.getByPlaceholder('Argument name').fill('key_non_parallel'); @@ -139,7 +139,7 @@ test('Import/export arguments [v2]', async ({ page, workflow }) => { }); await test.step('Non parallel task with args schema', async () => { - await workflow.addUserTask(nonParallelTaskWithArgsSchema); + await workflow.addTask(nonParallelTaskWithArgsSchema); await workflow.selectTask(nonParallelTaskWithArgsSchema); await page.getByRole('textbox', { name: 'test_non_parallel' }).fill('value_non_parallel'); await page.getByRole('button', { name: 'Save changes' }).click(); @@ -155,7 +155,7 @@ test('Import/export arguments [v2]', async ({ page, workflow }) => { }); await test.step('Parallel task with args schema', async () => { - await workflow.addUserTask(parallelTaskWithArgsSchema); + await workflow.addTask(parallelTaskWithArgsSchema); await workflow.selectTask(parallelTaskWithArgsSchema); await page.getByRole('textbox', { name: 'test_parallel' }).fill('value_parallel'); await page.getByRole('button', { name: 'Save changes' }).click(); @@ -171,7 +171,7 @@ test('Import/export arguments [v2]', async ({ page, workflow }) => { }); await test.step('Compound task with args schema', async () => { - await workflow.addUserTask(compoundTaskWithArgsSchema); + await workflow.addTask(compoundTaskWithArgsSchema); await workflow.selectTask(compoundTaskWithArgsSchema); await page.getByRole('textbox', { name: 'test_non_parallel' }).fill('value_non_parallel'); await page.getByRole('textbox', { name: 'test_parallel' }).fill('value_parallel'); diff --git a/tests/v2/jschema.spec.js b/tests/v2/jschema.spec.js index b882a284..2414f12a 100644 --- a/tests/v2/jschema.spec.js +++ b/tests/v2/jschema.spec.js @@ -1,9 +1,9 @@ -import { waitModalClosed, waitPageLoading } from '../utils.js'; +import { waitPageLoading } from '../utils.js'; import { expect, test } from './workflow_fixture.js'; import { fileURLToPath } from 'url'; import path from 'path'; import fs from 'fs'; -import { createFakeTask } from './task_utils.js'; +import { createFakeTask, deleteTask } from './task_utils.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -33,7 +33,7 @@ test('JSON Schema validation', async ({ page, workflow }) => { await test.step('Add task to workflow and select it', async () => { await workflow.openWorkflowPage(); - await workflow.addUserTask(randomTaskName); + await workflow.addTask(randomTaskName); await workflow.selectTask(randomTaskName); }); @@ -232,33 +232,6 @@ test('JSON Schema validation', async ({ page, workflow }) => { }); await test.step('Delete task', async () => { - await page.goto('/v2/tasks'); - await waitPageLoading(page); - const taskRow = /** @type {import('@playwright/test').Locator} */ ( - await getTaskRow(page, randomTaskName) - ); - const deleteBtn = taskRow.getByRole('button', { name: 'Delete' }); - await deleteBtn.click(); - const modal = page.locator('.modal.show'); - await modal.waitFor(); - await modal.getByRole('button', { name: 'Confirm' }).click(); - await waitModalClosed(page); - expect(await getTaskRow(page, randomTaskName)).toBeNull(); + await deleteTask(page, randomTaskName); }); }); - -/** - * @param {import('@playwright/test').Page} page - * @param {string} taskName - * @returns {Promise} - */ -async function getTaskRow(page, taskName) { - const rows = await page.getByRole('table').last().getByRole('row').all(); - for (const row of rows) { - const rowText = await row.innerText(); - if (rowText.includes(taskName)) { - return row; - } - } - return null; -} diff --git a/tests/v2/run_mock_tasks.spec.js b/tests/v2/run_mock_tasks.spec.js index 573226ca..ae4cd959 100644 --- a/tests/v2/run_mock_tasks.spec.js +++ b/tests/v2/run_mock_tasks.spec.js @@ -42,7 +42,7 @@ test('Run mock tasks [v2]', async ({ page, workflow }) => { }); await test.step('Add and select create_ome_zarr_compound', async () => { - await workflow.addCollectedTask('create_ome_zarr_compound'); + await workflow.addTask('create_ome_zarr_compound'); await workflow.selectTask('create_ome_zarr_compound'); }); @@ -123,7 +123,7 @@ test('Run mock tasks [v2]', async ({ page, workflow }) => { }); await test.step('Add and select generic_task', async () => { - await workflow.addCollectedTask('generic_task'); + await workflow.addTask('generic_task'); await workflow.selectTask('generic_task'); }); diff --git a/tests/v2/slurm_account.spec.js b/tests/v2/slurm_account.spec.js index 7b0b7384..3ce7e749 100644 --- a/tests/v2/slurm_account.spec.js +++ b/tests/v2/slurm_account.spec.js @@ -43,7 +43,7 @@ test('Add SLURM accounts for the admin and execute workflow using a specific acc await test.step('Add task to workflow', async () => { await page.goto(workflow.url); await waitPageLoading(page); - await workflow.addCollectedTask('generic_task'); + await workflow.addTask('generic_task'); await workflow.selectTask('generic_task'); }); diff --git a/tests/v2/stop_workflow.spec.js b/tests/v2/stop_workflow.spec.js index 3043c771..d80b1856 100644 --- a/tests/v2/stop_workflow.spec.js +++ b/tests/v2/stop_workflow.spec.js @@ -17,7 +17,7 @@ test('Stop workflow [v2]', async ({ page, workflow }) => { }); await test.step('Add and select generic_task', async () => { - await workflow.addCollectedTask('generic_task'); + await workflow.addTask('generic_task'); await workflow.selectTask('generic_task'); }); diff --git a/tests/v2/task_version_update.spec.js b/tests/v2/task_version_update.spec.js index 08d97175..1a89ff1e 100644 --- a/tests/v2/task_version_update.spec.js +++ b/tests/v2/task_version_update.spec.js @@ -114,7 +114,7 @@ test('Task version update [v2]', async ({ page, workflow }) => { }); await test.step('Add non parallel task v1', async () => { - await workflow.addUserTask(nonParallelTask, 'v0.0.1'); + await workflow.addTask(nonParallelTask, '0.0.1'); await workflow.selectTask(nonParallelTask); }); @@ -160,7 +160,7 @@ test('Task version update [v2]', async ({ page, workflow }) => { }); await test.step('Add parallel task v1', async () => { - await workflow.addUserTask(parallelTask, 'v0.0.1'); + await workflow.addTask(parallelTask, '0.0.1'); await workflow.selectTask(parallelTask); }); @@ -206,7 +206,7 @@ test('Task version update [v2]', async ({ page, workflow }) => { }); await test.step('Add compound task v1', async () => { - await workflow.addUserTask(compoundTask, 'v0.0.1'); + await workflow.addTask(compoundTask, '0.0.1'); await workflow.selectTask(compoundTask); }); @@ -233,11 +233,11 @@ test('Task version update [v2]', async ({ page, workflow }) => { }); await test.step('Cleanup test tasks', async () => { - await deleteTask(page, nonParallelTask); // v0.0.1 - await deleteTask(page, nonParallelTask); // v0.0.2 - await deleteTask(page, parallelTask); // v0.0.1 - await deleteTask(page, parallelTask); // v0.0.2 - await deleteTask(page, compoundTask); // v0.0.1 - await deleteTask(page, compoundTask); // v0.0.2 + await deleteTask(page, nonParallelTask); // 0.0.1 + await deleteTask(page, nonParallelTask); // 0.0.2 + await deleteTask(page, parallelTask); // 0.0.1 + await deleteTask(page, parallelTask); // 0.0.2 + await deleteTask(page, compoundTask); // 0.0.1 + await deleteTask(page, compoundTask); // 0.0.2 }); }); diff --git a/tests/v2/tasks_admin.spec.js b/tests/v2/tasks_admin.spec.js index fa422aab..13bf1386 100644 --- a/tests/v2/tasks_admin.spec.js +++ b/tests/v2/tasks_admin.spec.js @@ -25,7 +25,7 @@ test('Tasks admin page [v2]', async ({ page, workflow }) => { }); await test.step('Add task to workflow', async () => { - await workflow.addUserTask(taskName); + await workflow.addTask(taskName); }); await test.step('Open tasks admin page', async () => { diff --git a/tests/v2/workflow_fixture.js b/tests/v2/workflow_fixture.js index 9a0079ef..e0d6c0e5 100644 --- a/tests/v2/workflow_fixture.js +++ b/tests/v2/workflow_fixture.js @@ -1,5 +1,5 @@ import { test as baseTest, mergeTests } from '@playwright/test'; -import { selectSlimSelect, waitModalClosed, waitPageLoading } from '../utils.js'; +import { waitModalClosed, waitPageLoading } from '../utils.js'; import { PageWithProject } from './project_fixture.js'; export class PageWithWorkflow extends PageWithProject { @@ -51,7 +51,7 @@ export class PageWithWorkflow extends PageWithProject { } async addFakeTask() { - await this.addUserTask('Fake Task'); + await this.addTask('Fake Task'); } /** @@ -95,47 +95,36 @@ export class PageWithWorkflow extends PageWithProject { /** * @param {string} taskName * @param {string|null=} taskVersion - * @returns {Promise} */ - async addCollectedTask(taskName, taskVersion = null) { + async addTask(taskName, taskVersion = null) { await this.page.getByRole('button', { name: 'Add task to workflow' }).click(); const modal = this.page.locator('.modal.show'); await modal.waitFor(); await expect(modal.locator('.spinner-border')).toHaveCount(0); - await this.page.getByText('Common tasks').click(); - await this.addTask(modal, taskName, taskVersion); - } - - /** - * @param {string} taskName - * @param {string|null=} taskVersion - * @returns {Promise} - */ - async addUserTask(taskName, taskVersion = null) { - await this.page.getByRole('button', { name: 'Add task to workflow' }).click(); - const modal = this.page.locator('.modal.show'); - await modal.waitFor(); - await expect(modal.locator('.spinner-border')).toHaveCount(0); - await this.page.getByText('User tasks').click(); - await this.addTask(modal, taskName, taskVersion); + const row = await this.getTaskRow(modal, taskName); + await row.scrollIntoViewIfNeeded(); + if (taskVersion) { + await row + .getByRole('combobox', { name: `Version for task ${taskName}` }) + .selectOption(taskVersion); + } + await row.getByRole('button', { name: 'Add task' }).click(); + await waitModalClosed(this.page); } /** * @param {import('@playwright/test').Locator} modal * @param {string} taskName - * @param {string|null} taskVersion */ - async addTask(modal, taskName, taskVersion) { - const selector = modal.getByRole('combobox').first(); - await selectSlimSelect(this.page, selector, taskName); - await this.page.locator('#taskId').waitFor(); - if (taskVersion) { - await this.page - .getByRole('combobox', { name: 'Select task version' }) - .selectOption(taskVersion); + async getTaskRow(modal, taskName) { + const rows = await modal.getByRole('row', { name: 'Add task' }).all(); + for (const row of rows) { + const cellContent = (await row.getByRole('cell').first().innerText()).trim(); + if (cellContent === taskName) { + return row; + } } - await this.page.getByRole('button', { name: 'Insert' }).click(); - await waitModalClosed(this.page); + throw new Error(`Unable to find row for task ${taskName}`); } async removeCurrentTask() { diff --git a/tests/v2/workflow_task_input_filters.spec.js b/tests/v2/workflow_task_input_filters.spec.js index b8d5d9db..2b4e4700 100644 --- a/tests/v2/workflow_task_input_filters.spec.js +++ b/tests/v2/workflow_task_input_filters.spec.js @@ -67,8 +67,8 @@ test('Workflow task input filters [v2]', async ({ page, workflow }) => { }); await test.step('Add tasks to workflow', async () => { - await workflow.addUserTask(taskName1); - await workflow.addUserTask(taskName2); + await workflow.addTask(taskName1); + await workflow.addTask(taskName2); await workflow.selectTask(taskName1); }); diff --git a/tests/v2/workflow_task_meta_properties.spec.js b/tests/v2/workflow_task_meta_properties.spec.js index cea33774..0fde1484 100644 --- a/tests/v2/workflow_task_meta_properties.spec.js +++ b/tests/v2/workflow_task_meta_properties.spec.js @@ -16,7 +16,7 @@ test('Workflow task meta properties [v2]', async ({ page, workflow }) => { }); await test.step('Add tasks to workflow', async () => { - await workflow.addUserTask(taskName); + await workflow.addTask(taskName); await workflow.selectTask(taskName); }); diff --git a/tests/v2/workflow_task_without_schema.spec.js b/tests/v2/workflow_task_without_schema.spec.js index d09386f8..6d462337 100644 --- a/tests/v2/workflow_task_without_schema.spec.js +++ b/tests/v2/workflow_task_without_schema.spec.js @@ -16,7 +16,7 @@ test('Workflow task without JSON Schema [v2]', async ({ page, workflow }) => { }); await test.step('Add tasks to workflow', async () => { - await workflow.addUserTask(taskName); + await workflow.addTask(taskName); await workflow.selectTask(taskName); }); From 614be19a08ad2ea210365b2552b48abf51439b55 Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Thu, 10 Oct 2024 12:28:22 +0200 Subject: [PATCH 07/16] Updated CHANGELOG.md --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2cfb638..e39ab7d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ *Note: Numbers like (\#123) point to closed Pull Requests on the fractal-web repository.* +# Unreleased + +* Alignment with fractal-server 2.7 (\#583): + * Removed owner from v2 tasks; + * Displayed workflow task warning message; + * Implemented selection of group in task creation; + * First implementation of new workflow task modal; + * Used new group_ids_names field; + # 1.8.0 * Drop Node 16 support (\#574); From acbbfa4cfd048542b49ed606991c900a399454c0 Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Thu, 10 Oct 2024 15:04:48 +0200 Subject: [PATCH 08/16] Fixes to e2e tests --- tests/v2/add_single_task.spec.js | 2 +- tests/v2/tasks_admin.spec.js | 2 +- tests/v2/workflow_fixture.js | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/v2/add_single_task.spec.js b/tests/v2/add_single_task.spec.js index 8b44788a..3d27fec1 100644 --- a/tests/v2/add_single_task.spec.js +++ b/tests/v2/add_single_task.spec.js @@ -356,7 +356,7 @@ async function getTaskDataCompound(page, items) { */ async function getCreatedTaskRow(page, taskName) { const table = page.getByRole('table').last(); - const rows = await table.getByRole('row').all(); + const rows = await table.getByRole('row', { name: taskName }).all(); let taskRow; for (const row of rows) { const text = await row.getByRole('cell').first().innerText(); diff --git a/tests/v2/tasks_admin.spec.js b/tests/v2/tasks_admin.spec.js index 13bf1386..067d972f 100644 --- a/tests/v2/tasks_admin.spec.js +++ b/tests/v2/tasks_admin.spec.js @@ -119,7 +119,7 @@ test('Tasks admin page [v2]', async ({ page, workflow }) => { */ async function searchTasks(page) { // Increasing the results limit since during the tests many tasks may have been created - await page.getByRole('spinbutton', { name: 'Max number of results' }).fill('200'); + await page.getByRole('spinbutton', { name: 'Max number of results' }).fill('500'); await page.getByRole('button', { name: 'Search tasks' }).click(); } diff --git a/tests/v2/workflow_fixture.js b/tests/v2/workflow_fixture.js index e0d6c0e5..aa621722 100644 --- a/tests/v2/workflow_fixture.js +++ b/tests/v2/workflow_fixture.js @@ -117,7 +117,10 @@ export class PageWithWorkflow extends PageWithProject { * @param {string} taskName */ async getTaskRow(modal, taskName) { - const rows = await modal.getByRole('row', { name: 'Add task' }).all(); + const rows = await modal + .getByRole('row', { name: taskName }) + .filter({ hasText: /Add task/ }) + .all(); for (const row of rows) { const cellContent = (await row.getByRole('cell').first().innerText()).trim(); if (cellContent === taskName) { From 2ad03031379d408eb68891ee15c56b7c7c736457 Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Thu, 10 Oct 2024 15:53:10 +0200 Subject: [PATCH 09/16] Fixed issue with task version sorting --- __tests__/v2/AddWorkflowTaskModal.test.js | 166 ++++++++++++++++++ .../v2/workflow/AddWorkflowTaskModal.svelte | 42 +++-- src/lib/types-v2.d.ts | 2 +- 3 files changed, 192 insertions(+), 18 deletions(-) create mode 100644 __tests__/v2/AddWorkflowTaskModal.test.js diff --git a/__tests__/v2/AddWorkflowTaskModal.test.js b/__tests__/v2/AddWorkflowTaskModal.test.js new file mode 100644 index 00000000..91ce39b4 --- /dev/null +++ b/__tests__/v2/AddWorkflowTaskModal.test.js @@ -0,0 +1,166 @@ +import { describe, it, beforeEach, expect, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/svelte'; + +// Mocking fetch +global.fetch = vi.fn(); + +// Mocking bootstrap.Modal +class MockModal { + constructor() { + this.show = vi.fn(); + this.hide = vi.fn(); + } +} +MockModal.getInstance = vi.fn(); + +global.window.bootstrap = { + Modal: MockModal +}; + +const mockedWorkflow = { + id: 20, + name: 'test', + project_id: 24, + task_list: [] +}; + +// The component to be tested must be imported after the mock setup +import AddWorkflowTaskModal from '../../src/lib/components/v2/workflow/AddWorkflowTaskModal.svelte'; + +describe('AddWorkflowTaskModal', () => { + beforeEach(() => { + fetch.mockClear(); + }); + + it('Display task with loosely valid version', async () => { + fetch.mockResolvedValue({ + ok: true, + status: 200, + json: async () => [ + { + id: 1, + task_list: [ + { + id: 1, + name: 'test_task', + taskgroupv2_id: 1, + category: null, + modality: null, + authors: null, + tags: [] + } + ], + user_id: 1, + user_group_id: 1, + pkg_name: 'test_task_package', + version: '0.0.1', + active: true + }, + { + id: 2, + task_list: [ + { + id: 2, + name: 'test_task', + taskgroupv2_id: 2, + category: null, + modality: null, + authors: null, + tags: [] + } + ], + user_id: 1, + user_group_id: 1, + pkg_name: 'test_task_package', + version: '0.7.10.dev4+g4c8ebe3', + active: true + } + ] + }); + + const result = render(AddWorkflowTaskModal, { + props: { + workflow: mockedWorkflow, + onWorkflowTaskAdded: vi.fn() + } + }); + + result.component.show(); + + await waitFor(() => screen.getByText(/Add new workflow task/)); + expect(fetch).toHaveBeenCalledWith('/api/v2/task-group', expect.anything()); + await waitFor(() => screen.getAllByText(/test_task/)); + + const dropdown = screen.getByRole('combobox'); + expect(dropdown.options.length).eq(2); + expect(dropdown.options[0].text).eq('0.7.10.dev4+g4c8ebe3'); + expect(dropdown.options[1].text).eq('0.0.1'); + expect(dropdown.value).eq('0.7.10.dev4+g4c8ebe3'); + }); + + it('Display task with invalid version', async () => { + fetch.mockResolvedValue({ + ok: true, + status: 200, + json: async () => [ + { + id: 1, + task_list: [ + { + id: 1, + name: 'test_task', + taskgroupv2_id: 1, + category: null, + modality: null, + authors: null, + tags: [] + } + ], + user_id: 1, + user_group_id: 1, + pkg_name: 'test_task_package', + version: '0.0.1', + active: true + }, + { + id: 2, + task_list: [ + { + id: 2, + name: 'test_task', + taskgroupv2_id: 2, + category: null, + modality: null, + authors: null, + tags: [] + } + ], + user_id: 1, + user_group_id: 1, + pkg_name: 'test_task_package', + version: 'INVALID', + active: true + } + ] + }); + + const result = render(AddWorkflowTaskModal, { + props: { + workflow: mockedWorkflow, + onWorkflowTaskAdded: vi.fn() + } + }); + + result.component.show(); + + await waitFor(() => screen.getByText(/Add new workflow task/)); + expect(fetch).toHaveBeenCalledWith('/api/v2/task-group', expect.anything()); + await waitFor(() => screen.getAllByText(/test_task/)); + + const dropdown = screen.getByRole('combobox'); + expect(dropdown.options.length).eq(2); + expect(dropdown.options[0].text).eq('0.0.1'); + expect(dropdown.options[1].text).eq('INVALID'); + expect(dropdown.value).eq('0.0.1'); + }); +}); diff --git a/src/lib/components/v2/workflow/AddWorkflowTaskModal.svelte b/src/lib/components/v2/workflow/AddWorkflowTaskModal.svelte index c914fad3..5c854fdd 100644 --- a/src/lib/components/v2/workflow/AddWorkflowTaskModal.svelte +++ b/src/lib/components/v2/workflow/AddWorkflowTaskModal.svelte @@ -1,7 +1,7 @@ {groupByLabels[groupBy]} + Metadata Version @@ -230,14 +254,16 @@ {#each row.tasks as task} - {task.taskVersions[task.selectedVersion]['name']} + {task.taskVersions[task.selectedVersion].task_name} + + {getMetadataCell(task.taskVersions[task.selectedVersion])} + {#if Object.keys(task.taskVersions).length > 1} {:else} - {task.taskVersions[task.selectedVersion]['version']} + {task.taskVersions[task.selectedVersion].version} {/if} @@ -253,7 +279,7 @@ class="btn btn-primary" disabled={addingTask} on:click={() => - addTaskToWorkflow(task.taskVersions[task.selectedVersion]['id'])} + addTaskToWorkflow(task.taskVersions[task.selectedVersion].task_id)} > Add task @@ -297,4 +323,8 @@ padding-top: 18px; padding-bottom: 12px; } + .metadata-cell { + font-size: 85%; + max-width: 150px; + } diff --git a/src/lib/types-v2.d.ts b/src/lib/types-v2.d.ts index 3fc77240..034ea1ad 100644 --- a/src/lib/types-v2.d.ts +++ b/src/lib/types-v2.d.ts @@ -62,6 +62,10 @@ export type TaskV2 = { docs_info: string meta_non_parallel: object meta_parallel: object + category: string | null + modality: string | null + authors: string | null + tags: string[] } export type ApplyWorkflowV2 = { @@ -164,12 +168,23 @@ export type TaskGroupV2 = { timestamp_created: string } -export type TasksTableRow = { +export type TasksTableRowGroup = { groupTitle: string tasks: Array<{ selectedVersion: string taskVersions: { - [version: string]: { [key: string]: string } + [version: string]: TasksTableRow } }> } + +export type TasksTableRow = { + pkg_name: string + task_id: number + task_name: string + version: string + category: string | null + modality: string | null + authors: string | null + tags: string[] +} From 459480dd3672a29ce9c8c17e0fd17121e8721ee3 Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Fri, 11 Oct 2024 11:28:57 +0200 Subject: [PATCH 13/16] Renamed source to label in custom python env task creation --- .../components/v2/tasks/CustomEnvTask.svelte | 25 ++++++++----------- src/lib/types-v2.d.ts | 1 - tests/v2/custom_env_task.spec.js | 4 +-- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/lib/components/v2/tasks/CustomEnvTask.svelte b/src/lib/components/v2/tasks/CustomEnvTask.svelte index 2cddeae4..45f3f1c1 100644 --- a/src/lib/components/v2/tasks/CustomEnvTask.svelte +++ b/src/lib/components/v2/tasks/CustomEnvTask.svelte @@ -13,7 +13,7 @@ export let user; let python_interpreter = ''; - let source = ''; + let label = ''; let version = ''; let package_name = ''; let package_root = ''; @@ -24,7 +24,7 @@ const formErrorHandler = new FormErrorHandler('errorAlert-customEnvTask', [ 'python_interpreter', - 'source', + 'label', 'version', 'package_name', 'package_root' @@ -88,7 +88,7 @@ try { const body = { python_interpreter, - source, + label, version, package_name, package_root, @@ -115,7 +115,7 @@ addNewTasks(result); successMessage = 'Tasks collected successfully'; python_interpreter = ''; - source = ''; + label = ''; version = ''; package_name = ''; package_root = ''; @@ -184,23 +184,20 @@
- +
- {$validationErrors['source']} -
-
- A common label identifying this package (e.g. if you set this to "mypackage" - then tasks will have source like "myusername:mypackage:task_module_name") + {$validationErrors['label']}
+
A common label identifying this package.
diff --git a/src/lib/types-v2.d.ts b/src/lib/types-v2.d.ts index 034ea1ad..7d64d625 100644 --- a/src/lib/types-v2.d.ts +++ b/src/lib/types-v2.d.ts @@ -131,7 +131,6 @@ type TaskV2Minimal = { type: string command_non_parallel: string | null command_parallel: string | null - source: string source: string | null version: string | null } diff --git a/tests/v2/custom_env_task.spec.js b/tests/v2/custom_env_task.spec.js index 5223ba7e..efcd48df 100644 --- a/tests/v2/custom_env_task.spec.js +++ b/tests/v2/custom_env_task.spec.js @@ -33,7 +33,7 @@ test('Custom Python env task [v2]', async ({ page }) => { await test.step('Test "Python interpreter path must be absolute" error', async () => { await page.getByRole('textbox', { name: 'Python Intepreter' }).fill('foo'); - await page.getByRole('textbox', { name: 'Source' }).fill(`${randomName}-source`); + await page.getByRole('textbox', { name: 'Label' }).fill(`${randomName}-label`); tmpManifest = await uploadFile( page, 'Manifest', @@ -72,7 +72,7 @@ test('Custom Python env task [v2]', async ({ page }) => { await test.step('Verify that fields have been cleaned', async () => { await expect(page.getByRole('textbox', { name: 'Python Intepreter' })).toHaveValue(''); - await expect(page.getByRole('textbox', { name: 'Source' })).toHaveValue(''); + await expect(page.getByRole('textbox', { name: 'Label' })).toHaveValue(''); await expect(page.getByLabel('Manifest')).toHaveValue(''); await expect(page.getByRole('textbox', { name: 'Package Name' })).toHaveValue(''); await expect(page.getByRole('textbox', { name: 'Version' })).toHaveValue(''); From 51fad24a3daff7debe0f4b60f9f49f107dbbe314 Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Fri, 11 Oct 2024 11:45:24 +0200 Subject: [PATCH 14/16] Removed kind from admin tasks page --- src/routes/v2/admin/tasks/+page.svelte | 23 ---------------------- tests/v2/tasks_admin.spec.js | 27 -------------------------- 2 files changed, 50 deletions(-) diff --git a/src/routes/v2/admin/tasks/+page.svelte b/src/routes/v2/admin/tasks/+page.svelte index 5775bb4b..ecbe50c0 100644 --- a/src/routes/v2/admin/tasks/+page.svelte +++ b/src/routes/v2/admin/tasks/+page.svelte @@ -4,7 +4,6 @@ import { PropertyDescription } from 'fractal-jschema'; let name = ''; - let kind = ''; let id = ''; let source = ''; let version = ''; @@ -39,9 +38,6 @@ if (id) { url.searchParams.append('id', id); } - if (kind) { - url.searchParams.append('kind', kind); - } if (source) { url.searchParams.append('source', source); } @@ -72,7 +68,6 @@ searchErrorAlert.hide(); } name = ''; - kind = ''; id = ''; source = ''; version = ''; @@ -145,24 +140,6 @@
-
-
-
- - -
-
- -
-
-
diff --git a/tests/v2/tasks_admin.spec.js b/tests/v2/tasks_admin.spec.js index 067d972f..b5b82b3e 100644 --- a/tests/v2/tasks_admin.spec.js +++ b/tests/v2/tasks_admin.spec.js @@ -60,33 +60,6 @@ test('Tasks admin page [v2]', async ({ page, workflow }) => { await reset(page); }); - await test.step('Search tasks by common kind', async () => { - await page.getByRole('combobox', { name: 'Kind' }).selectOption('Common'); - await searchTasks(page); - await expect(page.getByRole('table')).toBeVisible(); - let count = await page.getByRole('row').count(); - expect(count).toBeLessThan(total); - await page.getByRole('cell', { name: 'cellpose_segmentation', exact: true }).waitFor(); - await reset(page); - }); - - await test.step('Search tasks by users kind', async () => { - await page.getByRole('combobox', { name: 'Kind' }).selectOption('Users'); - await searchTasks(page); - await expect(page.getByRole('table')).toBeVisible(); - let count = await page.getByRole('row').count(); - expect(count).toBeLessThan(total); - await page.getByText(taskName).waitFor(); - await reset(page); - }); - - await test.step('Search tasks by source', async () => { - await page.getByRole('textbox', { name: 'Source' }).fill('cellpose_segmentation'); - await searchTasks(page); - await expect(page.getByRole('row')).toHaveCount(2); - await reset(page); - }); - await test.step('Search tasks by version', async () => { await page.getByRole('textbox', { name: 'Version' }).fill(randomTaskVersion); await searchTasks(page); From fbf2649e2a2ea54ad809b50b45cdd4e7acf5e4fb Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Fri, 11 Oct 2024 12:18:19 +0200 Subject: [PATCH 15/16] Updated tests --- playwright.config.js | 2 +- .../v2/admin/jobs/healthcheck/+page.svelte | 2 +- tests/data/workflow_to_import.json | 24 +++++++++---------- tests/v2/add_single_task.spec.js | 18 +++----------- 4 files changed, 17 insertions(+), 29 deletions(-) diff --git a/playwright.config.js b/playwright.config.js index 5be49395..bf2f071c 100644 --- a/playwright.config.js +++ b/playwright.config.js @@ -107,7 +107,7 @@ export default defineConfig({ webServer: [ { - command: './tests/start-test-server.sh 2.7.0a3', + command: './tests/start-test-server.sh 2.7.0a4', port: 8000, waitForPort: true, stdout: 'pipe', diff --git a/src/routes/v2/admin/jobs/healthcheck/+page.svelte b/src/routes/v2/admin/jobs/healthcheck/+page.svelte index 4a1ff7a2..4a879320 100644 --- a/src/routes/v2/admin/jobs/healthcheck/+page.svelte +++ b/src/routes/v2/admin/jobs/healthcheck/+page.svelte @@ -140,7 +140,7 @@ if (!response.ok) { throw new AlertError(result); } - const tasks = result.filter((t) => t.source.endsWith(':job_submission_health_check')); + const tasks = result.filter((t) => t.name === 'job_submission_health_check'); return tasks.length > 0 ? tasks[0].id : undefined; } diff --git a/tests/data/workflow_to_import.json b/tests/data/workflow_to_import.json index 438e668f..7e3222e0 100644 --- a/tests/data/workflow_to_import.json +++ b/tests/data/workflow_to_import.json @@ -2,11 +2,11 @@ "name": "My Workflow", "task_list": [ { - "meta": { + "meta_non_parallel": { "cpus_per_task": 1, "mem": 4000 }, - "args": { + "args_non_parallel": { "image_extension": "png", "num_levels": 5, "coarsening_xy": 2, @@ -45,12 +45,12 @@ } }, { - "meta": { + "meta_non_parallel": { "cpus_per_task": 1, "mem": 4000, "parallelization_level": "image" }, - "args": { + "args_non_parallel": { "delete_input": false }, "task": { @@ -58,11 +58,11 @@ } }, { - "meta": { + "meta_non_parallel": { "cpus_per_task": 1, "mem": 1000 }, - "args": { + "args_non_parallel": { "project_to_2D": true, "suffix": "mip" }, @@ -71,24 +71,24 @@ } }, { - "meta": { + "meta_non_parallel": { "cpus_per_task": 1, "mem": 4000, "parallelization_level": "image" }, - "args": null, + "args_non_parallel": null, "task": { "source": "pip_remote:fractal_tasks_core:0.10.1:fractal-tasks::maximum_intensity_projection" } }, { - "meta": { + "meta_non_parallel": { "cpus_per_task": 1, "mem": 4000, "needs_gpu": true, "parallelization_level": "image" }, - "args": { + "args_non_parallel": { "input_ROI_table": "well_ROI_table", "use_masks": true, "relabeling": true, @@ -111,12 +111,12 @@ } }, { - "meta": { + "meta_non_parallel": { "cpus_per_task": 1, "mem": 4000, "parallelization_level": "image" }, - "args": { + "args_non_parallel": { "input_ROI_table": "well_ROI_table", "level": 0, "relabeling": true, diff --git a/tests/v2/add_single_task.spec.js b/tests/v2/add_single_task.spec.js index 3d27fec1..f6c6f559 100644 --- a/tests/v2/add_single_task.spec.js +++ b/tests/v2/add_single_task.spec.js @@ -47,7 +47,6 @@ test('Add single tasks [v2]', async ({ page }) => { const task = await getCreatedTaskModalData(page, randomTaskName1, 'non_parallel'); expect(task.name).toEqual(randomTaskName1); expect(task.command_non_parallel).toEqual('/tmp/test'); - expect(task.source).toEqual(`admin:${randomTaskName1}-source`); expect(task.input_types).toContain('input_type'); expect(task.output_types).toContain('output_type'); expect(task.args_schema_version).toEqual('pydantic_v2'); @@ -80,7 +79,6 @@ test('Add single tasks [v2]', async ({ page }) => { const task = await getCreatedTaskModalData(page, randomTaskName2, 'parallel'); expect(task.name).toEqual(randomTaskName2); expect(task.command_parallel).toEqual('/tmp/test'); - expect(task.source).toEqual(`admin:${randomTaskName2}-source`); expect(task.input_types).toContain('input_type'); expect(task.output_types).toContain('output_type'); expect(task.args_schema_version).toEqual('pydantic_v2'); @@ -117,7 +115,6 @@ test('Add single tasks [v2]', async ({ page }) => { expect(task.name).toEqual(randomTaskName3); expect(task.command_non_parallel).toEqual('/tmp/test-np'); expect(task.command_parallel).toEqual('/tmp/test-p'); - expect(task.source).toEqual(`admin:${randomTaskName3}-source`); expect(task.input_types).toContain('input_type'); expect(task.output_types).toContain('output_type'); expect(task.args_schema_version).toEqual('pydantic_v2'); @@ -127,21 +124,13 @@ test('Add single tasks [v2]', async ({ page }) => { const randomTaskName4 = Math.random().toString(36).substring(7); - await test.step('Attempt to create task reusing task source', async () => { - await page.getByRole('textbox', { name: 'Task name' }).fill(randomTaskName4); - await page.getByRole('textbox', { name: 'Command non parallel' }).fill('/tmp/test-np'); - await page.getByRole('textbox', { name: 'Command parallel' }).fill('/tmp/test-p'); - await page.getByRole('textbox', { name: 'Source' }).fill(`${randomTaskName1}-source`); - await createBtn.click(); - await page - .getByText(`Source 'admin:${randomTaskName1}-source' already used by some TaskV2`) - .waitFor(); - }); - const addInputTypeBtn = page.getByRole('button', { name: 'Add input type' }); const addOutputTypeBtn = page.getByRole('button', { name: 'Add output type' }); await test.step('Attempt to create task with types with empty keys', async () => { + await page.getByRole('textbox', { name: 'Task name' }).fill(randomTaskName4); + await page.getByRole('textbox', { name: 'Command non parallel' }).fill('/tmp/test-np'); + await page.getByRole('textbox', { name: 'Command parallel' }).fill('/tmp/test-p'); await page.getByRole('textbox', { name: 'Source' }).fill(`${randomTaskName4}-source`); await addInputTypeBtn.click(); await addOutputTypeBtn.click(); @@ -218,7 +207,6 @@ test('Add single tasks [v2]', async ({ page }) => { expect(task.name).toEqual(randomTaskName4); expect(task.command_non_parallel).toEqual('/tmp/test-np'); expect(task.command_parallel).toEqual('/tmp/test-p'); - expect(task.source).toEqual(`admin:${randomTaskName4}-source`); expect(task.args_schema_version).toEqual('pydantic_v1'); }); From 7a9a82724e3d4de084422b78ff13f1ef6e3fa206 Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Fri, 11 Oct 2024 12:19:39 +0200 Subject: [PATCH 16/16] Updated CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e39ab7d8..32b77c2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ * Implemented selection of group in task creation; * First implementation of new workflow task modal; * Used new group_ids_names field; + * Changed "new version available" icon; + * Renamed source to label in custom python env task creation; + * Removed kind from admin tasks page; # 1.8.0