From 61041c16f00c1e15b250143af37f9b4f9ee11b14 Mon Sep 17 00:00:00 2001 From: Xavier Carron <33637571+xav-car@users.noreply.github.com> Date: Fri, 27 Sep 2024 16:59:35 +0200 Subject: [PATCH 1/4] refactor?(api): fix naming property on import format repository --- ...zation-learner-import-format-repository.js | 2 +- ...n-learner-import-format-repository_test.js | 24 +++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/api/src/prescription/learner-management/infrastructure/repositories/organization-learner-import-format-repository.js b/api/src/prescription/learner-management/infrastructure/repositories/organization-learner-import-format-repository.js index 7bc188ac427..21848f01a23 100644 --- a/api/src/prescription/learner-management/infrastructure/repositories/organization-learner-import-format-repository.js +++ b/api/src/prescription/learner-management/infrastructure/repositories/organization-learner-import-format-repository.js @@ -35,7 +35,7 @@ const get = async function (organizationId) { /** * @type {function} * @param {Object} params - * @param {Array|Object} params.organizationImports + * @param {Array|Object} params.organizationLearnerImportFormats * @return {Promise} */ const updateAllByName = async function ({ organizationLearnerImportFormats }) { diff --git a/api/tests/prescription/learner-management/integration/infrastructure/repositories/organization-learner-import-format-repository_test.js b/api/tests/prescription/learner-management/integration/infrastructure/repositories/organization-learner-import-format-repository_test.js index 4c5ffde9e25..50d889371ae 100644 --- a/api/tests/prescription/learner-management/integration/infrastructure/repositories/organization-learner-import-format-repository_test.js +++ b/api/tests/prescription/learner-management/integration/infrastructure/repositories/organization-learner-import-format-repository_test.js @@ -99,8 +99,16 @@ describe('Integration | Repository | Organization Learner Management | Organizat it('update several learner import format given', async function () { // given const organizationLearnerImportFormats = [ - { name: 'FIRST_FORMAT', fileType: 'csv', config: { new_config: 'awesome' } }, - { name: 'SECOND_FORMAT', fileType: 'csv', config: { new_config: 'not_bad' } }, + new OrganizationLearnerImportFormat({ + name: 'FIRST_FORMAT', + fileType: 'csv', + config: { new_config: 'awesome' }, + }), + new OrganizationLearnerImportFormat({ + name: 'SECOND_FORMAT', + fileType: 'csv', + config: { new_config: 'not_bad' }, + }), ]; // when await organizationLearnerImportFormatRepository.updateAllByName({ organizationLearnerImportFormats }); @@ -128,7 +136,11 @@ describe('Integration | Repository | Organization Learner Management | Organizat it('set updatedAt field to today', async function () { // given const organizationLearnerImportFormats = [ - { name: 'FIRST_FORMAT', fileType: 'csv', config: { new_config: 'awesome' } }, + new OrganizationLearnerImportFormat({ + name: 'FIRST_FORMAT', + fileType: 'csv', + config: { new_config: 'awesome' }, + }), ]; // when await organizationLearnerImportFormatRepository.updateAllByName({ organizationLearnerImportFormats }); @@ -146,7 +158,11 @@ describe('Integration | Repository | Organization Learner Management | Organizat it('should not update other import format', async function () { // given const organizationLearnerImportFormats = [ - { name: 'FIRST_FORMAT', fileType: 'csv', config: { new_config: 'awesome' } }, + new OrganizationLearnerImportFormat({ + name: 'FIRST_FORMAT', + fileType: 'csv', + config: { new_config: 'awesome' }, + }), ]; // when await organizationLearnerImportFormatRepository.updateAllByName({ organizationLearnerImportFormats }); From 638245d2a3313aaea839b5e2e299ef2a7ba88445 Mon Sep 17 00:00:00 2001 From: Xavier Carron <33637571+xav-car@users.noreply.github.com> Date: Fri, 27 Sep 2024 16:43:12 +0200 Subject: [PATCH 2/4] feat(api): update usecase to payload instead of object --- .../update-organization-import-format.js | 52 ------------------- ...ate-organization-learner-import-formats.js | 18 +++++-- .../test-file/import-format-file/ko.json | 4 ++ .../import-format-file/not-a-json.json | 1 + .../test-file/import-format-file/ok.json | 1 + ...ganization-learner-import-formats_test.js} | 48 ++++++++++++++--- 6 files changed, 61 insertions(+), 63 deletions(-) delete mode 100644 api/scripts/organization-import-format/update-organization-import-format.js create mode 100644 api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/ko.json create mode 100644 api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/not-a-json.json create mode 100644 api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/ok.json rename api/tests/prescription/learner-management/integration/domain/usecases/{update-organization-import-learner-import-format_test.js => update-organization-learner-import-formats_test.js} (58%) diff --git a/api/scripts/organization-import-format/update-organization-import-format.js b/api/scripts/organization-import-format/update-organization-import-format.js deleted file mode 100644 index 5c7102eba7d..00000000000 --- a/api/scripts/organization-import-format/update-organization-import-format.js +++ /dev/null @@ -1,52 +0,0 @@ -import { readFile } from 'node:fs/promises'; -import * as url from 'node:url'; - -import { disconnect } from '../../db/knex-database-connection.js'; -import { usecases } from '../../src/prescription/learner-management/domain/usecases/index.js'; -import { DomainTransaction } from '../../src/shared/domain/DomainTransaction.js'; - -// Usage: node scripts/organization-import-format/update-organization-import-format.js path/data.json -// data.json -// [{ -// "name": 'FORMAT_NAME', -// "config": { -// "new_config": "awesome", -// }, -// "fileType": "csv", -// }] - -const modulePath = url.fileURLToPath(import.meta.url); -const isLaunchedFromCommandLine = process.argv[1] === modulePath; - -async function main() { - console.log('Starting update import format configuration'); - - const filePath = process.argv[2]; - - console.log('Reading json data file... '); - const jsonFile = await readFile(filePath); - const rawImportFormats = JSON.parse(jsonFile); - console.log(`Import Format to update : ${rawImportFormats.length}`); - - try { - await DomainTransaction.execute(async () => { - await usecases.updateOrganizationLearnerImportFormats({ rawImportFormats }); - }); - console.log('ok'); - } catch (error) { - console.log(error); - } -} - -(async () => { - if (isLaunchedFromCommandLine) { - try { - await main(); - } catch (error) { - console.error(error); - process.exitCode = 1; - } finally { - await disconnect(); - } - } -})(); diff --git a/api/src/prescription/learner-management/domain/usecases/update-organization-learner-import-formats.js b/api/src/prescription/learner-management/domain/usecases/update-organization-learner-import-formats.js index a467a6d6b35..20cdfb851f8 100644 --- a/api/src/prescription/learner-management/domain/usecases/update-organization-learner-import-formats.js +++ b/api/src/prescription/learner-management/domain/usecases/update-organization-learner-import-formats.js @@ -1,17 +1,27 @@ -import { EntityValidationError } from '../../../../shared/domain/errors.js'; -import { OrganizationLearnerImportFormat } from '../models/OrganizationLearnerImportFormat.js'; +import { readFile } from 'node:fs/promises'; +import { EntityValidationError, FileValidationError } from '../../../../shared/domain/errors.js'; +import { OrganizationLearnerImportFormat } from '../models/OrganizationLearnerImportFormat.js'; /** * @param {Object} params * @param {Object} params.importFormats - * @param {OrganizationImportFormat} params.organizationLearnerImportFormatRepository + * @param {OrganizationLearnerImportFormatRepository} params.organizationLearnerImportFormatRepository * @returns {Promise} */ const updateOrganizationLearnerImportFormats = async function ({ - rawImportFormats, + payload, organizationLearnerImportFormatRepository, + dependencies = { readFile, jsonParse: JSON.parse }, }) { const errors = []; + let rawImportFormats; + try { + const buffer = await dependencies.readFile(payload.path); + rawImportFormats = dependencies.jsonParse(buffer); + } catch (error) { + throw new FileValidationError(error); + } + const organizationLearnerImportFormats = rawImportFormats.flatMap((rawImportFormat) => { try { return new OrganizationLearnerImportFormat(rawImportFormat); diff --git a/api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/ko.json b/api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/ko.json new file mode 100644 index 00000000000..0cca40160c7 --- /dev/null +++ b/api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/ko.json @@ -0,0 +1,4 @@ +[ + { "name": "FIRST_FORMAT", "fileType": "csv", "config": "toto" }, + { "name": "SECOND_FORMAT", "fileType": "unsupportedFileType", "config": { "new_config": "not_bad" } } +] diff --git a/api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/not-a-json.json b/api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/not-a-json.json new file mode 100644 index 00000000000..289edc78982 --- /dev/null +++ b/api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/not-a-json.json @@ -0,0 +1 @@ +this is not a json diff --git a/api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/ok.json b/api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/ok.json new file mode 100644 index 00000000000..87af5c7e7de --- /dev/null +++ b/api/tests/prescription/learner-management/integration/domain/usecases/test-file/import-format-file/ok.json @@ -0,0 +1 @@ +[{ "name": "FIRST_FORMAT", "fileType": "csv", "config": { "new_config": "awesome" } }] diff --git a/api/tests/prescription/learner-management/integration/domain/usecases/update-organization-import-learner-import-format_test.js b/api/tests/prescription/learner-management/integration/domain/usecases/update-organization-learner-import-formats_test.js similarity index 58% rename from api/tests/prescription/learner-management/integration/domain/usecases/update-organization-import-learner-import-format_test.js rename to api/tests/prescription/learner-management/integration/domain/usecases/update-organization-learner-import-formats_test.js index 03f885dde1e..dafc07a51dc 100644 --- a/api/tests/prescription/learner-management/integration/domain/usecases/update-organization-import-learner-import-format_test.js +++ b/api/tests/prescription/learner-management/integration/domain/usecases/update-organization-learner-import-formats_test.js @@ -1,7 +1,14 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import * as url from 'node:url'; + import { usecases } from '../../../../../../src/prescription/learner-management/domain/usecases/index.js'; -import { EntityValidationError } from '../../../../../../src/shared/domain/errors.js'; +import { EntityValidationError, FileValidationError } from '../../../../../../src/shared/domain/errors.js'; import { catchErr, databaseBuilder, expect, knex } from '../../../../../test-helper.js'; +// Get __dirname in ESM +const __dirname = url.fileURLToPath(new URL('.', import.meta.url)); + describe('Integration | Organizational Entities | Domain | UseCase | update-organization-learner-import-formats', function () { beforeEach(function () { databaseBuilder.factory.buildOrganizationLearnerImportFormat({ @@ -21,9 +28,11 @@ describe('Integration | Organizational Entities | Domain | UseCase | update-orga describe('success case', function () { it('update organization learner import format given parameter', async function () { - // given && when + // given + const payload = fs.createReadStream(path.join(__dirname, 'test-file/import-format-file', 'ok.json')); + // when await usecases.updateOrganizationLearnerImportFormats({ - rawImportFormats: [{ name: 'FIRST_FORMAT', fileType: 'csv', config: { new_config: 'awesome' } }], + payload, }); // then @@ -37,11 +46,10 @@ describe('Integration | Organizational Entities | Domain | UseCase | update-orga describe('error case', function () { it('should not update organization learner import format when error occured', async function () { // given && when + const payload = fs.createReadStream(path.join(__dirname, 'test-file/import-format-file', 'ko.json')); + const error = await catchErr(usecases.updateOrganizationLearnerImportFormats)({ - rawImportFormats: [ - { name: 'FIRST_FORMAT', fileType: 'csv', config: 'toto' }, - { name: 'SECOND_FORMAT', fileType: 'unsupportedFileType', config: { new_config: 'not_bad' } }, - ], + payload, }); // then @@ -61,5 +69,31 @@ describe('Integration | Organizational Entities | Domain | UseCase | update-orga expect(error).to.be.instanceOf(EntityValidationError); }); + + it('should throw an error when file is not a json', async function () { + // given && when + const payload = fs.createReadStream(path.join(__dirname, 'test-file/import-format-file', 'not-a-json.json')); + + const error = await catchErr(usecases.updateOrganizationLearnerImportFormats)({ + payload, + }); + + // then + const firstImportFormat = await knex('organization-learner-import-formats') + .where({ name: 'FIRST_FORMAT' }) + .first(); + + expect(firstImportFormat.fileType).to.be.equal('xml'); + expect(firstImportFormat.config).to.be.deep.equal({ basic_config: 'first_format' }); + + const secondImportFormat = await knex('organization-learner-import-formats') + .where({ name: 'SECOND_FORMAT' }) + .first(); + + expect(secondImportFormat.fileType).to.be.equal('csv'); + expect(secondImportFormat.config).to.be.deep.equal({ basic_config: 'second_format' }); + + expect(error).to.be.instanceOf(FileValidationError); + }); }); }); From 397ca2a071ed6ae68e6266db18f3b6f63f256e9d Mon Sep 17 00:00:00 2001 From: Xavier Carron <33637571+xav-car@users.noreply.github.com> Date: Fri, 27 Sep 2024 18:00:54 +0200 Subject: [PATCH 3/4] feat(api): add controller to update import format configuration --- .../organization-import-controller.js | 11 +- .../application/organization-import-route.js | 40 ++++++ .../infrastructure/ApplicationTransaction.js | 4 + api/src/shared/domain/constants.js | 35 ++--- .../organization-import-route_test.js | 32 ++++- .../organization-import-controller_test.js | 69 ++++++--- .../organization-import-route_test.js | 131 ++++++++++++------ 7 files changed, 243 insertions(+), 79 deletions(-) diff --git a/api/src/prescription/learner-management/application/organization-import-controller.js b/api/src/prescription/learner-management/application/organization-import-controller.js index f5012997135..f7ebf15334b 100644 --- a/api/src/prescription/learner-management/application/organization-import-controller.js +++ b/api/src/prescription/learner-management/application/organization-import-controller.js @@ -1,3 +1,4 @@ +import { DomainTransaction } from '../../../shared/domain/DomainTransaction.js'; import { usecases } from '../domain/usecases/index.js'; import * as organizationImportDetailSerializer from '../infrastructure/serializers/jsonapi/organization-import-detail-serializer.js'; @@ -10,6 +11,14 @@ const getOrganizationImportStatus = async function (request, h, dependencies = { return h.response(dependencies.organizationImportDetailSerializer.serialize(organizationImportDetail)).code(200); }; -const organizationImportController = { getOrganizationImportStatus }; +const updateOrganizationLearnerImportFormats = async function (request) { + await DomainTransaction.execute(async () => { + await usecases.updateOrganizationLearnerImportFormats({ payload: request.payload }); + }); + + return null; +}; + +const organizationImportController = { getOrganizationImportStatus, updateOrganizationLearnerImportFormats }; export { organizationImportController }; diff --git a/api/src/prescription/learner-management/application/organization-import-route.js b/api/src/prescription/learner-management/application/organization-import-route.js index e787cce39e2..f5086cc072b 100644 --- a/api/src/prescription/learner-management/application/organization-import-route.js +++ b/api/src/prescription/learner-management/application/organization-import-route.js @@ -1,7 +1,10 @@ import Joi from 'joi'; +import { sendJsonApiError } from '../../../shared/application/http-errors.js'; +import { PayloadTooLargeError } from '../../../shared/application/http-errors.js'; import { securityPreHandlers } from '../../../shared/application/security-pre-handlers.js'; import { ORGANIZATION_FEATURE } from '../../../shared/domain/constants.js'; +import { MAX_FILE_SIZE_UPLOAD } from '../../../shared/domain/constants.js'; import { identifiersType } from '../../../shared/domain/types/identifiers-type.js'; import { organizationImportController } from './organization-import-controller.js'; @@ -39,6 +42,43 @@ const register = async (server) => { }, }, ]); + + server.route([ + { + method: 'POST', + path: '/api/admin/import-organization-learners-format', + config: { + pre: [ + { + method: (request, h) => + securityPreHandlers.hasAtLeastOneAccessOf([securityPreHandlers.checkAdminMemberHasRoleSuperAdmin])( + request, + h, + ), + assign: 'hasAuthorizationToAccessAdminScope', + }, + ], + payload: { + maxBytes: MAX_FILE_SIZE_UPLOAD, + output: 'file', + failAction: async (_, h) => { + return sendJsonApiError( + new PayloadTooLargeError('An error occurred, payload is too large', 'PAYLOAD_TOO_LARGE', { + maxSize: 20, + }), + h, + ); + }, + }, + handler: organizationImportController.updateOrganizationLearnerImportFormats, + notes: [ + "- **Cette route est restreinte aux utilisateurs authentifiés en tant qu'administrateur de l'organisation**\n" + + "- Elle permet de mettre à jour la liste des participants de l'organisation.", + ], + tags: ['api', 'organization-learners'], + }, + }, + ]); }; const name = 'organization-import-api'; diff --git a/api/src/prescription/shared/infrastructure/ApplicationTransaction.js b/api/src/prescription/shared/infrastructure/ApplicationTransaction.js index a084c171ec8..f95898e9b95 100644 --- a/api/src/prescription/shared/infrastructure/ApplicationTransaction.js +++ b/api/src/prescription/shared/infrastructure/ApplicationTransaction.js @@ -2,6 +2,10 @@ import { knex } from '../../../../db/knex-database-connection.js'; import { DomainTransaction } from '../../../shared/domain/DomainTransaction.js'; import { asyncLocalStorage } from './utils/async-local-storage.js'; +/** + * @deprecated + * use only DomainTransaction instead + */ class ApplicationTransaction { static getConnection(domainTransaction = DomainTransaction.emptyTransaction()) { const store = asyncLocalStorage.getStore(); diff --git a/api/src/shared/domain/constants.js b/api/src/shared/domain/constants.js index f26616475e0..628ba6d4316 100644 --- a/api/src/shared/domain/constants.js +++ b/api/src/shared/domain/constants.js @@ -2,6 +2,8 @@ import { config } from '../config.js'; const LEVENSHTEIN_DISTANCE_MAX_RATE = 0.25; +const MAX_FILE_SIZE_UPLOAD = 1048576 * 20; + const LOCALE = { ENGLISH_SPOKEN: 'en', FRENCH_FRANCE: 'fr-fr', @@ -129,31 +131,31 @@ const CERTIFICATION_CENTER_TYPES = { }; const constants = { - PIX_COUNT_BY_LEVEL, + ALL_TREATMENTS, + AUTONOMOUS_COURSES_ORGANIZATION_ID, + CERTIFICATION_CENTER_TYPES, COMPETENCES_COUNT, - MAX_REACHABLE_LEVEL, - MAX_REACHABLE_PIX_SCORE, - MAX_REACHABLE_PIX_BY_COMPETENCE, - MAX_CHALLENGES_PER_COMPETENCE_FOR_CERTIFICATION, + DEFAULT_LEVEL_FOR_FIRST_CHALLENGE, MAX_CHALLENGES_PER_AREA_FOR_CERTIFICATION_PLUS, + MAX_CHALLENGES_PER_COMPETENCE_FOR_CERTIFICATION, + MAX_DIFF_BETWEEN_USER_LEVEL_AND_SKILL_LEVEL, + MAX_LEVEL_TO_BE_AN_EASY_TUBE, MAX_MASTERY_RATE, - MINIMUM_DELAY_IN_DAYS_FOR_RESET, - MINIMUM_DELAY_IN_DAYS_BEFORE_IMPROVING, - MINIMUM_DELAY_IN_DAYS_BEFORE_RETRYING, + MAX_REACHABLE_LEVEL, + MAX_REACHABLE_PIX_BY_COMPETENCE, + MAX_REACHABLE_PIX_SCORE, MINIMUM_CERTIFIABLE_COMPETENCES_FOR_CERTIFIABILITY, MINIMUM_COMPETENCE_LEVEL_FOR_CERTIFIABILITY, + MINIMUM_DELAY_IN_DAYS_BEFORE_IMPROVING, + MINIMUM_DELAY_IN_DAYS_BEFORE_RETRYING, + MINIMUM_DELAY_IN_DAYS_FOR_RESET, MINIMUM_REPRODUCIBILITY_RATE_TO_BE_CERTIFIED, MINIMUM_REPRODUCIBILITY_RATE_TO_BE_TRUSTED, - UNCERTIFIED_LEVEL, - MAX_LEVEL_TO_BE_AN_EASY_TUBE, - DEFAULT_LEVEL_FOR_FIRST_CHALLENGE, - MAX_DIFF_BETWEEN_USER_LEVEL_AND_SKILL_LEVEL, - ALL_TREATMENTS, + OIDC_ERRORS, + PIX_COUNT_BY_LEVEL, PIX_ORIGIN, STUDENT_RECONCILIATION_ERRORS, - OIDC_ERRORS, - CERTIFICATION_CENTER_TYPES, - AUTONOMOUS_COURSES_ORGANIZATION_ID, + UNCERTIFIED_LEVEL, }; export { @@ -167,6 +169,7 @@ export { MAX_CHALLENGES_PER_AREA_FOR_CERTIFICATION_PLUS, MAX_CHALLENGES_PER_COMPETENCE_FOR_CERTIFICATION, MAX_DIFF_BETWEEN_USER_LEVEL_AND_SKILL_LEVEL, + MAX_FILE_SIZE_UPLOAD, MAX_LEVEL_TO_BE_AN_EASY_TUBE, MAX_MASTERY_RATE, MAX_REACHABLE_LEVEL, diff --git a/api/tests/prescription/learner-management/acceptance/application/organization-import-route_test.js b/api/tests/prescription/learner-management/acceptance/application/organization-import-route_test.js index 68f1eb87fbc..169e8b2456c 100644 --- a/api/tests/prescription/learner-management/acceptance/application/organization-import-route_test.js +++ b/api/tests/prescription/learner-management/acceptance/application/organization-import-route_test.js @@ -1,3 +1,4 @@ +import { PIX_ADMIN } from '../../../../../src/authorization/domain/constants.js'; import { ORGANIZATION_FEATURE } from '../../../../../src/shared/domain/constants.js'; import { CsvImportError } from '../../../../../src/shared/domain/errors.js'; import { Membership } from '../../../../../src/shared/domain/models/Membership.js'; @@ -11,7 +12,7 @@ import { describe('Acceptance | Application | organization-import', function () { let server; - beforeEach(async function () { + before(async function () { server = await createServer(); }); @@ -126,4 +127,33 @@ describe('Acceptance | Application | organization-import', function () { expect(response.statusCode).to.equal(200); }); }); + + describe('POST /api/admin/import-organization-learners-format', function () { + let options, connectedUser; + + beforeEach(async function () { + connectedUser = databaseBuilder.factory.buildUser.withRole({ role: PIX_ADMIN.ROLES.SUPER_ADMIN }); + await databaseBuilder.commit(); + }); + + it('should upload file with no error', async function () { + // given + const buffer = '[{"name":"GENERIC","fileType":"csv","config":{"awesome_config": "pouet"}}]'; + await databaseBuilder.commit(); + + options = { + method: 'POST', + url: `/api/admin/import-organization-learners-format`, + headers: { + authorization: generateValidRequestAuthorizationHeader(connectedUser.id), + }, + payload: buffer, + }; + // when + const response = await server.inject(options); + + // then + expect(response.statusCode).to.equal(204); + }); + }); }); diff --git a/api/tests/prescription/learner-management/unit/application/organization-import-controller_test.js b/api/tests/prescription/learner-management/unit/application/organization-import-controller_test.js index 48528a5ef77..84c7e19b0fd 100644 --- a/api/tests/prescription/learner-management/unit/application/organization-import-controller_test.js +++ b/api/tests/prescription/learner-management/unit/application/organization-import-controller_test.js @@ -1,28 +1,59 @@ +import { DomainTransaction } from '../../../../../lib/infrastructure/DomainTransaction.js'; import { organizationImportController } from '../../../../../src/prescription/learner-management/application/organization-import-controller.js'; import { usecases } from '../../../../../src/prescription/learner-management/domain/usecases/index.js'; import { expect, hFake, sinon } from '../../../../test-helper.js'; - describe('Unit | Application | Learner Management | organization-import-controller', function () { let dependencies, serializeStub, usecaseResultSymbol; - const organizationId = 123; - const request = { - params: { organizationId }, - }; - - beforeEach(function () { - sinon.stub(usecases, 'getOrganizationImportStatus'); - usecaseResultSymbol = Symbol(); - usecases.getOrganizationImportStatus.resolves(usecaseResultSymbol); - serializeStub = sinon.stub(); - dependencies = { organizationImportDetailSerializer: { serialize: serializeStub } }; - }); - it('should get last organization import', async function () { - hFake.request = { - path: `/api/organizations/${organizationId}/import-information`, + describe('#getOrganizationImportStatus', function () { + const organizationId = 123; + const request = { + params: { organizationId }, }; - await organizationImportController.getOrganizationImportStatus(request, hFake, dependencies); - expect(usecases.getOrganizationImportStatus).to.have.been.calledOnceWithExactly({ organizationId }); - expect(serializeStub).to.have.been.calledOnceWithExactly(usecaseResultSymbol); + + beforeEach(function () { + sinon.stub(usecases, 'getOrganizationImportStatus'); + usecaseResultSymbol = Symbol(); + usecases.getOrganizationImportStatus.resolves(usecaseResultSymbol); + serializeStub = sinon.stub(); + dependencies = { organizationImportDetailSerializer: { serialize: serializeStub } }; + }); + + it('should get last organization import', async function () { + hFake.request = { + path: `/api/organizations/${organizationId}/import-information`, + }; + await organizationImportController.getOrganizationImportStatus(request, hFake, dependencies); + expect(usecases.getOrganizationImportStatus).to.have.been.calledOnceWithExactly({ organizationId }); + expect(serializeStub).to.have.been.calledOnceWithExactly(usecaseResultSymbol); + }); + }); + + describe('#updateOrganizationLearnerImportFormats', function () { + let request, payload; + + beforeEach(function () { + payload = Symbol('Payload'); + request = { + payload, + }; + + sinon.stub(DomainTransaction, 'execute'); + DomainTransaction.execute.callsFake((callback) => callback()); + + sinon.stub(usecases, 'updateOrganizationLearnerImportFormats'); + usecases.updateOrganizationLearnerImportFormats.resolves(null); + }); + + afterEach(function () { + sinon.restore(); + }); + + it('should update organization import format', async function () { + await organizationImportController.updateOrganizationLearnerImportFormats(request); + expect(usecases.updateOrganizationLearnerImportFormats).to.have.been.calledOnceWithExactly({ + payload, + }); + }); }); }); diff --git a/api/tests/prescription/learner-management/unit/application/organization-import-route_test.js b/api/tests/prescription/learner-management/unit/application/organization-import-route_test.js index d9fb874f79f..630d9dcd6eb 100644 --- a/api/tests/prescription/learner-management/unit/application/organization-import-route_test.js +++ b/api/tests/prescription/learner-management/unit/application/organization-import-route_test.js @@ -7,32 +7,32 @@ import { ORGANIZATION_FEATURE } from '../../../../../src/shared/domain/constants import { expect, HttpTestServer, sinon } from '../../../../test-helper.js'; describe('Unit | Router | organization-import-router', function () { - let checkOrganizationHasLearnerImportFeature, respondWithError; - - beforeEach(function () { - sinon.stub(securityPreHandlers, 'checkUserIsAdminInSUPOrganizationManagingStudents'); - sinon.stub(securityPreHandlers, 'checkUserIsAdminInSCOOrganizationManagingStudents'); - sinon.stub(securityPreHandlers, 'checkUserIsAdminInOrganization'); - checkOrganizationHasLearnerImportFeature = sinon.stub(); - sinon - .stub(securityPreHandlers, 'makeCheckOrganizationHasFeature') - .withArgs(ORGANIZATION_FEATURE.LEARNER_IMPORT.key) - .returns(checkOrganizationHasLearnerImportFeature); - - respondWithError = (_, h) => - h - .response( - new jsonapiSerializer.Error({ - code: 403, - title: 'Forbidden access', - detail: 'Missing or insufficient permissions.', - }), - ) - .code(403) - .takeover(); - }); - describe('GET /api/organizations/{organizationId}/import-information', function () { + let checkOrganizationHasLearnerImportFeature, respondWithError; + + beforeEach(function () { + sinon.stub(securityPreHandlers, 'checkUserIsAdminInSUPOrganizationManagingStudents'); + sinon.stub(securityPreHandlers, 'checkUserIsAdminInSCOOrganizationManagingStudents'); + sinon.stub(securityPreHandlers, 'checkUserIsAdminInOrganization'); + checkOrganizationHasLearnerImportFeature = sinon.stub(); + sinon + .stub(securityPreHandlers, 'makeCheckOrganizationHasFeature') + .withArgs(ORGANIZATION_FEATURE.LEARNER_IMPORT.key) + .returns(checkOrganizationHasLearnerImportFeature); + + respondWithError = (_, h) => + h + .response( + new jsonapiSerializer.Error({ + code: 403, + title: 'Forbidden access', + detail: 'Missing or insufficient permissions.', + }), + ) + .code(403) + .takeover(); + }); + it('should throw an error when id is invalid', async function () { // given const method = 'GET'; @@ -158,26 +158,73 @@ describe('Unit | Router | organization-import-router', function () { }); }); }); + + context( + 'when the user is not admin for the SUP organization nor SCO organizations nor has learner import feature', + function () { + it('responds 403', async function () { + checkOrganizationHasLearnerImportFeature.callsFake(respondWithError); + securityPreHandlers.checkUserIsAdminInOrganization.callsFake(respondWithError); + securityPreHandlers.checkUserIsAdminInSUPOrganizationManagingStudents.callsFake(respondWithError); + securityPreHandlers.checkUserIsAdminInSCOOrganizationManagingStudents.callsFake(respondWithError); + + const httpTestServer = new HttpTestServer(); + await httpTestServer.register(moduleUnderTest); + + const method = 'GET'; + const url = '/api/organizations/1/import-information'; + + const response = await httpTestServer.request(method, url); + + expect(response.statusCode).to.equal(403); + }); + }, + ); }); - context( - 'when the user is not admin for the SUP organization nor SCO organizations nor has learner import feature', - function () { - it('responds 403', async function () { - checkOrganizationHasLearnerImportFeature.callsFake(respondWithError); - securityPreHandlers.checkUserIsAdminInOrganization.callsFake(respondWithError); - securityPreHandlers.checkUserIsAdminInSUPOrganizationManagingStudents.callsFake(respondWithError); - securityPreHandlers.checkUserIsAdminInSCOOrganizationManagingStudents.callsFake(respondWithError); - const httpTestServer = new HttpTestServer(); - await httpTestServer.register(moduleUnderTest); + describe('POST /api/admin/import-organization-learners-format', function () { + let hasAtLeastOneAccessOfStub, updateOrganizationLearnerImportFormatsStub; - const method = 'GET'; - const url = '/api/organizations/1/import-information'; + beforeEach(function () { + hasAtLeastOneAccessOfStub = sinon + .stub(securityPreHandlers, 'hasAtLeastOneAccessOf') + .withArgs([securityPreHandlers.checkAdminMemberHasRoleSuperAdmin]); - const response = await httpTestServer.request(method, url); + updateOrganizationLearnerImportFormatsStub = sinon + .stub(organizationImportController, 'updateOrganizationLearnerImportFormats') + .resolves(null); + }); - expect(response.statusCode).to.equal(403); - }); - }, - ); + it('should not called controller when user is not super admin', async function () { + hasAtLeastOneAccessOfStub.callsFake( + () => (request, h) => + h + .response({ errors: new Error('forbidden') }) + .code(403) + .takeover(), + ); + const method = 'POST'; + const url = '/api/admin/import-organization-learners-format'; + + const httpTestServer = new HttpTestServer(moduleUnderTest); + await httpTestServer.register(moduleUnderTest); + + await httpTestServer.request(method, url); + + expect(updateOrganizationLearnerImportFormatsStub.notCalled).to.be.true; + }); + + it('should called controller when user is super admin', async function () { + hasAtLeastOneAccessOfStub.returns(() => true); + const method = 'POST'; + const url = '/api/admin/import-organization-learners-format'; + + const httpTestServer = new HttpTestServer(moduleUnderTest); + await httpTestServer.register(moduleUnderTest); + + await httpTestServer.request(method, url); + + expect(updateOrganizationLearnerImportFormatsStub.called).to.be.true; + }); + }); }); From 4e0565464cdbc53a12c19f29a99d912fddbb4d0d Mon Sep 17 00:00:00 2001 From: Xavier Carron <33637571+xav-car@users.noreply.github.com> Date: Fri, 27 Sep 2024 18:01:18 +0200 Subject: [PATCH 4/4] feat(admin): add page to update organization import format --- admin/app/adapters/campaigns-import.js | 10 -- admin/app/adapters/import-files.js | 14 +++ .../campaigns/campaigns-import.gjs | 2 +- admin/app/components/administration/nav.gjs | 4 + .../administration/organizations/index.gjs | 3 + .../update-organization-import-format.gjs | 57 +++++++++ admin/app/router.js | 1 + .../administration/organizations.hbs | 1 + admin/mirage/handlers/badge-criteria.js | 2 - .../campaigns/campaigns-import-test.gjs | 2 +- ...update-organization-import-format-test.gjs | 108 ++++++++++++++++++ .../users/campaign-participations-test.gjs | 2 +- .../tests/unit/adapters/import-files-test.js | 22 +++- admin/translations/en.json | 11 ++ admin/translations/fr.json | 11 ++ 15 files changed, 234 insertions(+), 16 deletions(-) delete mode 100644 admin/app/adapters/campaigns-import.js create mode 100644 admin/app/components/administration/organizations/index.gjs create mode 100644 admin/app/components/administration/organizations/update-organization-import-format.gjs create mode 100644 admin/app/templates/authenticated/administration/organizations.hbs create mode 100644 admin/tests/integration/components/administration/organizations/update-organization-import-format-test.gjs diff --git a/admin/app/adapters/campaigns-import.js b/admin/app/adapters/campaigns-import.js deleted file mode 100644 index d4e7976e64c..00000000000 --- a/admin/app/adapters/campaigns-import.js +++ /dev/null @@ -1,10 +0,0 @@ -import ApplicationAdapter from './application'; - -export default class CampaignsImportAdapter extends ApplicationAdapter { - addCampaignsCsv(files) { - if (!files || files.length === 0) return; - - const url = `${this.host}/${this.namespace}/admin/campaigns`; - return this.ajax(url, 'POST', { data: files[0] }); - } -} diff --git a/admin/app/adapters/import-files.js b/admin/app/adapters/import-files.js index e2a6ec4783e..eeab7d2c596 100644 --- a/admin/app/adapters/import-files.js +++ b/admin/app/adapters/import-files.js @@ -9,4 +9,18 @@ export default class ImportFilesAdapter extends ApplicationAdapter { const url = `${this.host}/${this.namespace}/campaigns/archive-campaigns`; return this.ajax(url, 'POST', { data: files[0] }); } + + updateOrganizationImportFormat(files) { + if (!files || files.length === 0) return; + + const url = `${this.host}/${this.namespace}/import-organization-learners-format`; + return this.ajax(url, 'POST', { data: files[0] }); + } + + addCampaignsCsv(files) { + if (!files || files.length === 0) return; + + const url = `${this.host}/${this.namespace}/campaigns`; + return this.ajax(url, 'POST', { data: files[0] }); + } } diff --git a/admin/app/components/administration/campaigns/campaigns-import.gjs b/admin/app/components/administration/campaigns/campaigns-import.gjs index 82051cea956..3398db471a6 100644 --- a/admin/app/components/administration/campaigns/campaigns-import.gjs +++ b/admin/app/components/administration/campaigns/campaigns-import.gjs @@ -16,7 +16,7 @@ export default class CampaignsImport extends Component { async importCampaigns(files) { this.notifications.clearAll(); - const adapter = this.store.adapterFor('campaigns-import'); + const adapter = this.store.adapterFor('import-files'); try { await adapter.addCampaignsCsv(files); this.notifications.success(this.intl.t('components.administration.campaigns-import.notifications.success')); diff --git a/admin/app/components/administration/nav.gjs b/admin/app/components/administration/nav.gjs index e955e79c4e0..3a7e0d60fac 100644 --- a/admin/app/components/administration/nav.gjs +++ b/admin/app/components/administration/nav.gjs @@ -12,6 +12,10 @@ import { t } from 'ember-intl'; {{t "pages.administration.navigation.campaigns.label"}} + + {{t "pages.administration.navigation.organizations.label"}} + + {{t "pages.administration.navigation.certification.label"}} diff --git a/admin/app/components/administration/organizations/index.gjs b/admin/app/components/administration/organizations/index.gjs new file mode 100644 index 00000000000..a6a18df4e7c --- /dev/null +++ b/admin/app/components/administration/organizations/index.gjs @@ -0,0 +1,3 @@ +import UpdateOrganizationImportFormat from './update-organization-import-format'; + + diff --git a/admin/app/components/administration/organizations/update-organization-import-format.gjs b/admin/app/components/administration/organizations/update-organization-import-format.gjs new file mode 100644 index 00000000000..c87e1e7fb1d --- /dev/null +++ b/admin/app/components/administration/organizations/update-organization-import-format.gjs @@ -0,0 +1,57 @@ +import PixButtonUpload from '@1024pix/pix-ui/components/pix-button-upload'; +import { action } from '@ember/object'; +import { service } from '@ember/service'; +import Component from '@glimmer/component'; +import { t } from 'ember-intl'; + +import AdministrationBlockLayout from '../block-layout'; + +export default class UpdateOrganizationImportFormat extends Component { + @service intl; + @service notifications; + @service router; + @service store; + + @action + async uploadOrganizationImportFile(files) { + this.notifications.clearAll(); + const adapter = this.store.adapterFor('import-files'); + try { + await adapter.updateOrganizationImportFormat(files); + this.notifications.success( + this.intl.t('components.administration.organization-import-format.notifications.success'), + ); + } catch (errorResponse) { + const errors = errorResponse.errors; + if (!errors) { + return this.notifications.error(this.intl.t('common.notifications.generic-error')); + } + + errors.forEach((error) => { + switch (error.code) { + case 'MISSING_REQUIRED_FIELD_NAMES': + this.notifications.error(`${error.meta}`, { autoClear: false }); + break; + default: + this.notifications.error(error.detail, { autoClear: false }); + } + }); + } finally { + this.isLoading = false; + } + } + +} diff --git a/admin/app/router.js b/admin/app/router.js index 243c31336f1..5ee9137d2c8 100644 --- a/admin/app/router.js +++ b/admin/app/router.js @@ -141,6 +141,7 @@ Router.map(function () { this.route('administration', function () { this.route('common'); this.route('campaigns'); + this.route('organizations'); this.route('certification'); this.route('deployment'); this.route('access'); diff --git a/admin/app/templates/authenticated/administration/organizations.hbs b/admin/app/templates/authenticated/administration/organizations.hbs new file mode 100644 index 00000000000..2eef8f4c678 --- /dev/null +++ b/admin/app/templates/authenticated/administration/organizations.hbs @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/admin/mirage/handlers/badge-criteria.js b/admin/mirage/handlers/badge-criteria.js index e5b798526ef..03f00aefb7e 100644 --- a/admin/mirage/handlers/badge-criteria.js +++ b/admin/mirage/handlers/badge-criteria.js @@ -3,8 +3,6 @@ function updateBadgeCriterion(schema, request) { const badgeCriterionId = request.params.id; const params = JSON.parse(request.requestBody); - console.log(schema); - const badgeCriterion = schema.badgeCriteria.find(badgeCriterionId); badgeCriterion.update(params.data.attributes); diff --git a/admin/tests/integration/components/administration/campaigns/campaigns-import-test.gjs b/admin/tests/integration/components/administration/campaigns/campaigns-import-test.gjs index 7463ed8c9ca..c800a9be63c 100644 --- a/admin/tests/integration/components/administration/campaigns/campaigns-import-test.gjs +++ b/admin/tests/integration/components/administration/campaigns/campaigns-import-test.gjs @@ -16,7 +16,7 @@ module('Integration | Component | administration/campaigns-import', function (h let store, adapter, notificationSuccessStub, clearAllStub, saveAdapterStub, notificationErrorStub; hooks.beforeEach(function () { store = this.owner.lookup('service:store'); - adapter = store.adapterFor('campaigns-import'); + adapter = store.adapterFor('import-files'); saveAdapterStub = sinon.stub(adapter, 'addCampaignsCsv'); notificationSuccessStub = sinon.stub(); notificationErrorStub = sinon.stub().returns(); diff --git a/admin/tests/integration/components/administration/organizations/update-organization-import-format-test.gjs b/admin/tests/integration/components/administration/organizations/update-organization-import-format-test.gjs new file mode 100644 index 00000000000..0c5622c3bb1 --- /dev/null +++ b/admin/tests/integration/components/administration/organizations/update-organization-import-format-test.gjs @@ -0,0 +1,108 @@ +import { render } from '@1024pix/ember-testing-library'; +import Service from '@ember/service'; +import { triggerEvent } from '@ember/test-helpers'; +import { setupMirage } from 'ember-cli-mirage/test-support'; +import { t } from 'ember-intl/test-support'; +import UpdateOrganizationImportFormat from 'pix-admin/components/administration/organizations/update-organization-import-format'; +import { module, test } from 'qunit'; +import sinon from 'sinon'; + +import setupIntlRenderingTest from '../../../../helpers/setup-intl-rendering'; + +module('Integration | Component | administration/update-organization-import-format', function (hooks) { + setupIntlRenderingTest(hooks); + setupMirage(hooks); + + let store, adapter, notificationSuccessStub, clearAllStub, saveAdapterStub, notificationErrorStub; + hooks.beforeEach(function () { + store = this.owner.lookup('service:store'); + adapter = store.adapterFor('import-files'); + saveAdapterStub = sinon.stub(adapter, 'updateOrganizationImportFormat'); + notificationSuccessStub = sinon.stub(); + notificationErrorStub = sinon.stub().returns(); + + clearAllStub = sinon.stub(); + }); + + module('when import succeeds', function () { + test('it displays a success notification', async function (assert) { + // given + const files = Symbol('file'); + class NotificationsStub extends Service { + success = notificationSuccessStub; + error = notificationErrorStub; + clearAll = clearAllStub; + } + this.owner.register('service:notifications', NotificationsStub); + saveAdapterStub.withArgs([files]).resolves(); + + // when + const screen = await render(); + const input = await screen.findByLabelText( + t('components.administration.organization-import-format.upload-button'), + ); + await triggerEvent(input, 'change', { files: [files] }); + + // then + assert.ok(true); + assert.ok(notificationErrorStub.notCalled); + assert.ok( + notificationSuccessStub.calledWith( + t('components.administration.organization-import-format.notifications.success'), + ), + ); + }); + }); + + module('when import fails', function () { + test('it displays a specific error notification on missing required field', async function (assert) { + // given + const files = Symbol('file'); + class NotificationsStub extends Service { + error = notificationErrorStub; + success = notificationSuccessStub; + clearAll = clearAllStub; + } + saveAdapterStub.withArgs([files]).rejects({ + errors: [{ status: '422', meta: 'POUET', code: 'MISSING_REQUIRED_FIELD_NAMES' }], + }); + this.owner.register('service:notifications', NotificationsStub); + + // when + const screen = await render(); + const input = await screen.findByLabelText( + t('components.administration.organization-import-format.upload-button'), + ); + await triggerEvent(input, 'change', { files: [files] }); + + // then + assert.ok(notificationSuccessStub.notCalled); + assert.ok(notificationErrorStub.calledWithExactly('POUET', { autoClear: false })); + }); + + test('it displays an error notification', async function (assert) { + // given + const files = Symbol('file'); + class NotificationsStub extends Service { + error = notificationErrorStub; + success = notificationSuccessStub; + clearAll = clearAllStub; + } + saveAdapterStub.withArgs([files]).rejects({ + errors: [{ status: '422', title: "Un soucis avec l'import", code: '422', detail: 'Erreur d’import' }], + }); + this.owner.register('service:notifications', NotificationsStub); + + // when + const screen = await render(); + const input = await screen.findByLabelText( + t('components.administration.organization-import-format.upload-button'), + ); + await triggerEvent(input, 'change', { files: [files] }); + + // then + assert.ok(notificationSuccessStub.notCalled); + assert.ok(notificationErrorStub.called); + }); + }); +}); diff --git a/admin/tests/integration/components/users/campaign-participations-test.gjs b/admin/tests/integration/components/users/campaign-participations-test.gjs index 6aa89ff0407..9d1d49c5b18 100644 --- a/admin/tests/integration/components/users/campaign-participations-test.gjs +++ b/admin/tests/integration/components/users/campaign-participations-test.gjs @@ -62,7 +62,7 @@ module('Integration | Component | users | campaign-participation', function (hoo assert.dom(screen.getByRole('link', { name: 'SOMECODE' })).exists(); }); - test('it should display orgnaization learner information', async function (assert) { + test('it should display organization learner information', async function (assert) { // given const participation = EmberObject.create({ organizationLearnerFullName: 'Un nom bien long', diff --git a/admin/tests/unit/adapters/import-files-test.js b/admin/tests/unit/adapters/import-files-test.js index c82ea84a052..55393af4d0c 100644 --- a/admin/tests/unit/adapters/import-files-test.js +++ b/admin/tests/unit/adapters/import-files-test.js @@ -12,7 +12,7 @@ module('Unit | Adapter | ImportFiles', function (hooks) { }); module('#importCampaignsToArchive', function () { - test('should build importCampaignsToArchive url from organizationId', async function (assert) { + test('should build importCampaignsToArchive url', async function (assert) { // when await adapter.importCampaignsToArchive([Symbol()]); @@ -20,4 +20,24 @@ module('Unit | Adapter | ImportFiles', function (hooks) { assert.ok(adapter.ajax.calledWith('http://localhost:3000/api/admin/campaigns/archive-campaigns', 'POST')); }); }); + + module('#updateOrganizationImportFormat', function () { + test('should build updateOrganizationImportFormat url', async function (assert) { + // when + await adapter.updateOrganizationImportFormat([Symbol()]); + + // then + assert.ok(adapter.ajax.calledWith('http://localhost:3000/api/admin/import-organization-learners-format', 'POST')); + }); + }); + + module('#addCampaignsCsv', function () { + test('should build addCampaignsCsv url', async function (assert) { + // when + await adapter.addCampaignsCsv([Symbol()]); + + // then + assert.ok(adapter.ajax.calledWith('http://localhost:3000/api/admin/campaigns', 'POST')); + }); + }); }); diff --git a/admin/translations/en.json b/admin/translations/en.json index e80a03e076b..0e4001ddb73 100644 --- a/admin/translations/en.json +++ b/admin/translations/en.json @@ -68,6 +68,14 @@ }, "upload-button": "Import JSON file" }, + "organization-import-format": { + "title": "Update existing import formats", + "description": "Only existing format imports will be updated. If the name does not match, no format import will be created.", + "notifications": { + "success": "Format imports are updated correctly" + }, + "upload-button": "Importing a JSON file" + }, "organization-tags-import": { "title": "Bulk addition of tags on organisations", "description": "Allow to add tags to organisations. Each line of the CSV file must be made of the ID of an organisation, followed by the name of a tag.", @@ -407,6 +415,9 @@ }, "deployment": { "label": "Deployment" + }, + "organizations": { + "label": "Organizations" } } }, diff --git a/admin/translations/fr.json b/admin/translations/fr.json index 24b625b7949..eee1c067d37 100644 --- a/admin/translations/fr.json +++ b/admin/translations/fr.json @@ -76,6 +76,14 @@ }, "upload-button": "Importer un fichier JSON" }, + "organization-import-format": { + "title": "Mise à jour d'import à format existants", + "description": "Seul les imports à format existants seront mis à jour. Dans le cas où le nom ne correspond pas, il n'y aura pas de création d'import à format.", + "notifications": { + "success": "Les imports à formats sont bien mis à jour" + }, + "upload-button": "Importer un fichier JSON" + }, "organization-tags-import": { "title": "Ajout de tags en masse sur des organisations", "description": "Permet d’ajouter des tags à des organisations. Chaque ligne du fichier CSV doit être constituée de l’ID d’une organisation, suivie du nom d’un tag.", @@ -417,6 +425,9 @@ }, "deployment": { "label": "Déploiement" + }, + "organizations": { + "label": "Organisations" } } },