Skip to content

Commit

Permalink
HAI-2067 Add error messages to file upload component
Browse files Browse the repository at this point in the history 
Show error message to user for each file that fails to be uploaded,
either because the file is a duplicate, fails client side validation,
fails validation in the server or fails because of other server error.

There are three kind of error messages. One for duplicate file,
one for not passing client side or server side validation and one
for other server errors.
  • Loading branch information
@markohaarni
markohaarni committed Nov 1, 2023
1 parent 49dda13 commit 84dbf63
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 26 deletions.
97 changes: 85 additions & 12 deletions src/common/components/fileUpload/FileUpload.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ function uploadFunction(file: File) {
}

test('Should upload files successfully and loading indicator is displayed', async () => {
server.use(
rest.post('/api/hakemukset/:id/liitteet', async (req, res, ctx) => {
return res(ctx.delay(), ctx.status(200));
}),
);

const inputLabel = 'Choose a file';
const { user } = render(
<FileUpload
Expand All @@ -54,7 +48,23 @@ test('Should upload files successfully and loading indicator is displayed', asyn
});
});

test('Should show amount of successful files uploaded correctly when file fails in validation', async () => {
test('Should show amount of successful files uploaded and errors correctly when files fails in validation', async () => {
const fileNameA = 'test-file-a.png';
const fileNameB = 'test-file-b.pdf';
const fileNameC = 'test-file-c.png';
const fileNameD = 'test-file-d.png';
const existingFileA: AttachmentMetadata = {
id: '4f08ce3f-a0de-43c6-8ccc-9fe93822ed18',
fileName: fileNameA,
createdByUserId: 'b9a58f4c-f5fe-11ec-997f-0a580a800284',
createdAt: '2023-07-04T12:07:52.324684Z',
};
const existingFileB: AttachmentMetadata = {
id: '4f08ce3f-a0de-43c6-8ccc-9fe93822ed20',
fileName: fileNameD,
createdByUserId: 'b9a58f4c-f5fe-11ec-997f-0a580a800284',
createdAt: '2023-07-04T12:08:52.324684Z',
};
const inputLabel = 'Choose a file';
const { user } = render(
<FileUpload
Expand All @@ -64,29 +74,83 @@ test('Should show amount of successful files uploaded correctly when file fails
multiple
uploadFunction={uploadFunction}
fileDeleteFunction={() => Promise.resolve()}
existingAttachments={[existingFileA, existingFileB]}
/>,
undefined,
undefined,
{ applyAccept: false },
);
const fileUpload = screen.getByLabelText(inputLabel);
user.upload(fileUpload, [
new File(['test-a'], 'test-file-a.png', { type: 'image/png' }),
new File(['test-b'], 'test-file-b.pdf', { type: 'application/pdf' }),
new File(['test-a'], fileNameA, { type: 'image/png' }),
new File(['test-b'], fileNameB, { type: 'application/pdf' }),
new File(['test-c'], fileNameC, { type: 'image/png' }),
]);

await waitFor(() => {
expect(screen.queryByText('1/2 tiedosto(a) tallennettu')).toBeInTheDocument();
expect(screen.queryByText('1/3 tiedosto(a) tallennettu')).toBeInTheDocument();
});
expect(
screen.queryByText('Liitteen tallennus epäonnistui 2/3 tiedostolle', { exact: false }),
).toBeInTheDocument();
expect(
screen.queryByText(
`Tiedosto (${fileNameB}) on viallinen. Tarkistathan, että tiedostomuoto on oikea eikä sen koko ylitä sallittua maksimikokoa.`,
{ exact: false },
),
).toBeInTheDocument();
expect(
screen.queryByText(
`Valittu tiedostonimi (${fileNameA}) on jo käytössä. Nimeä tiedosto uudelleen.`,
{ exact: false },
),
).toBeInTheDocument();
});

test('Should show amount of successful files uploaded correctly when request fails', async () => {
test('Should show amount of successful files uploaded and errors correctly when request fails for bad request', async () => {
server.use(
rest.post('/api/hakemukset/:id/liitteet', async (req, res, ctx) => {
return res(ctx.status(400), ctx.json({ errorMessage: 'Failed for testing purposes' }));
}),
);

const fileNameA = 'test-file-a.png';
const inputLabel = 'Choose a file';
const { user } = render(
<FileUpload
id="test-file-upload"
label={inputLabel}
accept=".png,.jpg"
multiple
uploadFunction={uploadFunction}
fileDeleteFunction={() => Promise.resolve()}
/>,
);
const fileUpload = screen.getByLabelText(inputLabel);
user.upload(fileUpload, [new File(['test-a'], fileNameA, { type: 'image/png' })]);

await waitFor(() => {
expect(screen.queryByText('0/1 tiedosto(a) tallennettu')).toBeInTheDocument();
});
expect(
screen.queryByText('Liitteen tallennus epäonnistui 1/1 tiedostolle', { exact: false }),
).toBeInTheDocument();
expect(
screen.queryByText(
`Tiedosto (${fileNameA}) on viallinen. Tarkistathan, että tiedostomuoto on oikea eikä sen koko ylitä sallittua maksimikokoa.`,
{ exact: false },
),
).toBeInTheDocument();
});

test('Should show correct error message when request fails for server error', async () => {
server.use(
rest.post('/api/hakemukset/:id/liitteet', async (req, res, ctx) => {
return res(ctx.status(500), ctx.json({ errorMessage: 'Failed for testing purposes' }));
}),
);

const fileNameA = 'test-file-a.png';
const inputLabel = 'Choose a file';
const { user } = render(
<FileUpload
Expand All @@ -99,11 +163,20 @@ test('Should show amount of successful files uploaded correctly when request fai
/>,
);
const fileUpload = screen.getByLabelText(inputLabel);
user.upload(fileUpload, [new File(['test-a'], 'test-file-a.png', { type: 'image/png' })]);
user.upload(fileUpload, [new File(['test-a'], fileNameA, { type: 'image/png' })]);

await waitFor(() => {
expect(screen.queryByText('0/1 tiedosto(a) tallennettu')).toBeInTheDocument();
});
expect(
screen.queryByText('Liitteen tallennus epäonnistui 1/1 tiedostolle', { exact: false }),
).toBeInTheDocument();
expect(
screen.queryByText(
`Tiedoston (${fileNameA}) lataus epäonnistui, yritä hetken päästä uudelleen.`,
{ exact: false },
),
).toBeInTheDocument();
});

test('Should upload files when user drops them into drag-and-drop area', async () => {
Expand Down
65 changes: 53 additions & 12 deletions src/common/components/fileUpload/FileUpload.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useEffect, useRef, useState } from 'react';
import { useMutation } from 'react-query';
import { Flex } from '@chakra-ui/react';
import { FileInput, IconCheckCircleFill, LoadingSpinner } from 'hds-react';
import { FileInput, IconAlertCircleFill, IconCheckCircleFill, LoadingSpinner } from 'hds-react';
import { useTranslation } from 'react-i18next';
import { differenceBy } from 'lodash';
import { AxiosError } from 'axios';
Expand Down Expand Up @@ -34,10 +34,10 @@ function useDragAndDropFiles() {

function SuccessNotification({
successfulCount,
totalCount,
newFiles,
}: {
successfulCount: number;
totalCount: number;
newFiles: number;
}) {
const { t } = useTranslation();

Expand All @@ -47,13 +47,36 @@ function SuccessNotification({
<p>
{t('common:components:fileUpload:successNotification', {
successful: successfulCount,
total: totalCount,
newFiles: newFiles,
})}
</p>
</Flex>
);
}

function ErrorNotification({ errors, newFiles }: { errors: string[]; newFiles: number }) {
const { t } = useTranslation();

return (
<Flex color="var(--color-error)">
<IconAlertCircleFill style={{ marginRight: 'var(--spacing-2-xs)' }} />
<div>
<p>
{t('common:components:fileUpload:errorNotification', {
errors: errors.length,
newFiles: newFiles,
})}
</p>
<ul style={{ listStyle: 'none' }}>
{errors.map((error, index) => (
<li key={index}>- {error}</li>
))}
</ul>
</div>
</Flex>
);
}

type Props<T extends AttachmentMetadata> = {
/** id of the input element */
id: string;
Expand Down Expand Up @@ -95,10 +118,16 @@ export default function FileUpload<T extends AttachmentMetadata>({
const { t } = useTranslation();
const locale = useLocale();
const [newFiles, setNewFiles] = useState<File[]>([]);
const [invalidFiles, setInvalidFiles] = useState<File[]>([]);
const [fileUploadErrors, setFileUploadErrors] = useState<string[]>([]);
const uploadMutation = useMutation(uploadFunction, {
onError(error: AxiosError, file) {
setInvalidFiles((files) => [...files, file]);
const errorText =
error.response?.status === 400
? t('form:errors:fileLoadBadFileError', { fileName: file.name })
: t('form:errors:fileLoadTechnicalError', { fileName: file.name });
setFileUploadErrors((errors) => {
return [...errors, errorText];
});
},
});
const [filesUploading, setFilesUploading] = useState(false);
Expand All @@ -122,20 +151,28 @@ export default function FileUpload<T extends AttachmentMetadata>({
}
}

function handleFilesChange(files: File[]) {
function handleFilesChange(validFiles: File[]) {
// Filter out attachments that have same names as those that have already been sent
const [filesToUpload] = removeDuplicateAttachments(files, existingAttachments);
const [filesToUpload, duplicateFiles] = removeDuplicateAttachments(
validFiles,
existingAttachments,
);

// Determine which files haven't passed HDS FileInput validation by comparing
// files in input element or files dropped into drop zone to files received as
// argument to this onChange function
const inputElem = document.getElementById(id) as HTMLInputElement;
const inputElemFiles = inputElem.files ? Array.from(inputElem.files) : [];
const allFiles = inputElemFiles.length > 0 ? inputElemFiles : dragAndDropFiles.current;
const invalidFilesArr = differenceBy(allFiles, filesToUpload, 'name');

const invalidFiles = differenceBy(allFiles, validFiles, 'name');
const errors: string[] = invalidFiles
.map((file) => t('form:errors:fileLoadBadFileError', { fileName: file.name }))
.concat(
duplicateFiles.map((file) => t('form:errors:duplicateFileError', { fileName: file.name })),
);
setFileUploadErrors(errors);
setNewFiles(allFiles);
setInvalidFiles(invalidFilesArr);
uploadFiles(filesToUpload);
}

Expand Down Expand Up @@ -173,11 +210,15 @@ export default function FileUpload<T extends AttachmentMetadata>({

{!filesUploading && newFiles.length > 0 && (
<SuccessNotification
successfulCount={newFiles.length - invalidFiles.length}
totalCount={newFiles.length}
successfulCount={newFiles.length - fileUploadErrors.length}
newFiles={newFiles.length}
/>
)}

{!filesUploading && fileUploadErrors.length > 0 && (
<ErrorNotification errors={fileUploadErrors} newFiles={newFiles.length} />
)}

<FileList
files={existingAttachments}
fileDownLoadFunction={fileDownLoadFunction}
Expand Down
6 changes: 4 additions & 2 deletions src/locales/fi.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@
},
"fileUpload": {
"loadingText": "Tallennetaan tiedostoja",
"successNotification": "{{successful}}/{{total}} tiedosto(a) tallennettu",
"successNotification": "{{successful}}/{{newFiles}} tiedosto(a) tallennettu",
"errorNotification": "Liitteen tallennus epäonnistui {{errors}}/{{newFiles}} tiedostolle:",
"deleteError": {
"title": "Tiedoston poistamisessa tapahtui virhe",
"fileNotFound": "Tiedostoa, jonka yritit poistaa ei löydy (virhe 404). Yritä myöhemmin uudelleen.",
Expand Down Expand Up @@ -92,7 +93,8 @@
"fileLoadError": "Tiedoston latauksessa tapahtui virhe",
"selfIntersectingArea": "Alueen reunat eivät saa leikata toisiaan",
"fileLoadTechnicalError": "Tiedoston ({{fileName}}) lataus epäonnistui, yritä hetken päästä uudelleen.",
"fileLoadBadFileError": "Tiedosto ({{fileName}}) on viallinen. Tarkistathan, että tiedostomuoto on oikea eikä sen koko ylitä sallittua maksimikokoa."
"fileLoadBadFileError": "Tiedosto ({{fileName}}) on viallinen. Tarkistathan, että tiedostomuoto on oikea eikä sen koko ylitä sallittua maksimikokoa.",
"duplicateFileError": "Valittu tiedostonimi ({{fileName}}) on jo käytössä. Nimeä tiedosto uudelleen."
},
"headers": {
"perustiedot": "Perustiedot",
Expand Down

0 comments on commit 84dbf63

Please sign in to comment.