From d28ec8602665c2499e4aeb8a6460b07b0a045450 Mon Sep 17 00:00:00 2001 From: LionelB Date: Fri, 13 Dec 2024 15:29:19 +0100 Subject: [PATCH] feat(orga): improve error message after learner import --- orga/app/components/import/banner.gjs | 6 ++-- orga/app/components/import/index.gjs | 2 +- orga/app/models/organization-import-detail.js | 4 +++ .../components/import/banner_test.gjs | 24 +++++++++---- ....js => organization-import-detail-test.js} | 36 +++++++++++++++++-- orga/translations/en.json | 3 +- orga/translations/fr.json | 3 +- orga/translations/nl.json | 3 +- 8 files changed, 66 insertions(+), 15 deletions(-) rename orga/tests/unit/models/{organization-import-test.js => organization-import-detail-test.js} (65%) diff --git a/orga/app/components/import/banner.gjs b/orga/app/components/import/banner.gjs index 7cceb7dae05..9b63b37c0d7 100644 --- a/orga/app/components/import/banner.gjs +++ b/orga/app/components/import/banner.gjs @@ -63,7 +63,7 @@ export default class ImportBanner extends Component { } get anchorMessage() { - if (this.args.organizationImportDetail?.hasError) { + if (this.args.organizationImportDetail?.hasFixableError) { return this.intl.t('pages.organization-participants-import.banner.anchor-error'); } return null; @@ -87,7 +87,9 @@ export default class ImportBanner extends Component { get actionMessage() { if (this.args.organizationImportDetail?.hasError) - return this.intl.t('pages.organization-participants-import.banner.error-text'); + if (this.args.organizationImportDetail?.hasFixableError) + return this.intl.t('pages.organization-participants-import.banner.fix-error-text'); + else return this.intl.t('pages.organization-participants-import.banner.retry-error-text'); return this.intl.t('pages.organization-participants-import.banner.in-progress-text'); } diff --git a/orga/app/components/import/index.gjs b/orga/app/components/import/index.gjs index c0df6502048..2df593abc41 100644 --- a/orga/app/components/import/index.gjs +++ b/orga/app/components/import/index.gjs @@ -18,7 +18,7 @@ export default class Import extends Component { @service intl; get displayImportMessagePanel() { - return this.args.organizationImportDetail?.hasError || this.args.organizationImportDetail?.hasWarning; + return this.args.organizationImportDetail?.hasFixableError || this.args.organizationImportDetail?.hasWarning; } get inProgress() { diff --git a/orga/app/models/organization-import-detail.js b/orga/app/models/organization-import-detail.js index 89e626f6f00..ad9cbc64bce 100644 --- a/orga/app/models/organization-import-detail.js +++ b/orga/app/models/organization-import-detail.js @@ -11,6 +11,10 @@ export default class OrganizationImportDetail extends Model { return /ERROR/.test(this.status) && this.errors?.length > 0; } + get hasFixableError() { + return this.hasError && this.errors.some((error) => error.code || /aggregate/i.test(error.name)); + } + get hasWarning() { return this.isDone && this.errors?.length > 0; } diff --git a/orga/tests/integration/components/import/banner_test.gjs b/orga/tests/integration/components/import/banner_test.gjs index 253456b84b0..430666f20b6 100644 --- a/orga/tests/integration/components/import/banner_test.gjs +++ b/orga/tests/integration/components/import/banner_test.gjs @@ -200,14 +200,14 @@ module('Integration | Component | Import::Banner', function (hooks) { }); module('errors', function () { - test('display validation error banner when validation failed', async function (assert) { + test('display validation error banner when validation failed (fix)', async function (assert) { const store = this.owner.lookup('service:store'); const createdAt = new Date(2023, 1, 10); const organizationImportDetail = store.createRecord('organization-import-detail', { status: 'VALIDATION_ERROR', createdAt, createdBy: { firstName: 'Dark', lastName: 'Vador' }, - errors: [{ code: 'UAI_MISMATCHED', meta: {} }], + errors: [{ meta: [{ code: 'UAI_MISMATCHED' }], name: 'AggregateImportError', stack: '' }], }); // when const isLoading = false; @@ -226,6 +226,7 @@ module('Integration | Component | Import::Banner', function (hooks) { exact: false, }), ); + assert.ok(screen.getByRole('link', { name: t('pages.organization-participants-import.banner.anchor-error') })); assert.ok( screen.getByText( t('pages.organization-participants-import.banner.upload-completed', { @@ -237,17 +238,23 @@ module('Integration | Component | Import::Banner', function (hooks) { ), ); - assert.ok(screen.getByText(t('pages.organization-participants-import.banner.error-text'), { exact: false })); + assert.ok(screen.getByText(t('pages.organization-participants-import.banner.fix-error-text'), { exact: false })); }); - test('display import error banner when import failed', async function (assert) { + test('display import error banner when import failed (retry)', async function (assert) { const store = this.owner.lookup('service:store'); const createdAt = new Date(2023, 1, 10); const organizationImportDetail = store.createRecord('organization-import-detail', { status: 'IMPORT_ERROR', createdAt, createdBy: { firstName: 'Dark', lastName: 'Vador' }, - errors: [{ code: 'ERROR', meta: {} }], + errors: [ + { + name: 'OrganizationLearnersCouldNotBeSavedError', + stack: 'OrganizationLearnersCouldNotBeSavedError: An error occurred during process', + message: 'An error occurred during process', + }, + ], }); // when const isLoading = false; @@ -262,6 +269,9 @@ module('Integration | Component | Import::Banner', function (hooks) { exact: false, }), ); + assert.notOk( + screen.queryByRole('link', { name: t('pages.organization-participants-import.banner.anchor-error') }), + ); assert.ok( screen.getByText( t('pages.organization-participants-import.banner.upload-completed', { @@ -272,7 +282,9 @@ module('Integration | Component | Import::Banner', function (hooks) { { exact: false }, ), ); - assert.ok(screen.getByText(t('pages.organization-participants-import.banner.error-text'), { exact: false })); + assert.ok( + screen.getByText(t('pages.organization-participants-import.banner.retry-error-text'), { exact: false }), + ); }); }); diff --git a/orga/tests/unit/models/organization-import-test.js b/orga/tests/unit/models/organization-import-detail-test.js similarity index 65% rename from orga/tests/unit/models/organization-import-test.js rename to orga/tests/unit/models/organization-import-detail-test.js index fb65de53d5f..c17fc320f75 100644 --- a/orga/tests/unit/models/organization-import-test.js +++ b/orga/tests/unit/models/organization-import-detail-test.js @@ -24,6 +24,36 @@ module('Unit | Model | organization-import-detail', function (hooks) { }); }); }); + module('#hasFixableError', function () { + ['UPLOAD_ERROR', 'VALIDATION_ERROR', 'IMPORT_ERROR'].forEach((status) => { + test(`it should return true for error with code - ${status}`, function (assert) { + const store = this.owner.lookup('service:store'); + const model = store.createRecord('organization-import-detail', { + status, + errors: [{ code: 'toto' }], + }); + assert.ok(model.hasFixableError); + }); + test(`it should return true for aggregated errors - ${status}`, function (assert) { + const store = this.owner.lookup('service:store'); + const model = store.createRecord('organization-import-detail', { + status, + errors: [{ name: 'AggregateImportError', meta: [{ code: 'toto' }] }], + }); + assert.ok(model.hasFixableError); + }); + }); + ['UPLOADED', 'VALIDATED', 'IMPORTED'].forEach((status) => { + test(`it should return false - ${status}`, function (assert) { + const store = this.owner.lookup('service:store'); + const model = store.createRecord('organization-import-detail', { + status, + errors: [{ stack: 'error' }], + }); + assert.notOk(model.hasFixableError); + }); + }); + }); module('#hasWarning', function () { test('it should return true', function (assert) { const store = this.owner.lookup('service:store'); @@ -50,7 +80,7 @@ module('Unit | Model | organization-import-detail', function (hooks) { assert.ok(model.isDone); }); ['UPLOADED', 'VALIDATED', 'UPLOAD_ERROR', 'VALIDATION_ERROR', 'IMPORT_ERROR'].forEach((status) => { - test('it should return false', function (assert) { + test(`it should return false ${status}`, function (assert) { const store = this.owner.lookup('service:store'); const model = store.createRecord('organization-import-detail', { status, @@ -61,7 +91,7 @@ module('Unit | Model | organization-import-detail', function (hooks) { }); module('inProgress', function () { ['UPLOADED', 'VALIDATED'].forEach((status) => { - test('it should return true', function (assert) { + test(`it should return true - ${status}`, function (assert) { const store = this.owner.lookup('service:store'); const model = store.createRecord('organization-import-detail', { status, @@ -70,7 +100,7 @@ module('Unit | Model | organization-import-detail', function (hooks) { }); }); ['UPLOAD_ERROR', 'VALIDATION_ERROR', 'IMPORT_ERROR', 'IMPORTED'].forEach((status) => { - test('it should return false', function (assert) { + test(`it should return false - ${status}`, function (assert) { const store = this.owner.lookup('service:store'); const model = store.createRecord('organization-import-detail', { status, diff --git a/orga/translations/en.json b/orga/translations/en.json index e9c754982aa..e0be292d02f 100644 --- a/orga/translations/en.json +++ b/orga/translations/en.json @@ -1184,10 +1184,11 @@ }, "banner": { "anchor-error": "See the list of errors", - "error-text": "Please correct them and import the file again.", + "fix-error-text": "Please correct them and import the file again.", "import-error": "The import of participants has failed.", "import-in-progress": "The content of the file has been validated and the participants are now being imported in PixOrga.", "in-progress-text": "Please refresh this page in a few moments to see the status of the file.", + "retry-error-text": "An error has occurred, please import later or write to Pix support", "upload-completed": "The file was imported on {date} by {firstName} {lastName}.", "upload-error": "The file could not be imported.", "upload-in-progress": "It may take a few minutes to import the file. Please do not leave this page.", diff --git a/orga/translations/fr.json b/orga/translations/fr.json index ff89c0c2a14..ce3be5f61b0 100644 --- a/orga/translations/fr.json +++ b/orga/translations/fr.json @@ -1190,10 +1190,11 @@ }, "banner": { "anchor-error": "Voir la liste des erreurs", - "error-text": "Merci de corriger les erreurs et d’importer à nouveau le fichier.", + "fix-error-text": "Merci de corriger les erreurs et d’importer à nouveau le fichier.", "import-error": "L'import des participants a échoué.", "import-in-progress": "Le contenu du fichier est validé, l’import des participants est en cours.", "in-progress-text": "Merci de rafraîchir cette page dans quelques instants pour connaitre l’état de traitement du fichier.", + "retry-error-text": "Une erreur est survenue, merci d'importer plus tard ou d'écrire au support Pix", "upload-completed": "Le fichier a été importé le {date} par {firstName} {lastName}.", "upload-error": "Le fichier n'a pas pu être importé.", "upload-in-progress": "L’import du fichier peut prendre quelques minutes. Merci de ne pas quitter cette page.", diff --git a/orga/translations/nl.json b/orga/translations/nl.json index c5e853896f8..72e21315f8c 100644 --- a/orga/translations/nl.json +++ b/orga/translations/nl.json @@ -1104,7 +1104,8 @@ }, "banner": { "anchor-error": "De lijst met foutmeldingen bekijken", - "error-text": "Corrigeer de fouten en importeer het bestand opnieuw.", + "fix-error-text": "Corrigeer de fouten en importeer het bestand opnieuw.", + "retry-error-text": "Er is een fout opgetreden, importeer later of schrijf naar Pix support.", "import-error": "De import van deelnemers is mislukt.", "import-in-progress": "De inhoud van het bestand is gevalideerd en de deelnemers worden geïmporteerd.", "in-progress-text": "Vernieuw deze pagina over enkele ogenblikken om de status van het bestand te controleren.",