Skip to content

Commit 8de20fe

Browse files
zonia3000zonia3000
authored
Merge pull request #364 from fractal-analytics-platform/tasks-improvements
Tasks collection and creation improvements
2 parents f56aac5 + cb8ec39 commit 8de20fe

18 files changed

+1753
-299
lines changed

‎.prettierignore‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,6 @@ package-lock.json
1313
yarn.lock
1414

1515
# Ignore local files
16-
/local
16+
/local
17+
18+
tests/data/broken.json

‎CHANGELOG.md‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
*Note: Numbers like (\#123) point to closed Pull Requests on the fractal-web repository.*
22

3-
# Unreleased
4-
3+
# 0.7.1
4+
5+
* Auto-refresh of tasks table when a tasks collection completes successfully (\#364).
6+
* Auto-refresh of tasks collection status (\#364).
7+
* Made tasks list more compact by hiding the older versions (\#364).
8+
* Added the following fields on single task creation:
9+
* args schema file upload (\#364).
10+
* meta file upload (\#364).
11+
* docs info and docs link (\#364).
12+
* Added editing of pinned package versions on tasks collection (\#364).
513
* Supported editing of cache_dir from user profile page (\#365).
614
* Added experimental workflow page with job monitoring (\#363).
715

‎__tests__/TaskCollection.test.js‎

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest';
2+
import { fireEvent, render, screen, act } from '@testing-library/svelte';
3+
4+
// Mocking fetch
5+
global.fetch = vi.fn();
6+
7+
function createFetchResponse(data) {
8+
return {
9+
ok: true,
10+
json: () => new Promise((resolve) => resolve(data))
11+
};
12+
}
13+
14+
// Mocking public variables
15+
vi.mock('$env/dynamic/public', () => {
16+
return { env: {} };
17+
});
18+
19+
// The component to be tested must be imported after the mock setup
20+
import TaskCollection from '../src/lib/components/tasks/TaskCollection.svelte';
21+
22+
describe('TaskCollection', () => {
23+
it('collect tasks with pinned package versions', async () => {
24+
render(TaskCollection);
25+
26+
const addPpvBtn = screen.getByRole('button', { name: 'Add pinned package version' });
27+
28+
expect(screen.getAllByRole('textbox').length).eq(4);
29+
30+
// add ppv
31+
await fireEvent.click(addPpvBtn);
32+
expect(screen.getAllByRole('textbox').length).eq(6);
33+
await fireEvent.click(addPpvBtn);
34+
await fireEvent.click(addPpvBtn);
35+
expect(screen.getAllByRole('textbox').length).eq(10);
36+
37+
// remove ppv
38+
const removePpvBtn = screen.getAllByRole('button', {
39+
name: 'Remove pinned package version'
40+
})[1];
41+
await fireEvent.click(removePpvBtn);
42+
expect(screen.getAllByRole('textbox').length).eq(8);
43+
44+
const [key1, key2] = screen.getAllByRole('textbox', { name: 'Name' });
45+
const [value1, value2] = screen.getAllByRole('textbox', { name: 'Version' });
46+
47+
await fireEvent.input(key1, { target: { value: 'package1' } });
48+
await fireEvent.input(value1, { target: { value: '1.2.3' } });
49+
await fireEvent.input(key2, { target: { value: 'package2' } });
50+
await fireEvent.input(value2, { target: { value: '0.0.8' } });
51+
52+
const packageInput = screen.getByRole('textbox', { name: 'Package' });
53+
await fireEvent.input(packageInput, { target: { value: 'main-package' } });
54+
55+
fetch.mockResolvedValue({
56+
ok: true,
57+
status: 201,
58+
json: () =>
59+
new Promise((resolve) =>
60+
resolve({
61+
data: {
62+
status: 'pending'
63+
}
64+
})
65+
)
66+
});
67+
68+
const collectBtn = screen.getByRole('button', { name: 'Collect' });
69+
await fireEvent.click(collectBtn);
70+
71+
expect(fetch).toHaveBeenCalledWith(
72+
'/api/v1/task/collect/pip',
73+
expect.objectContaining({
74+
body: JSON.stringify({
75+
package: 'main-package',
76+
pinned_package_versions: {
77+
package1: '1.2.3',
78+
package2: '0.0.8'
79+
}
80+
})
81+
})
82+
);
83+
84+
await new Promise(setTimeout);
85+
expect(screen.getAllByRole('textbox').length).eq(4);
86+
});
87+
88+
beforeEach(() => {
89+
vi.useFakeTimers({ shouldAdvanceTime: true });
90+
});
91+
92+
afterEach(() => {
93+
vi.runOnlyPendingTimers();
94+
vi.useRealTimers();
95+
});
96+
97+
it('Update tasks collection in background', async () => {
98+
let reloaded = false;
99+
async function reloadTaskList() {
100+
reloaded = true;
101+
}
102+
103+
const coll1 = {
104+
id: 5,
105+
status: 'OK',
106+
pkg: 'fractal-tasks-core',
107+
package_version: '0.11.0',
108+
timestamp: '2023-12-14T10:47:03.372651'
109+
};
110+
const coll2 = {
111+
id: 6,
112+
status: 'installing',
113+
pkg: 'fractal-tasks-core',
114+
package_version: '0.12.1',
115+
timestamp: '2023-12-14T10:47:03.372651'
116+
};
117+
const coll3 = {
118+
id: 7,
119+
status: 'pending',
120+
pkg: 'fractal-tasks-core',
121+
package_version: '0.12.1',
122+
timestamp: '2023-12-14T10:47:03.372651'
123+
};
124+
125+
const storageContent = JSON.stringify([coll1, coll2, coll3]);
126+
window.localStorage.setItem('TaskCollections', storageContent);
127+
128+
fetch
129+
.mockResolvedValueOnce(
130+
createFetchResponse({
131+
id: 5,
132+
data: { status: 'OK', logs: '...' }
133+
})
134+
)
135+
.mockResolvedValueOnce(
136+
createFetchResponse({
137+
id: 6,
138+
data: { status: 'installing', logs: '...' }
139+
})
140+
)
141+
.mockResolvedValueOnce(
142+
createFetchResponse({
143+
id: 7,
144+
data: { status: 'installing', logs: '...' }
145+
})
146+
)
147+
.mockResolvedValueOnce(
148+
createFetchResponse({
149+
id: 6,
150+
data: { status: 'fail', logs: '...' }
151+
})
152+
)
153+
.mockResolvedValueOnce(
154+
createFetchResponse({
155+
id: 7,
156+
data: { status: 'installing', logs: '...' }
157+
})
158+
)
159+
.mockResolvedValueOnce(
160+
createFetchResponse({
161+
id: 7,
162+
data: { status: 'OK', logs: '...' }
163+
})
164+
);
165+
166+
const result = render(TaskCollection, { props: { reloadTaskList } });
167+
168+
const table = result.getAllByRole('table')[0];
169+
let rows = table.querySelectorAll('tbody tr');
170+
expect(rows.length).eq(3);
171+
172+
let statuses = getStatuses(rows);
173+
expect(statuses[0]).eq('OK');
174+
expect(statuses[1]).eq('installing');
175+
expect(statuses[2]).eq('pending');
176+
177+
await act(() => vi.runOnlyPendingTimersAsync());
178+
179+
rows = table.querySelectorAll('tbody tr');
180+
expect(rows.length).eq(3);
181+
182+
statuses = getStatuses(rows);
183+
expect(statuses[0]).eq('OK');
184+
expect(statuses[1]).eq('fail');
185+
expect(statuses[2]).eq('installing');
186+
expect(reloaded).eq(false);
187+
188+
await act(() => vi.runOnlyPendingTimersAsync());
189+
190+
rows = table.querySelectorAll('tbody tr');
191+
expect(rows.length).eq(3);
192+
193+
statuses = getStatuses(rows);
194+
expect(statuses[0]).eq('OK');
195+
expect(statuses[1]).eq('fail');
196+
expect(statuses[2]).eq('OK');
197+
expect(reloaded).eq(true);
198+
});
199+
});
200+
201+
/**
202+
* @param {NodeListOf<Element>} rows
203+
*/
204+
function getStatuses(rows) {
205+
const statuses = [];
206+
for (const row of rows) {
207+
statuses.push(row.querySelectorAll('td')[3].textContent);
208+
}
209+
return statuses;
210+
}

‎__tests__/tasks_page.test.js‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { describe, it, expect, vi } from 'vitest';
2+
import { fireEvent, render } from '@testing-library/svelte';
3+
import { readable } from 'svelte/store';
4+
5+
// Mocking public variables
6+
vi.mock('$env/dynamic/public', () => {
7+
return { env: {} };
8+
});
9+
10+
// Mocking the page store
11+
vi.mock('$app/stores', () => {
12+
return {
13+
page: readable({
14+
data: {
15+
tasks: [
16+
{ id: 1, name: 'Apply Registration to Image', version: '0.13.1', owner: null },
17+
{ id: 2, name: 'Apply Registration to Image', version: '0.12.0', owner: null },
18+
{ id: 3, name: 'Apply Registration to Image', version: '0.10.0', owner: null },
19+
{ id: 4, name: 'Cellpose Segmentation', version: '0.13.1', owner: null },
20+
{ id: 5, name: 'Cellpose Segmentation', version: '0.12.0', owner: null },
21+
{ id: 6, name: 'Cellpose Segmentation', version: '0.10.0', owner: null },
22+
{ id: 7, name: 'Cellpose Segmentation', version: '0.10.0', owner: 'user1' },
23+
{ id: 8, name: 'task 2', version: '1.0.0', owner: 'user1' },
24+
{ id: 9, name: 'task 2', version: '0.5.0', owner: 'user1' },
25+
{ id: 10, name: 'task 3', version: '0.5.0', owner: 'user1' },
26+
{ id: 11, name: 'task 3', version: '0.4.0', owner: 'user1' },
27+
{ id: 12, name: 'task 3', version: '0.7.0', owner: 'user2' },
28+
{ id: 13, name: 'task 3', version: '0.6.0', owner: 'user2' },
29+
{ id: 14, name: 'task 3', version: '0.5.0', owner: 'user2' },
30+
{ id: 15, name: 'task 4', version: '0.5.0', owner: 'user2' },
31+
{ id: 16, name: 'task 4', version: '0.5.0', owner: 'user2' }
32+
]
33+
}
34+
})
35+
};
36+
});
37+
38+
import page from '../src/routes/tasks/+page.svelte';
39+
40+
describe('Tasks page', () => {
41+
it('display the task table', async () => {
42+
const result = render(page);
43+
44+
const [, ...rows] = result.getAllByRole('row');
45+
46+
expect(rows.length).eq(16);
47+
48+
const expandButtons = rows
49+
.map((r) => r.querySelector('td:nth-child(2) button'))
50+
.filter((b) => !!b);
51+
expect(expandButtons.length).eq(6);
52+
53+
checkCollapsedRows(rows, [1, 2, 4, 5, 8, 10, 12, 13, 15]);
54+
55+
// expand first group
56+
await fireEvent.click(expandButtons[0]);
57+
checkCollapsedRows(rows, [4, 5, 8, 10, 12, 13, 15]);
58+
59+
// expand second group
60+
await fireEvent.click(expandButtons[1]);
61+
checkCollapsedRows(rows, [1, 2, 8, 10, 12, 13, 15]);
62+
63+
// collapse second group
64+
await fireEvent.click(expandButtons[1]);
65+
checkCollapsedRows(rows, [1, 2, 4, 5, 8, 10, 12, 13, 15]);
66+
});
67+
});
68+
69+
/**
70+
* @param {HTMLElement[]} rows
71+
* @param {number[]} indexes
72+
*/
73+
function checkCollapsedRows(rows, indexes) {
74+
for (let i = 0; i < rows.length; i++) {
75+
const collapsed = indexes.includes(i);
76+
expect(rows[i].classList.contains('collapsed'), `Row #${i}`).eq(collapsed);
77+
}
78+
}

‎src/lib/common/component_utilities.js‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,32 @@ export function greatestVersionDesc(t1, t2) {
4545
* @param {import('$lib/types').Task} t2
4646
* @returns {-1|0|1}
4747
*/
48-
function compareTaskNameAndVersion(t1, t2) {
48+
export function compareTaskNameAscAndVersionAsc(t1, t2) {
4949
if (t1.name < t2.name) return -1;
5050
if (t1.name > t2.name) return 1;
5151
// Names are equal, sort by version
5252
return greatestVersionAsc(t1, t2);
5353
}
5454

55-
export function orderTasksByOwnerThenByNameThenByVersion(tasks, ownerName = null) {
55+
/**
56+
* @param {import('$lib/types').Task} t1
57+
* @param {import('$lib/types').Task} t2
58+
* @returns {-1|0|1}
59+
*/
60+
export function compareTaskNameAscAndVersionDesc(t1, t2) {
61+
if (t1.name < t2.name) return -1;
62+
if (t1.name > t2.name) return 1;
63+
// Names are equal, sort by version
64+
return greatestVersionDesc(t1, t2);
65+
}
66+
67+
/**
68+
* @param {import('$lib/types').Task[]} tasks
69+
* @param {string|null} ownerName
70+
* @param {'asc'|'desc'} order
71+
*/
72+
export function orderTasksByOwnerThenByNameThenByVersion(tasks, ownerName = null, order = 'asc') {
73+
const sortingFunction = getVersionSortingFunction(order);
5674
// Sort tasks by owner, by name and by version
5775
return tasks.sort((t1, t2) => {
5876
// If ownerName is not null, filter tasks by owner
@@ -61,7 +79,7 @@ export function orderTasksByOwnerThenByNameThenByVersion(tasks, ownerName = null
6179
if (t1.owner === ownerName) {
6280
if (t2.owner !== ownerName) return -1;
6381
// Both owners are same, sort by name and version
64-
return compareTaskNameAndVersion(t1, t2);
82+
return sortingFunction(t1, t2);
6583
} else {
6684
// t1 owner is not same as ownerName, t2 should go before t1
6785
if (t2.owner === ownerName) return 1;
@@ -72,7 +90,7 @@ export function orderTasksByOwnerThenByNameThenByVersion(tasks, ownerName = null
7290
// Should check if t2 owner is null too
7391
if (t2.owner === null) {
7492
// Both owners are null, sort by name
75-
return compareTaskNameAndVersion(t1, t2);
93+
return sortingFunction(t1, t2);
7694
} else {
7795
// t1 owner is null, t2 owner is not null, t1 should go before t2
7896
return -1;
@@ -84,11 +102,21 @@ export function orderTasksByOwnerThenByNameThenByVersion(tasks, ownerName = null
84102
if (t1.owner < t2.owner) return -1;
85103
if (t1.owner > t2.owner) return 1;
86104
// Owners are equal, sort by name
87-
return compareTaskNameAndVersion(t1, t2);
105+
return sortingFunction(t1, t2);
88106
}
89107
});
90108
}
91109

110+
/**
111+
* @param {'asc'|'desc'} order
112+
*/
113+
function getVersionSortingFunction(order) {
114+
if (order === 'asc') {
115+
return compareTaskNameAscAndVersionAsc;
116+
}
117+
return compareTaskNameAscAndVersionDesc;
118+
}
119+
92120
export function fieldHasValue(event) {
93121
const inputValue = event.target?.value || undefined;
94122
return inputValue !== undefined && inputValue !== '';

0 commit comments

Comments
 (0)