Skip to content

Commit 4c48b15

Browse files
committed
Improved handling of viewer paths errors
1 parent 07b7447 commit 4c48b15

File tree

4 files changed

+66
-9
lines changed

4 files changed

+66
-9
lines changed

__tests__/errors.test.js

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect } from 'vitest';
2-
import { getFieldValidationError } from '../src/lib/common/errors';
2+
import { getFieldValidationError, getValidationMessagesMap } from '../src/lib/common/errors';
33

44
describe('Error utility functions', () => {
55
it('get field validation error without specifying loc', () => {
@@ -17,4 +17,41 @@ describe('Error utility functions', () => {
1717
);
1818
expect(error).eq('value is not a valid list');
1919
});
20+
21+
it('get validation errors array', () => {
22+
const errorMap = getValidationMessagesMap(
23+
{
24+
detail: [
25+
{
26+
type: 'string_too_short',
27+
loc: ['body', 'viewer_paths', 1],
28+
msg: 'String should have at least 1 character',
29+
input: '',
30+
ctx: {
31+
min_length: 1
32+
}
33+
},
34+
{
35+
type: 'value_error',
36+
loc: ['body', 'viewer_paths', 3],
37+
msg: "Value error, String must be an absolute path (given 'foobar').",
38+
input: 'foobar',
39+
ctx: {
40+
error: {}
41+
}
42+
}
43+
]
44+
},
45+
422
46+
);
47+
48+
/** @type {string[]} */
49+
const errors = /** @type {string[]} */ ((errorMap && errorMap['viewer_paths']) || []);
50+
51+
expect(errors.length).toEqual(4);
52+
expect(errors[0]).toBeUndefined();
53+
expect(errors[1]).toEqual('String should have at least 1 character');
54+
expect(errors[2]).toBeUndefined();
55+
expect(errors[3]).toEqual("Value error, String must be an absolute path (given 'foobar').");
56+
});
2057
});

src/lib/common/errors.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ function extractFieldValidationError(simpleValidationMessage, loc) {
154154
/**
155155
* @param {any} reason
156156
* @param {number | null} statusCode
157-
* @returns {null | { [key: string]: string }}
157+
* @returns {null | { [key: string]: string | string[] }}
158158
*/
159159
export function getValidationMessagesMap(reason, statusCode) {
160160
if (!isValidationError(reason, statusCode)) {
@@ -163,16 +163,25 @@ export function getValidationMessagesMap(reason, statusCode) {
163163
if (!Array.isArray(reason.detail) || reason.detail.length === 0) {
164164
return null;
165165
}
166-
/** @type {{[key: string]: string}} */
166+
/** @type {{[key: string]: string | string[]}} */
167167
const map = {};
168168
for (const error of reason.detail) {
169169
if (!hasValidationErrorPayload(error)) {
170170
return null;
171171
}
172-
if (error.loc.length !== 2 || error.loc[0] !== 'body') {
172+
if (error.loc[0] !== 'body') {
173+
return null;
174+
}
175+
if (error.loc.length === 2) {
176+
map[error.loc[1]] = error.msg;
177+
} else if (error.loc.length === 3 && typeof error.loc[2] === 'number') {
178+
if (!(error.loc[1] in map)) {
179+
map[error.loc[1]] = [];
180+
}
181+
/** @type {string[]} */ (map[error.loc[1]])[error.loc[2]] = error.msg;
182+
} else {
173183
return null;
174184
}
175-
map[error.loc[1]] = error.msg;
176185
}
177186
return map;
178187
}
@@ -294,7 +303,7 @@ export class FormErrorHandler {
294303
* Returns true if all the keys of the error map are handled by the current page or component.
295304
* Used to decide if it possible to show user friendly validation messages
296305
* or if it is necessary to display a generic error message.
297-
* @param {{[key:string]: string}} errorsMap
306+
* @param {{[key:string]: string | string[] }} errorsMap
298307
* @return {boolean}
299308
*/
300309
validateErrorMapKeys(errorsMap) {

src/routes/settings/+page.svelte

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
let errorShown = false;
6161
if (errorMap) {
6262
if ('slurm_accounts' in errorMap) {
63-
slurmAccountsError = errorMap['slurm_accounts'];
63+
slurmAccountsError = /** @type {string} */ (errorMap['slurm_accounts']);
6464
errorShown = true;
6565
}
6666
}

src/routes/v2/admin/groups/[groupId]/edit/+page.svelte

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,13 +326,19 @@
326326
<h4 class="fw-light">Viewer paths</h4>
327327
<!-- eslint-disable-next-line no-unused-vars -->
328328
{#each editableViewPaths as _, i (i)}
329-
<div class="input-group mb-2">
329+
<div
330+
class="input-group mb-2"
331+
class:has-validation={$viewerPathsValidationErrors['viewer_paths'] &&
332+
$viewerPathsValidationErrors['viewer_paths'][i]}
333+
>
330334
<input
331335
type="text"
332336
class="form-control"
333337
id={`viewerPath-${i}`}
334338
bind:value={editableViewPaths[i]}
335339
aria-label={`Viewer path #${i + 1}`}
340+
class:is-invalid={$viewerPathsValidationErrors['viewer_paths'] &&
341+
$viewerPathsValidationErrors['viewer_paths'][i]}
336342
required
337343
/>
338344
<button
@@ -344,11 +350,16 @@
344350
>
345351
<i class="bi bi-trash"></i>
346352
</button>
353+
{#if $viewerPathsValidationErrors['viewer_paths'] && $viewerPathsValidationErrors['viewer_paths'][i]}
354+
<div class="invalid-feedback">
355+
{$viewerPathsValidationErrors['viewer_paths'][i]}
356+
</div>
357+
{/if}
347358
</div>
348359
{/each}
349360
<button class="btn btn-secondary mb-2" onclick={addViewerPath}>Add viewer path</button>
350361
<div id="viewerPathGenericError"></div>
351-
{#if $viewerPathsValidationErrors['viewer_paths']}
362+
{#if $viewerPathsValidationErrors['viewer_paths'] && !Array.isArray($viewerPathsValidationErrors['viewer_paths'])}
352363
<div class="alert alert-danger mb-2">
353364
{$viewerPathsValidationErrors['viewer_paths']}
354365
</div>

0 commit comments

Comments
 (0)