Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] améliore les messages d'erreur d'import de prescrits #10814

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
import omit from 'lodash/omit.js';

export class OrganizationImportDetail {
#errors;
constructor({ id, status, errors, createdAt, updatedAt, firstName, lastName }) {
this.id = id;
this.status = status;
this.errors = errors;
this.#errors = errors;
this.updatedAt = updatedAt;
this.createdAt = createdAt;
this.createdBy = {
firstName,
lastName,
};
}
get hasFixableErrors() {
return this.#errors?.some((error) => error.code || error.name === 'AggregateImportError') === true;
}

get errors() {
return this.#errors?.map((error) => omit(error, 'stack')) ?? [];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { Serializer } = jsonapiSerializer;

const serialize = function (organizationImportDetail, meta) {
return new Serializer('organization-import-detail', {
attributes: ['id', 'status', 'errors', 'createdBy', 'createdAt', 'updatedAt'],
attributes: ['id', 'status', 'errors', 'hasFixableErrors', 'createdBy', 'createdAt', 'updatedAt'],
meta,
}).serialize(organizationImportDetail);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { IMPORT_STATUSES } from '../../../../../../src/prescription/learner-management/domain/constants.js';
import {
AggregateImportError,
SiecleXmlImportError,
} from '../../../../../../src/prescription/learner-management/domain/errors.js';
import { OrganizationImportDetail } from '../../../../../../src/prescription/learner-management/domain/read-models/OrganizationImportDetail.js';
import { expect } from '../../../../../test-helper.js';

Expand All @@ -19,7 +23,6 @@ describe('Unit | Models | OrganizationImportDetail', function () {
const expected = {
id: 1,
status: IMPORT_STATUSES.VALIDATION_ERROR,
errors: [{ message: 'Oups' }],
updatedAt: new Date('2023-01-02'),
createdAt: new Date('2023-01-01'),
createdBy: {
Expand All @@ -30,6 +33,78 @@ describe('Unit | Models | OrganizationImportDetail', function () {

const organizationImportDetail = new OrganizationImportDetail(attributes);

expect(organizationImportDetail).to.eql(expected);
expect(organizationImportDetail).to.deep.equal(expected);
expect(organizationImportDetail.errors).to.deep.equal([{ message: 'Oups' }]);
expect(organizationImportDetail.hasFixableErrors).to.equal(false);
});

describe('hasFixableErrors', function () {
let data;

beforeEach(function () {
data = {
id: 1,
status: IMPORT_STATUSES.VALIDATION_ERROR,
updatedAt: new Date('2023-01-02'),
firstName: 'Tomie',
lastName: 'Katana',
organizationId: 1,
createdBy: 12,
createdAt: new Date('2023-01-01'),
};
});

it('should return true if error is AggregateImportError', function () {
const organizationImportDetail = new OrganizationImportDetail({
...data,
errors: [new AggregateImportError([new Error('oups')])],
});
expect(organizationImportDetail.hasFixableErrors).to.be.true;
});

it('should return true if error has a code property', function () {
const organizationImportDetail = new OrganizationImportDetail({
...data,
errors: [new SiecleXmlImportError('oups')],
});
expect(organizationImportDetail.hasFixableErrors).to.be.true;
});

it('should return false if there is no error', function () {
const organizationImportDetail = new OrganizationImportDetail({
...data,
errors: undefined,
});
expect(organizationImportDetail.hasFixableErrors).to.be.false;
});

it("should return false if errors doesn't contains DomainError", function () {
const organizationImportDetail = new OrganizationImportDetail({
...data,
errors: [new Error('oups')],
});

expect(organizationImportDetail.hasFixableErrors).to.be.false;
});
});

describe('errors', function () {
it('should remove stack trace informations from errors', function () {
// given
const updatedAt = new Date();
const createdAt = new Date();

const organizationImport = new OrganizationImportDetail({
id: 1,
status: IMPORT_STATUSES.VALIDATION_ERROR,
firstName: 'Richard',
lastName: 'Aldana',
// use object spread to mimic what is saved in db
errors: [{ ...new SiecleXmlImportError('plop', 'line 2'), stack: 'Error stack' }],
updatedAt,
createdAt,
});
expect(organizationImport.errors).to.deep.equal([{ code: 'plop', meta: 'line 2', name: 'SiecleXmlImportError' }]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe('Unit | Serializer | JSONAPI | organization-import-detail-serializer',
'created-at': createdAt,
'created-by': { firstName: 'Richard', lastName: 'Aldana' },
errors: [{ code: 'header', name: 'CsvImportError', meta: { line: 3 } }],
'has-fixable-errors': true,
},
},
});
Expand Down
6 changes: 4 additions & 2 deletions orga/app/components/import/banner.gjs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export default class ImportBanner extends Component {
}

get anchorMessage() {
if (this.args.organizationImportDetail?.hasError) {
if (this.args.organizationImportDetail?.hasFixableErrors) {
return this.intl.t('pages.organization-participants-import.banner.anchor-error');
}
return null;
Expand All @@ -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?.hasFixableErrors)
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');
}

Expand Down
1 change: 1 addition & 0 deletions orga/app/models/organization-import-detail.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export default class OrganizationImportDetail extends Model {
@attr('date') createdAt;
@attr('date') updatedAt;
@attr() errors;
@attr() hasFixableErrors;
@attr() createdBy;

get hasError() {
Expand Down
26 changes: 20 additions & 6 deletions orga/tests/integration/components/import/banner_test.gjs
Original file line number Diff line number Diff line change
Expand Up @@ -200,14 +200,15 @@ 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' }],
hasFixableErrors: true,
});
// when
const isLoading = false;
Expand All @@ -226,6 +227,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', {
Expand All @@ -237,17 +239,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',
message: 'An error occurred during process',
},
],
hasFixableErrors: false,
});
// when
const isLoading = false;
Expand All @@ -262,6 +270,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', {
Expand All @@ -272,7 +283,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 }),
);
});
});

Expand All @@ -285,6 +298,7 @@ module('Integration | Component | Import::Banner', function (hooks) {
createdAt,
createdBy: { firstName: 'Dark', lastName: 'Vador' },
errors: [{ code: 'ERROR', meta: {} }],
hasFixableErrors: true,
});
// when
const isLoading = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,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,
Expand All @@ -61,7 +61,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,
Expand All @@ -70,7 +70,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,
Expand Down
3 changes: 2 additions & 1 deletion orga/translations/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
3 changes: 2 additions & 1 deletion orga/translations/fr.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
3 changes: 2 additions & 1 deletion orga/translations/nl.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
Loading