From ba800b8394d32319c6d916dfede0f1bf3488805a Mon Sep 17 00:00:00 2001 From: Xavier Carron <33637571+xav-car@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:56:03 +0100 Subject: [PATCH 1/5] feat(api): add job repository/controller to insert job/trigger job --- ...up-organization-learners-job-controller.js | 38 +++++++++ .../ImportSupOrganizationLearnersJob.js | 7 ++ .../domain/usecases/index.js | 36 ++++---- ...up-organization-learners-job-repository.js | 18 ++++ ...ganization-learners-job-repository_test.js | 27 ++++++ ...ganization-learners-job-controller_test.js | 82 +++++++++++++++++++ 6 files changed, 193 insertions(+), 15 deletions(-) create mode 100644 api/src/prescription/learner-management/application/jobs/import-sup-organization-learners-job-controller.js create mode 100644 api/src/prescription/learner-management/domain/models/ImportSupOrganizationLearnersJob.js create mode 100644 api/src/prescription/learner-management/infrastructure/repositories/jobs/import-sup-organization-learners-job-repository.js create mode 100644 api/tests/prescription/learner-management/integration/infrastructure/repositories/jobs/import-sup-organization-learners-job-repository_test.js create mode 100644 api/tests/prescription/learner-management/unit/application/jobs/import-sup-organization-learners-job-controller_test.js diff --git a/api/src/prescription/learner-management/application/jobs/import-sup-organization-learners-job-controller.js b/api/src/prescription/learner-management/application/jobs/import-sup-organization-learners-job-controller.js new file mode 100644 index 00000000000..8dd20f2aae7 --- /dev/null +++ b/api/src/prescription/learner-management/application/jobs/import-sup-organization-learners-job-controller.js @@ -0,0 +1,38 @@ +import { JobController } from '../../../../shared/application/jobs/job-controller.js'; +import { config } from '../../../../shared/config.js'; +import { DomainError } from '../../../../shared/domain/errors.js'; +import { getI18n } from '../../../../shared/infrastructure/i18n/i18n.js'; +import { logger as l } from '../../../../shared/infrastructure/utils/logger.js'; +import { ImportSupOrganizationLearnersJob } from '../../domain/models/ImportSupOrganizationLearnersJob.js'; +import { usecases } from '../../domain/usecases/index.js'; + +class ImportSupOrganizationLearnersJobController extends JobController { + #logger; + + constructor({ logger = l } = {}) { + super(ImportSupOrganizationLearnersJob.name); + this.#logger = logger; + } + + get isJobEnabled() { + return config.pgBoss.importFileJobEnabled; + } + + async handle({ data }) { + const { organizationImportId, locale } = data; + + try { + return await await usecases.importSupOrganizationLearners({ + organizationImportId, + i18n: getI18n(locale), + }); + } catch (err) { + if (!(err instanceof DomainError)) { + throw err; + } + this.#logger.error(err); + } + } +} + +export { ImportSupOrganizationLearnersJobController }; diff --git a/api/src/prescription/learner-management/domain/models/ImportSupOrganizationLearnersJob.js b/api/src/prescription/learner-management/domain/models/ImportSupOrganizationLearnersJob.js new file mode 100644 index 00000000000..99f1e17cd6b --- /dev/null +++ b/api/src/prescription/learner-management/domain/models/ImportSupOrganizationLearnersJob.js @@ -0,0 +1,7 @@ +export class ImportSupOrganizationLearnersJob { + constructor({ organizationImportId, type, locale }) { + this.organizationImportId = organizationImportId; + this.type = type; + this.locale = locale; + } +} diff --git a/api/src/prescription/learner-management/domain/usecases/index.js b/api/src/prescription/learner-management/domain/usecases/index.js index 562d29b8b81..78089168d5b 100644 --- a/api/src/prescription/learner-management/domain/usecases/index.js +++ b/api/src/prescription/learner-management/domain/usecases/index.js @@ -13,6 +13,7 @@ import * as membershipRepository from '../../../../team/infrastructure/repositor import * as campaignParticipationRepository from '../../infrastructure/repositories/campaign-participation-repository.js'; import { repositories } from '../../infrastructure/repositories/index.js'; import { importOrganizationLearnersJobRepository } from '../../infrastructure/repositories/jobs/import-organization-learners-job-repository.js'; +import { importSupOrganizationLearnersJobRepository } from '../../infrastructure/repositories/jobs/import-sup-organization-learners-job-repository.js'; import { validateOrganizationImportFileJobRepository } from '../../infrastructure/repositories/jobs/validate-organization-learners-import-file-job-repository.js'; import * as organizationImportRepository from '../../infrastructure/repositories/organization-import-repository.js'; import * as organizationLearnerImportFormatRepository from '../../infrastructure/repositories/organization-learner-import-format-repository.js'; @@ -21,37 +22,42 @@ import * as supOrganizationLearnerRepository from '../../infrastructure/reposito import { importStorage } from '../../infrastructure/storage/import-storage.js'; /** - * @typedef {import ('../../../../../lib/infrastructure/repositories/campaign-repository.js')} CampaignRepository * @typedef {import ('../../infrastructure/repositories/organization-feature-repository.js')} CampaignParticipationRepository + * @typedef {import ('../../../../../lib/infrastructure/repositories/campaign-repository.js')} CampaignRepository + * @typedef {import ('../../infrastructure/repositories/jobs/import-organization-learners-job-repository.js')} ImportOrganizationLearnersJobRepository * @typedef {import ('../../infrastructure/storage/import-storage.js')} ImportStorage + * @typedef {import ('../../infrastructure/repositories/jobs/import-sup-organization-learners-job-repository.js')} ImportSupOrganizationLearnersJobRepository + * @typedef {import ('../../../../shared/infrastructure/monitoring-tools.js')} LogErrorWithCorrelationIds + * @typedef {import ('../../../../shared/infrastructure/utils/logger.js')} Loggger * @typedef {import ('../../../../team/infrastructure/repositories/membership-repository.js')} MembershipRepository - * @typedef {import ('../../infrastructure/repositories/organization-learner-repository.js')} OrganizationLearnerRepository + * @typedef {import ('../../../../organizational-entities/application/api/organization-features-api.js')} OrganizationFeatureApi + * @typedef {import ('../../infrastructure/repositories/organization-feature-repository.js')} OrganizationFeatureRepository + * @typedef {import ('../../infrastructure/repositories/organization-import-repository.js')} OrganizationImportRepository * @typedef {import ('../../infrastructure/repositories/organization-learner-import-format-repository.js')} OrganizationLearnerImportFormatRepository + * @typedef {import ('../../infrastructure/repositories/organization-learner-repository.js')} OrganizationLearnerRepository * @typedef {import ('../../../../shared/infrastructure/repositories/organization-repository.js')} OrganizationRepository - * @typedef {import ('../../infrastructure/repositories/organization-import-repository.js')} OrganizationImportRepository * @typedef {import ('../../infrastructure/repositories/sup-organization-learner-repository.js')} SupOrganizationLearnerRepository - * @typedef {import ('../../../../organizational-entities/application/api/organization-features-api.js')} OrganizationFeatureApi - * @typedef {import ('../../../../../src/shared/infrastructure/utils/logger.js')} logger * @typedef {import ('../../../../../lib/domain/services/user-reconciliation-service.js')} UserReconciliationService - * @typedef {import ('../../infrastructure/repositories/organization-feature-repository.js')} OrganizationFeatureRepository + * @typedef {import ('../../infrastructure/repositories/jobs/validate-organization-learners-import-file-job-repository.js')} ValidateOrganizationImportFileJobRepository */ const dependencies = { - campaignRepository, campaignParticipationRepository, + campaignRepository, + importOrganizationLearnersJobRepository, importStorage, + importSupOrganizationLearnersJobRepository, + logErrorWithCorrelationIds, + logger, membershipRepository, - organizationLearnerRepository, + organizationFeatureApi, + organizationFeatureRepository: repositories.organizationFeatureRepository, + organizationImportRepository, organizationLearnerImportFormatRepository, + organizationLearnerRepository, organizationRepository, - importOrganizationLearnersJobRepository, - validateOrganizationImportFileJobRepository, - organizationImportRepository, supOrganizationLearnerRepository, - organizationFeatureApi, - logErrorWithCorrelationIds, userReconciliationService, - organizationFeatureRepository: repositories.organizationFeatureRepository, - logger, + validateOrganizationImportFileJobRepository, }; const path = dirname(fileURLToPath(import.meta.url)); diff --git a/api/src/prescription/learner-management/infrastructure/repositories/jobs/import-sup-organization-learners-job-repository.js b/api/src/prescription/learner-management/infrastructure/repositories/jobs/import-sup-organization-learners-job-repository.js new file mode 100644 index 00000000000..a7bb1294c54 --- /dev/null +++ b/api/src/prescription/learner-management/infrastructure/repositories/jobs/import-sup-organization-learners-job-repository.js @@ -0,0 +1,18 @@ +import { + JobExpireIn, + JobRepository, + JobRetry, +} from '../../../../../shared/infrastructure/repositories/jobs/job-repository.js'; +import { ImportSupOrganizationLearnersJob } from '../../../domain/models/ImportSupOrganizationLearnersJob.js'; + +class ImportSupOrganizationLearnersJobRepository extends JobRepository { + constructor() { + super({ + name: ImportSupOrganizationLearnersJob.name, + expireIn: JobExpireIn.HIGH, + retry: JobRetry.FEW_RETRY, + }); + } +} + +export const importSupOrganizationLearnersJobRepository = new ImportSupOrganizationLearnersJobRepository(); diff --git a/api/tests/prescription/learner-management/integration/infrastructure/repositories/jobs/import-sup-organization-learners-job-repository_test.js b/api/tests/prescription/learner-management/integration/infrastructure/repositories/jobs/import-sup-organization-learners-job-repository_test.js new file mode 100644 index 00000000000..2f47219ec32 --- /dev/null +++ b/api/tests/prescription/learner-management/integration/infrastructure/repositories/jobs/import-sup-organization-learners-job-repository_test.js @@ -0,0 +1,27 @@ +import { ImportSupOrganizationLearnersJob } from '../../../../../../../src/prescription/learner-management/domain/models/ImportSupOrganizationLearnersJob.js'; +import { importSupOrganizationLearnersJobRepository } from '../../../../../../../src/prescription/learner-management/infrastructure/repositories/jobs/import-sup-organization-learners-job-repository.js'; +import { + JobExpireIn, + JobRetry, +} from '../../../../../../../src/shared/infrastructure/repositories/jobs/job-repository.js'; +import { expect } from '../../../../../../test-helper.js'; + +describe('Integration | Prescription | Infrastructure | Repository | Jobs | importSupOrganizationLearnersJobRepository', function () { + describe('#performAsync', function () { + it('publish a job', async function () { + // when + await importSupOrganizationLearnersJobRepository.performAsync( + new ImportSupOrganizationLearnersJob({ organizationImportId: 4123132, type: 'REPLACE', locale: 'fr' }), + ); + + // then + await expect(ImportSupOrganizationLearnersJob.name).to.have.have.been.performed.withJob({ + expirein: JobExpireIn.HIGH, + retrylimit: JobRetry.FEW_RETRY.retryLimit, + retrydelay: JobRetry.FEW_RETRY.retryDelay, + retrybackoff: JobRetry.FEW_RETRY.retryBackoff, + data: { organizationImportId: 4123132, type: 'REPLACE', locale: 'fr' }, + }); + }); + }); +}); diff --git a/api/tests/prescription/learner-management/unit/application/jobs/import-sup-organization-learners-job-controller_test.js b/api/tests/prescription/learner-management/unit/application/jobs/import-sup-organization-learners-job-controller_test.js new file mode 100644 index 00000000000..e9082de5a3a --- /dev/null +++ b/api/tests/prescription/learner-management/unit/application/jobs/import-sup-organization-learners-job-controller_test.js @@ -0,0 +1,82 @@ +import { ImportSupOrganizationLearnersJobController } from '../../../../../../src/prescription/learner-management/application/jobs/import-sup-organization-learners-job-controller.js'; +import { usecases } from '../../../../../../src/prescription/learner-management/domain/usecases/index.js'; +import { config } from '../../../../../../src/shared/config.js'; +import { OrganizationLearnersCouldNotBeSavedError } from '../../../../../../src/shared/domain/errors.js'; +import { getI18n } from '../../../../../../src/shared/infrastructure/i18n/i18n.js'; +import { catchErr, expect, sinon } from '../../../../../test-helper.js'; + +describe('Unit | Prescription | Application | Jobs | importSupOrganizationLearnersJobController', function () { + describe('#isJobEnabled', function () { + it('return true when job is enabled', function () { + //given + sinon.stub(config.pgBoss, 'importFileJobEnabled').value(true); + + // when + const handler = new ImportSupOrganizationLearnersJobController(); + + // then + expect(handler.isJobEnabled).to.be.true; + }); + + it('return false when job is disabled', function () { + //given + sinon.stub(config.pgBoss, 'importFileJobEnabled').value(false); + + //when + const handler = new ImportSupOrganizationLearnersJobController(); + + //then + expect(handler.isJobEnabled).to.be.false; + }); + }); + + describe('#handle', function () { + it('should call usecase', async function () { + sinon.stub(usecases, 'importSupOrganizationLearners'); + + // given + const handler = new ImportSupOrganizationLearnersJobController(); + const data = { organizationImportId: Symbol('organizationImportId'), locale: 'en' }; + + // when + await handler.handle({ data }); + + // then + expect(usecases.importSupOrganizationLearners).to.have.been.calledOnce; + expect(usecases.importSupOrganizationLearners).to.have.been.calledWithExactly({ + organizationImportId: data.organizationImportId, + i18n: getI18n(data.locale), + }); + }); + + it('should not throw when error is from domain', async function () { + const error = new OrganizationLearnersCouldNotBeSavedError(); + sinon.stub(usecases, 'importSupOrganizationLearners').rejects(error); + + // given + const errorStub = sinon.stub(); + const handler = new ImportSupOrganizationLearnersJobController({ logger: { error: errorStub } }); + const data = { organizationImportId: Symbol('organizationImportId') }; + + // when & then + await handler.handle({ data }); + + expect(errorStub).to.have.been.calledWithExactly(error); + }); + + it('should throw when error is not from domain', async function () { + const error = new Error(); + sinon.stub(usecases, 'importSupOrganizationLearners').rejects(error); + + // given + const handler = new ImportSupOrganizationLearnersJobController(); + const data = { organizationImportId: Symbol('organizationImportId') }; + + // when + const result = await catchErr(handler.handle)({ data }); + + // then + expect(result).to.equal(error); + }); + }); +}); From 41757bfbf03dd7d59edc212dec25cf5b9c516ff6 Mon Sep 17 00:00:00 2001 From: Xavier Carron <33637571+xav-car@users.noreply.github.com> Date: Mon, 18 Nov 2024 15:17:30 +0100 Subject: [PATCH 2/5] tech?(api): use DomainTransaction insteadof ApplicationTransaction --- .../repositories/organization-import-repository.js | 3 +-- .../repositories/organization-import-repository_test.js | 9 ++++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/api/src/prescription/learner-management/infrastructure/repositories/organization-import-repository.js b/api/src/prescription/learner-management/infrastructure/repositories/organization-import-repository.js index adb141fc303..4e40d688437 100644 --- a/api/src/prescription/learner-management/infrastructure/repositories/organization-import-repository.js +++ b/api/src/prescription/learner-management/infrastructure/repositories/organization-import-repository.js @@ -1,6 +1,5 @@ import { knex } from '../../../../../db/knex-database-connection.js'; import { DomainTransaction } from '../../../../shared/domain/DomainTransaction.js'; -import { ApplicationTransaction } from '../../../shared/infrastructure/ApplicationTransaction.js'; import { OrganizationImport } from '../../domain/models/OrganizationImport.js'; import { OrganizationImportDetail } from '../../domain/read-models/OrganizationImportDetail.js'; @@ -31,7 +30,7 @@ const getLastImportDetailForOrganization = async function (organizationId) { }; const get = async function (id) { - const knexConn = ApplicationTransaction.getConnection(); + const knexConn = DomainTransaction.getConnection(); const result = await knexConn('organization-imports').where({ id }).first(); if (!result) return null; diff --git a/api/tests/prescription/learner-management/integration/infrastructure/repositories/organization-import-repository_test.js b/api/tests/prescription/learner-management/integration/infrastructure/repositories/organization-import-repository_test.js index f41ce98dca0..d8f1d0dfa33 100644 --- a/api/tests/prescription/learner-management/integration/infrastructure/repositories/organization-import-repository_test.js +++ b/api/tests/prescription/learner-management/integration/infrastructure/repositories/organization-import-repository_test.js @@ -1,7 +1,6 @@ import { IMPORT_STATUSES } from '../../../../../../src/prescription/learner-management/domain/constants.js'; import { OrganizationImport } from '../../../../../../src/prescription/learner-management/domain/models/OrganizationImport.js'; import * as organizationImportRepository from '../../../../../../src/prescription/learner-management/infrastructure/repositories/organization-import-repository.js'; -import { ApplicationTransaction } from '../../../../../../src/prescription/shared/infrastructure/ApplicationTransaction.js'; import { DomainTransaction } from '../../../../../../src/shared/domain/DomainTransaction.js'; import { catchErr, databaseBuilder, expect, sinon } from '../../../../../test-helper.js'; @@ -87,13 +86,13 @@ describe('Integration | Repository | Organization Learner Management | Organizat const expectedResult = databaseBuilder.factory.buildOrganizationImport(); await databaseBuilder.commit(); - const originalImp = ApplicationTransaction.getConnection; - sinon.stub(ApplicationTransaction, 'getConnection'); - ApplicationTransaction.getConnection.callsFake(originalImp); + const originalImp = DomainTransaction.getConnection; + sinon.stub(DomainTransaction, 'getConnection'); + DomainTransaction.getConnection.callsFake(originalImp); await organizationImportRepository.get(expectedResult.organizationId); - expect(ApplicationTransaction.getConnection).to.have.been.called; + expect(DomainTransaction.getConnection).to.have.been.called; }); }); From f10713a20c026381cf33beec29eb0bd14681121e Mon Sep 17 00:00:00 2001 From: Xavier Carron <33637571+xav-car@users.noreply.github.com> Date: Mon, 18 Nov 2024 16:26:05 +0100 Subject: [PATCH 3/5] feat(api): add job on validation case import student --- .../sup-organization-management-controller.js | 6 +- .../import-sup-organization-learners.js | 6 +- .../domain/usecases/validate-csv-file.js | 25 ++++++- ...organization-management-controller_test.js | 15 +--- .../import-sup-organization-learners_test.js | 22 +++--- .../domain/usecases/validate-csv-file_test.js | 68 ++++++++++++++++++- 6 files changed, 108 insertions(+), 34 deletions(-) diff --git a/api/src/prescription/learner-management/application/sup-organization-management-controller.js b/api/src/prescription/learner-management/application/sup-organization-management-controller.js index a5f884845bd..5a930a16ce6 100644 --- a/api/src/prescription/learner-management/application/sup-organization-management-controller.js +++ b/api/src/prescription/learner-management/application/sup-organization-management-controller.js @@ -28,10 +28,8 @@ const importSupOrganizationLearners = async function ( Parser: SupOrganizationLearnerParser, organizationId, i18n: request.i18n, - }); - await usecases.importSupOrganizationLearners({ - organizationId, - i18n: request.i18n, + type: 'ADDITIONNAL_STUDENT', + performJob: true, }); } finally { try { diff --git a/api/src/prescription/learner-management/domain/usecases/import-sup-organization-learners.js b/api/src/prescription/learner-management/domain/usecases/import-sup-organization-learners.js index 4351bb0a750..596b7e607b5 100644 --- a/api/src/prescription/learner-management/domain/usecases/import-sup-organization-learners.js +++ b/api/src/prescription/learner-management/domain/usecases/import-sup-organization-learners.js @@ -3,13 +3,13 @@ import { getDataBuffer } from '../../infrastructure/utils/bufferize/get-data-buf import { AggregateImportError } from '../errors.js'; const importSupOrganizationLearners = async function ({ - organizationId, + organizationImportId, i18n, supOrganizationLearnerRepository, organizationImportRepository, importStorage, }) { - const organizationImport = await organizationImportRepository.getLastByOrganizationId(organizationId); + const organizationImport = await organizationImportRepository.get(organizationImportId); const errors = []; // Reading File @@ -17,7 +17,7 @@ const importSupOrganizationLearners = async function ({ const readableStream = await importStorage.readFile({ filename: organizationImport.filename }); const buffer = await getDataBuffer(readableStream); - const parser = SupOrganizationLearnerParser.buildParser(buffer, organizationId, i18n); + const parser = SupOrganizationLearnerParser.buildParser(buffer, organizationImport.organizationId, i18n); const { learners } = parser.parse(parser.getFileEncoding()); diff --git a/api/src/prescription/learner-management/domain/usecases/validate-csv-file.js b/api/src/prescription/learner-management/domain/usecases/validate-csv-file.js index 247e434c461..6192e7827dc 100644 --- a/api/src/prescription/learner-management/domain/usecases/validate-csv-file.js +++ b/api/src/prescription/learner-management/domain/usecases/validate-csv-file.js @@ -1,6 +1,16 @@ import { AggregateImportError } from '../errors.js'; +import { ImportSupOrganizationLearnersJob } from '../models/ImportSupOrganizationLearnersJob.js'; -const validateCsvFile = async function ({ Parser, organizationId, i18n, organizationImportRepository, importStorage }) { +const validateCsvFile = async function ({ + Parser, + organizationId, + i18n, + type, + performJob, + importSupOrganizationLearnersJobRepository, + organizationImportRepository, + importStorage, +}) { const organizationImport = await organizationImportRepository.getLastByOrganizationId(organizationId); const errors = []; let warningsData; @@ -15,12 +25,25 @@ const validateCsvFile = async function ({ Parser, organizationId, i18n, organiza const { warnings } = parser.parse(parser.getFileEncoding()); warningsData = warnings; + + if (performJob) { + await importSupOrganizationLearnersJobRepository.performAsync( + new ImportSupOrganizationLearnersJob({ + organizationImportId: organizationImport.id, + type, + locale: i18n.getLocale(), + }), + ); + } } catch (error) { if (error instanceof AggregateImportError) { errors.push(...error.meta); } else { errors.push(error); } + + await importStorage.deleteFile({ filename: organizationImport.filename }); + throw error; } finally { organizationImport.validate({ errors, warnings: warningsData }); diff --git a/api/tests/prescription/learner-management/unit/application/sup-organization-management-controller_test.js b/api/tests/prescription/learner-management/unit/application/sup-organization-management-controller_test.js index fee83ffbce2..05002c133a3 100644 --- a/api/tests/prescription/learner-management/unit/application/sup-organization-management-controller_test.js +++ b/api/tests/prescription/learner-management/unit/application/sup-organization-management-controller_test.js @@ -23,7 +23,6 @@ describe('Unit | Controller | sup-organization-management-controller', function sinon.stub(usecases, 'uploadCsvFile'); sinon.stub(usecases, 'validateCsvFile'); - sinon.stub(usecases, 'importSupOrganizationLearners'); sinon.stub(usecases, 'replaceSupOrganizationLearners'); supOrganizationLearnerWarningSerializerStub = { serialize: sinon.stub() }; logErrorWithCorrelationIdsStub = sinon.stub(); @@ -41,12 +40,6 @@ describe('Unit | Controller | sup-organization-management-controller', function }; usecases.uploadCsvFile.withArgs({ userId, organizationId, payload: request.payload, i18n }).resolves(); usecases.validateCsvFile.withArgs({ organizationId, i18n }).resolves(); - usecases.importSupOrganizationLearners - .withArgs({ - organizationId, - i18n, - }) - .resolves(); supOrganizationLearnerWarningSerializerStub.serialize .withArgs({ id: organizationId, warnings }) @@ -60,13 +53,7 @@ describe('Unit | Controller | sup-organization-management-controller', function }); // then - expect( - sinon.assert.callOrder( - usecases.uploadCsvFile, - usecases.validateCsvFile, - usecases.importSupOrganizationLearners, - ), - ).to.not.throws; + expect(sinon.assert.callOrder(usecases.uploadCsvFile, usecases.validateCsvFile)).to.not.throws; expect(response.statusCode).to.be.equal(204); expect(unlinkStub).to.have.been.calledWith(path); diff --git a/api/tests/prescription/learner-management/unit/domain/usecases/import-sup-organization-learners_test.js b/api/tests/prescription/learner-management/unit/domain/usecases/import-sup-organization-learners_test.js index cac169653e9..d7d69481888 100644 --- a/api/tests/prescription/learner-management/unit/domain/usecases/import-sup-organization-learners_test.js +++ b/api/tests/prescription/learner-management/unit/domain/usecases/import-sup-organization-learners_test.js @@ -11,10 +11,12 @@ const supOrganizationLearnerImportHeader = new SupOrganizationLearnerImportHeade .join(';'); describe('Unit | UseCase | ImportSupOrganizationLearner', function () { - const organizationId = 1234; + let organizationImportId, organizationId; let organizationImport, organizationImportRepositoryStub, supOrganizationLearnerRepositoryStub, importStorageStub; beforeEach(function () { + organizationImportId = Symbol('organizationImportId'); + organizationId = 123; organizationImport = new OrganizationImport({ filename: 'file.csv', organizationId, @@ -25,7 +27,7 @@ describe('Unit | UseCase | ImportSupOrganizationLearner', function () { supOrganizationLearnerRepositoryStub = { addStudents: sinon.stub().resolves() }; organizationImportRepositoryStub = { - getLastByOrganizationId: sinon.stub(), + get: sinon.stub(), save: sinon.stub(), }; @@ -41,11 +43,11 @@ describe('Unit | UseCase | ImportSupOrganizationLearner', function () { Beatrix;The;Bride;Kiddo;Black Mamba;01/01/1970;thebride@example.net;12346;Assassination Squad;Hattori Hanzo;Deadly Viper Assassination Squad;Master;hello darkness my old friend; O-Ren;;;Ishii;Cottonmouth;01/01/1980;ishii@example.net;789;Assassination Squad;Bill;Deadly Viper Assassination Squad;DUT;; `.trim(); - organizationImportRepositoryStub.getLastByOrganizationId.withArgs(organizationId).resolves(organizationImport); + organizationImportRepositoryStub.get.withArgs(organizationImportId).resolves(organizationImport); importStorageStub.readFile.withArgs({ filename: organizationImport.filename }).resolves(toStream(input)); await importSupOrganizationLearners({ - organizationId, + organizationImportId, i18n, supOrganizationLearnerRepository: supOrganizationLearnerRepositoryStub, organizationImportRepository: organizationImportRepositoryStub, @@ -92,11 +94,11 @@ describe('Unit | UseCase | ImportSupOrganizationLearner', function () { const input = `${supOrganizationLearnerImportHeader} Beatrix;The;Bride;Kiddo;Black Mamba;01/01/1970;thebride@example.net;123456; `.trim(); - organizationImportRepositoryStub.getLastByOrganizationId.withArgs(organizationId).resolves(organizationImport); + organizationImportRepositoryStub.get.withArgs(organizationImportId).resolves(organizationImport); importStorageStub.readFile.withArgs({ filename: organizationImport.filename }).resolves(toStream(input)); await importSupOrganizationLearners({ - organizationId, + organizationImportId, i18n, importStorage: importStorageStub, supOrganizationLearnerRepository: supOrganizationLearnerRepositoryStub, @@ -111,7 +113,7 @@ describe('Unit | UseCase | ImportSupOrganizationLearner', function () { describe('success case', function () { it('should save imported state', async function () { // given - organizationImportRepositoryStub.getLastByOrganizationId.withArgs(organizationId).resolves(organizationImport); + organizationImportRepositoryStub.get.withArgs(organizationImportId).resolves(organizationImport); const csvContent = `${supOrganizationLearnerImportHeader} Beatrix;The;Bride;Kiddo;Black Mamba;01/01/1970;thebride@example.net;123456;Assassination Squad;Hattori Hanzo;Deadly Viper Assassination Squad;BAD;BAD; @@ -121,7 +123,7 @@ describe('Unit | UseCase | ImportSupOrganizationLearner', function () { // when await importSupOrganizationLearners({ - organizationId, + organizationImportId, i18n, supOrganizationLearnerRepository: supOrganizationLearnerRepositoryStub, organizationImportRepository: organizationImportRepositoryStub, @@ -135,7 +137,7 @@ describe('Unit | UseCase | ImportSupOrganizationLearner', function () { describe('errors case', function () { beforeEach(function () { - organizationImportRepositoryStub.getLastByOrganizationId.withArgs(organizationId).resolves(organizationImport); + organizationImportRepositoryStub.get.withArgs(organizationImportId).resolves(organizationImport); const csvContent = `${supOrganizationLearnerImportHeader} Beatrix;The;Bride;Kiddo;Black Mamba;01/01/1970;thebride@example.net;123456;Assassination Squad;Hattori Hanzo;Deadly Viper Assassination Squad;BAD;BAD; @@ -150,7 +152,7 @@ describe('Unit | UseCase | ImportSupOrganizationLearner', function () { // when const error = await catchErr(importSupOrganizationLearners)({ - organizationId, + organizationImportId, i18n, supOrganizationLearnerRepository: supOrganizationLearnerRepositoryStub, organizationImportRepository: organizationImportRepositoryStub, diff --git a/api/tests/prescription/learner-management/unit/domain/usecases/validate-csv-file_test.js b/api/tests/prescription/learner-management/unit/domain/usecases/validate-csv-file_test.js index 10b5bad04ad..a88d3bb7967 100644 --- a/api/tests/prescription/learner-management/unit/domain/usecases/validate-csv-file_test.js +++ b/api/tests/prescription/learner-management/unit/domain/usecases/validate-csv-file_test.js @@ -1,4 +1,5 @@ import { IMPORT_STATUSES } from '../../../../../../src/prescription/learner-management/domain/constants.js'; +import { AggregateImportError } from '../../../../../../src/prescription/learner-management/domain/errors.js'; import { OrganizationImport } from '../../../../../../src/prescription/learner-management/domain/models/OrganizationImport.js'; import { validateCsvFile } from '../../../../../../src/prescription/learner-management/domain/usecases/validate-csv-file.js'; import { SupOrganizationLearnerImportHeader } from '../../../../../../src/prescription/learner-management/infrastructure/serializers/csv/sup-organization-learner-import-header.js'; @@ -12,7 +13,7 @@ const supOrganizationLearnerImportHeader = new SupOrganizationLearnerImportHeade .map((column) => column.name) .join(';'); -describe('Unit | UseCase | ImportSupOrganizationLearner', function () { +describe('Unit | UseCase | validateCsvFile', function () { const organizationId = 1234; let csvContent, expectedWarnings, organizationImport, organizationImportRepositoryStub, importStorageStub; @@ -79,12 +80,75 @@ describe('Unit | UseCase | ImportSupOrganizationLearner', function () { }); context('when there is errors', function () { + let importSupOrganizationLearnersJobRepositoryStub, organizationImportStub; beforeEach(function () { - organizationImportRepositoryStub.getLastByOrganizationId.withArgs(organizationId).resolves(organizationImport); + importSupOrganizationLearnersJobRepositoryStub = { + performAsync: sinon.stub(), + }; + + organizationImportStub = { + id: 1, + filename: Symbol('filename'), + encoding: Symbol('encoding'), + validate: sinon.stub(), + save: sinon.stub(), + }; + organizationImportRepositoryStub.getLastByOrganizationId + .withArgs(organizationId) + .resolves(organizationImportStub); + }); + + context('when there is s3 errors', function () { + it('should save error when there is an error reading file from S3', async function () { + const s3Error = new Error('s3 error'); + importStorageStub.getParser.rejects(s3Error); + const error = await catchErr(validateCsvFile)({ + organizationId, + organizationImportRepository: organizationImportRepositoryStub, + importStorage: importStorageStub, + importSupOrganizationLearnersJobRepository: importSupOrganizationLearnersJobRepositoryStub, + }); + expect(error).to.eq(s3Error); + expect(importStorageStub.deleteFile).to.have.been.calledWithExactly({ + filename: organizationImportStub.filename, + }); + expect(organizationImportRepositoryStub.save).to.have.been.calledWithExactly(organizationImportStub); + expect(organizationImportStub.validate).to.have.been.calledWithExactly({ + errors: [s3Error], + warnings: undefined, + }); + }); + + it('should save error when there is an error deleting file from S3', async function () { + const parserStub = { parse: sinon.stub() }; + importStorageStub.getParser + .withArgs( + { Parser: SupOrganizationLearnerParser, filename: organizationImport.filename }, + organizationId, + i18n, + ) + .resolves(parserStub); + parserStub.parse.rejects(new AggregateImportError([new Error('parsing')])); + const s3Error = new Error('s3 error'); + importStorageStub.deleteFile.rejects(s3Error); + + const error = await catchErr(validateCsvFile)({ + organizationId, + organizationImportRepository: organizationImportRepositoryStub, + importStorage: importStorageStub, + importSupOrganizationLearnersJobRepositoryStub: importSupOrganizationLearnersJobRepositoryStub, + }); + expect(error).to.eq(s3Error); + expect(importStorageStub.deleteFile).to.have.been.calledWithExactly({ + filename: organizationImportStub.filename, + }); + expect(organizationImportRepositoryStub.save).to.have.been.calledWithExactly(organizationImportStub); + }); }); it('should save VALIDATION_ERROR status', async function () { // given + organizationImportRepositoryStub.getLastByOrganizationId.withArgs(organizationId).resolves(organizationImport); const csvContent = `${supOrganizationLearnerImportHeader} Beatrix;The;Bride;Kiddo;Black Mamba;01/01/1970;thebride@example.net;123456;Assassination Squad;Hattori Hanzo;Deadly Viper Assassination Squad;BAD;BAD; Beatrix;The;Bride;Kiddo;Black Mamba;01/01/1970;thebride@example.net;123456;Assassination Squad;Hattori Hanzo;Deadly Viper Assassination Squad;BAD;BAD; From 2c489602b6abc7a6493682d520b09589d07f935c Mon Sep 17 00:00:00 2001 From: Xavier Carron <33637571+xav-car@users.noreply.github.com> Date: Mon, 18 Nov 2024 16:54:59 +0100 Subject: [PATCH 4/5] feat(api): asynchronize replace student case --- ...up-organization-learners-job-controller.js | 19 ++- .../sup-organization-management-controller.js | 8 +- ...s => replace-sup-organization-learners.js} | 12 +- ...ganization-learners-job-controller_test.js | 118 +++++++++++++----- ...organization-management-controller_test.js | 15 +-- .../replace-sup-organization-learners_test.js | 2 +- 6 files changed, 112 insertions(+), 62 deletions(-) rename api/src/prescription/learner-management/domain/usecases/{replace-sup-organization-learner.js => replace-sup-organization-learners.js} (82%) diff --git a/api/src/prescription/learner-management/application/jobs/import-sup-organization-learners-job-controller.js b/api/src/prescription/learner-management/application/jobs/import-sup-organization-learners-job-controller.js index 8dd20f2aae7..e03dc29b345 100644 --- a/api/src/prescription/learner-management/application/jobs/import-sup-organization-learners-job-controller.js +++ b/api/src/prescription/learner-management/application/jobs/import-sup-organization-learners-job-controller.js @@ -19,13 +19,22 @@ class ImportSupOrganizationLearnersJobController extends JobController { } async handle({ data }) { - const { organizationImportId, locale } = data; + const { organizationImportId, locale, type } = data; + + const i18n = getI18n(locale); try { - return await await usecases.importSupOrganizationLearners({ - organizationImportId, - i18n: getI18n(locale), - }); + if (type === 'ADDITIONAL_STUDENT') { + return await await usecases.importSupOrganizationLearners({ + organizationImportId, + i18n, + }); + } else if (type === 'REPLACE_STUDENT') { + return await usecases.replaceSupOrganizationLearners({ + organizationImportId, + i18n, + }); + } } catch (err) { if (!(err instanceof DomainError)) { throw err; diff --git a/api/src/prescription/learner-management/application/sup-organization-management-controller.js b/api/src/prescription/learner-management/application/sup-organization-management-controller.js index 5a930a16ce6..b270048418a 100644 --- a/api/src/prescription/learner-management/application/sup-organization-management-controller.js +++ b/api/src/prescription/learner-management/application/sup-organization-management-controller.js @@ -28,7 +28,7 @@ const importSupOrganizationLearners = async function ( Parser: SupOrganizationLearnerParser, organizationId, i18n: request.i18n, - type: 'ADDITIONNAL_STUDENT', + type: 'ADDITIONAL_STUDENT', performJob: true, }); } finally { @@ -65,10 +65,8 @@ const replaceSupOrganizationLearners = async function ( Parser: SupOrganizationLearnerParser, organizationId, i18n: request.i18n, - }); - await usecases.replaceSupOrganizationLearners({ - organizationId, - i18n: request.i18n, + type: 'REPLACE_STUDENT', + performJob: true, }); } finally { // see https://hapi.dev/api/?v=21.3.3#-routeoptionspayloadoutput diff --git a/api/src/prescription/learner-management/domain/usecases/replace-sup-organization-learner.js b/api/src/prescription/learner-management/domain/usecases/replace-sup-organization-learners.js similarity index 82% rename from api/src/prescription/learner-management/domain/usecases/replace-sup-organization-learner.js rename to api/src/prescription/learner-management/domain/usecases/replace-sup-organization-learners.js index 772c459b67a..f6261796779 100644 --- a/api/src/prescription/learner-management/domain/usecases/replace-sup-organization-learner.js +++ b/api/src/prescription/learner-management/domain/usecases/replace-sup-organization-learners.js @@ -3,23 +3,27 @@ import { getDataBuffer } from '../../infrastructure/utils/bufferize/get-data-buf import { AggregateImportError } from '../errors.js'; const replaceSupOrganizationLearners = async function ({ - organizationId, + organizationImportId, i18n, supOrganizationLearnerRepository, organizationImportRepository, importStorage, }) { const errors = []; - const organizationImport = await organizationImportRepository.getLastByOrganizationId(organizationId); + const organizationImport = await organizationImportRepository.get(organizationImportId); try { const readableStream = await importStorage.readFile({ filename: organizationImport.filename }); const buffer = await getDataBuffer(readableStream); - const parser = SupOrganizationLearnerParser.buildParser(buffer, organizationId, i18n); + const parser = SupOrganizationLearnerParser.buildParser(buffer, organizationImport.organizationId, i18n); const { learners } = parser.parse(parser.getFileEncoding()); - await supOrganizationLearnerRepository.replaceStudents(organizationId, learners, organizationImport.createdBy); + await supOrganizationLearnerRepository.replaceStudents( + organizationImport.organizationId, + learners, + organizationImport.createdBy, + ); } catch (error) { if (error instanceof AggregateImportError) { errors.push(...error.meta); diff --git a/api/tests/prescription/learner-management/unit/application/jobs/import-sup-organization-learners-job-controller_test.js b/api/tests/prescription/learner-management/unit/application/jobs/import-sup-organization-learners-job-controller_test.js index e9082de5a3a..f58e1e4a164 100644 --- a/api/tests/prescription/learner-management/unit/application/jobs/import-sup-organization-learners-job-controller_test.js +++ b/api/tests/prescription/learner-management/unit/application/jobs/import-sup-organization-learners-job-controller_test.js @@ -31,52 +31,104 @@ describe('Unit | Prescription | Application | Jobs | importSupOrganizationLearne }); describe('#handle', function () { - it('should call usecase', async function () { - sinon.stub(usecases, 'importSupOrganizationLearners'); + describe('#importSupOrganizationLearners', function () { + it('should call usecase', async function () { + sinon.stub(usecases, 'importSupOrganizationLearners'); + + // given + const handler = new ImportSupOrganizationLearnersJobController(); + const data = { organizationImportId: Symbol('organizationImportId'), locale: 'en', type: 'ADDITIONAL_STUDENT' }; + + // when + await handler.handle({ data }); + + // then + expect(usecases.importSupOrganizationLearners).to.have.been.calledOnce; + expect(usecases.importSupOrganizationLearners).to.have.been.calledWithExactly({ + organizationImportId: data.organizationImportId, + i18n: getI18n(data.locale), + }); + }); - // given - const handler = new ImportSupOrganizationLearnersJobController(); - const data = { organizationImportId: Symbol('organizationImportId'), locale: 'en' }; + it('should not throw when error is from domain', async function () { + const error = new OrganizationLearnersCouldNotBeSavedError(); + sinon.stub(usecases, 'importSupOrganizationLearners').rejects(error); - // when - await handler.handle({ data }); + // given + const errorStub = sinon.stub(); + const handler = new ImportSupOrganizationLearnersJobController({ logger: { error: errorStub } }); + const data = { organizationImportId: Symbol('organizationImportId'), locale: 'en', type: 'ADDITIONAL_STUDENT' }; - // then - expect(usecases.importSupOrganizationLearners).to.have.been.calledOnce; - expect(usecases.importSupOrganizationLearners).to.have.been.calledWithExactly({ - organizationImportId: data.organizationImportId, - i18n: getI18n(data.locale), + // when & then + await handler.handle({ data }); + + expect(errorStub).to.have.been.calledWithExactly(error); }); - }); - it('should not throw when error is from domain', async function () { - const error = new OrganizationLearnersCouldNotBeSavedError(); - sinon.stub(usecases, 'importSupOrganizationLearners').rejects(error); + it('should throw when error is not from domain', async function () { + const error = new Error(); + sinon.stub(usecases, 'importSupOrganizationLearners').rejects(error); - // given - const errorStub = sinon.stub(); - const handler = new ImportSupOrganizationLearnersJobController({ logger: { error: errorStub } }); - const data = { organizationImportId: Symbol('organizationImportId') }; + // given + const handler = new ImportSupOrganizationLearnersJobController(); + const data = { organizationImportId: Symbol('organizationImportId'), locale: 'en', type: 'ADDITIONAL_STUDENT' }; - // when & then - await handler.handle({ data }); + // when + const result = await catchErr(handler.handle)({ data }); - expect(errorStub).to.have.been.calledWithExactly(error); + // then + expect(result).to.equal(error); + }); }); - it('should throw when error is not from domain', async function () { - const error = new Error(); - sinon.stub(usecases, 'importSupOrganizationLearners').rejects(error); + describe('#replaceSupOrganizationLearners', function () { + it('should call usecase', async function () { + sinon.stub(usecases, 'replaceSupOrganizationLearners'); - // given - const handler = new ImportSupOrganizationLearnersJobController(); - const data = { organizationImportId: Symbol('organizationImportId') }; + // given + const handler = new ImportSupOrganizationLearnersJobController(); + const data = { organizationImportId: Symbol('organizationImportId'), locale: 'en', type: 'REPLACE_STUDENT' }; - // when - const result = await catchErr(handler.handle)({ data }); + // when + await handler.handle({ data }); - // then - expect(result).to.equal(error); + // then + expect(usecases.replaceSupOrganizationLearners).to.have.been.calledOnce; + expect(usecases.replaceSupOrganizationLearners).to.have.been.calledWithExactly({ + organizationImportId: data.organizationImportId, + i18n: getI18n(data.locale), + }); + }); + + it('should not throw when error is from domain', async function () { + const error = new OrganizationLearnersCouldNotBeSavedError(); + sinon.stub(usecases, 'replaceSupOrganizationLearners').rejects(error); + + // given + const errorStub = sinon.stub(); + const handler = new ImportSupOrganizationLearnersJobController({ logger: { error: errorStub } }); + const data = { organizationImportId: Symbol('organizationImportId'), locale: 'en', type: 'REPLACE_STUDENT' }; + + // when & then + await handler.handle({ data }); + + expect(errorStub).to.have.been.calledWithExactly(error); + }); + + it('should throw when error is not from domain', async function () { + const error = new Error(); + sinon.stub(usecases, 'replaceSupOrganizationLearners').rejects(error); + + // given + const handler = new ImportSupOrganizationLearnersJobController(); + const data = { organizationImportId: Symbol('organizationImportId'), locale: 'en', type: 'REPLACE_STUDENT' }; + + // when + const result = await catchErr(handler.handle)({ data }); + + // then + expect(result).to.equal(error); + }); }); }); }); diff --git a/api/tests/prescription/learner-management/unit/application/sup-organization-management-controller_test.js b/api/tests/prescription/learner-management/unit/application/sup-organization-management-controller_test.js index 05002c133a3..7b09155e127 100644 --- a/api/tests/prescription/learner-management/unit/application/sup-organization-management-controller_test.js +++ b/api/tests/prescription/learner-management/unit/application/sup-organization-management-controller_test.js @@ -23,7 +23,6 @@ describe('Unit | Controller | sup-organization-management-controller', function sinon.stub(usecases, 'uploadCsvFile'); sinon.stub(usecases, 'validateCsvFile'); - sinon.stub(usecases, 'replaceSupOrganizationLearners'); supOrganizationLearnerWarningSerializerStub = { serialize: sinon.stub() }; logErrorWithCorrelationIdsStub = sinon.stub(); unlinkStub = sinon.stub(); @@ -114,12 +113,6 @@ describe('Unit | Controller | sup-organization-management-controller', function }; usecases.uploadCsvFile.withArgs({ userId, organizationId, payload: request.payload, i18n }).resolves(); usecases.validateCsvFile.withArgs({ organizationId, i18n }).resolves(); - usecases.replaceSupOrganizationLearners - .withArgs({ - organizationId, - i18n, - }) - .resolves(); supOrganizationLearnerWarningSerializerStub.serialize .withArgs({ id: organizationId, warnings }) @@ -134,13 +127,7 @@ describe('Unit | Controller | sup-organization-management-controller', function // then // then - expect( - sinon.assert.callOrder( - usecases.uploadCsvFile, - usecases.validateCsvFile, - usecases.replaceSupOrganizationLearners, - ), - ).to.not.throws; + expect(sinon.assert.callOrder(usecases.uploadCsvFile, usecases.validateCsvFile)).to.not.throws; expect(response.statusCode).to.be.equal(204); expect(unlinkStub).to.have.been.calledWith(path); diff --git a/api/tests/prescription/learner-management/unit/domain/usecases/replace-sup-organization-learners_test.js b/api/tests/prescription/learner-management/unit/domain/usecases/replace-sup-organization-learners_test.js index 3d16e5e4777..c3153b16677 100644 --- a/api/tests/prescription/learner-management/unit/domain/usecases/replace-sup-organization-learners_test.js +++ b/api/tests/prescription/learner-management/unit/domain/usecases/replace-sup-organization-learners_test.js @@ -1,5 +1,5 @@ import { OrganizationImport } from '../../../../../../src/prescription/learner-management/domain/models/OrganizationImport.js'; -import { replaceSupOrganizationLearners } from '../../../../../../src/prescription/learner-management/domain/usecases/replace-sup-organization-learner.js'; +import { replaceSupOrganizationLearners } from '../../../../../../src/prescription/learner-management/domain/usecases/replace-sup-organization-learners.js'; import { SupOrganizationLearnerImportHeader } from '../../../../../../src/prescription/learner-management/infrastructure/serializers/csv/sup-organization-learner-import-header.js'; import { getI18n } from '../../../../../../src/shared/infrastructure/i18n/i18n.js'; import { catchErr, expect, sinon, toStream } from '../../../../../test-helper.js'; From c46bc06301b7dfdff2fbe5a560da190e62ad3e61 Mon Sep 17 00:00:00 2001 From: Xavier Carron <33637571+xav-car@users.noreply.github.com> Date: Tue, 19 Nov 2024 10:23:21 +0100 Subject: [PATCH 5/5] tech(api): use organizationId instead of id --- .../sup-organization-management-controller.js | 8 ++++---- .../sup-organization-management-route.js | 16 ++++++++-------- .../sup-organization-management-route_test.js | 6 +++--- .../replace-sup-organization-learners_test.js | 16 ++++++++++------ 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/api/src/prescription/learner-management/application/sup-organization-management-controller.js b/api/src/prescription/learner-management/application/sup-organization-management-controller.js index b270048418a..434c1cc35f1 100644 --- a/api/src/prescription/learner-management/application/sup-organization-management-controller.js +++ b/api/src/prescription/learner-management/application/sup-organization-management-controller.js @@ -13,7 +13,7 @@ const importSupOrganizationLearners = async function ( unlink: fs.unlink, }, ) { - const organizationId = request.params.id; + const organizationId = request.params.organizationId; const userId = request.auth.credentials.userId; try { @@ -51,7 +51,7 @@ const replaceSupOrganizationLearners = async function ( }, ) { const userId = request.auth.credentials.userId; - const organizationId = request.params.id; + const organizationId = request.params.organizationId; try { await usecases.uploadCsvFile({ @@ -82,7 +82,7 @@ const replaceSupOrganizationLearners = async function ( }; const getOrganizationLearnersCsvTemplate = async function (request, h, dependencies = { tokenService }) { - const organizationId = request.params.id; + const organizationId = request.params.organizationId; const token = request.query.accessToken; const userId = dependencies.tokenService.extractUserId(token); const template = await usecases.getOrganizationLearnersCsvTemplate({ @@ -99,7 +99,7 @@ const getOrganizationLearnersCsvTemplate = async function (request, h, dependenc const updateStudentNumber = async function (request, h) { const payload = request.payload.data.attributes; - const organizationId = request.params.id; + const organizationId = request.params.organizationId; const studentNumber = payload['student-number']; const organizationLearnerId = request.params.organizationLearnerId; diff --git a/api/src/prescription/learner-management/application/sup-organization-management-route.js b/api/src/prescription/learner-management/application/sup-organization-management-route.js index cd18c88a10c..e385e1feeff 100644 --- a/api/src/prescription/learner-management/application/sup-organization-management-route.js +++ b/api/src/prescription/learner-management/application/sup-organization-management-route.js @@ -20,7 +20,7 @@ const register = async function (server) { server.route([ { method: 'POST', - path: '/api/organizations/{id}/sup-organization-learners/replace-csv', + path: '/api/organizations/{organizationId}/sup-organization-learners/replace-csv', config: { pre: [ { @@ -30,7 +30,7 @@ const register = async function (server) { ], validate: { params: Joi.object({ - id: identifiersType.organizationId, + organizationId: identifiersType.organizationId, }), }, payload: { @@ -57,7 +57,7 @@ const register = async function (server) { }, { method: 'POST', - path: '/api/organizations/{id}/sup-organization-learners/import-csv', + path: '/api/organizations/{organizationId}/sup-organization-learners/import-csv', config: { pre: [ { @@ -67,7 +67,7 @@ const register = async function (server) { ], validate: { params: Joi.object({ - id: identifiersType.organizationId, + organizationId: identifiersType.organizationId, }), }, payload: { @@ -94,12 +94,12 @@ const register = async function (server) { }, { method: 'GET', - path: '/api/organizations/{id}/organization-learners/csv-template', + path: '/api/organizations/{organizationId}/organization-learners/csv-template', config: { auth: false, validate: { params: Joi.object({ - id: identifiersType.organizationId, + organizationId: identifiersType.organizationId, }), query: Joi.object({ accessToken: Joi.string().required(), @@ -116,7 +116,7 @@ const register = async function (server) { }, { method: 'PATCH', - path: '/api/organizations/{id}/sup-organization-learners/{organizationLearnerId}', + path: '/api/organizations/{organizationId}/sup-organization-learners/{organizationLearnerId}', config: { pre: [ { @@ -129,7 +129,7 @@ const register = async function (server) { allowUnknown: true, }, params: Joi.object({ - id: identifiersType.organizationId, + organizationId: identifiersType.organizationId, organizationLearnerId: identifiersType.organizationLearnerId, }), payload: Joi.object({ diff --git a/api/tests/prescription/learner-management/acceptance/application/sup-organization-management-route_test.js b/api/tests/prescription/learner-management/acceptance/application/sup-organization-management-route_test.js index cec34950cc9..4641a718092 100644 --- a/api/tests/prescription/learner-management/acceptance/application/sup-organization-management-route_test.js +++ b/api/tests/prescription/learner-management/acceptance/application/sup-organization-management-route_test.js @@ -20,7 +20,7 @@ describe('Acceptance | Application | organization-controller-sup-organization-le server = await createServer(); }); - describe('POST organizations/:id/sup-organization-learners/import-csv', function () { + describe('POST organizations/{organizationId}/sup-organization-learners/import-csv', function () { let connectedUser; beforeEach(async function () { @@ -130,7 +130,7 @@ describe('Acceptance | Application | organization-controller-sup-organization-le ); }); - describe('POST organizations/:id/sup-organization-learners/replace-csv', function () { + describe('POST organizations/{organizationId}/sup-organization-learners/replace-csv', function () { let connectedUser; beforeEach(async function () { @@ -188,7 +188,7 @@ describe('Acceptance | Application | organization-controller-sup-organization-le }); }); - describe('GET /api/organizations/{id}/organization-learners/csv-template', function () { + describe('GET /api/organizations/{organizationId}/organization-learners/csv-template', function () { let userId, organization, accessToken; beforeEach(async function () { diff --git a/api/tests/prescription/learner-management/unit/domain/usecases/replace-sup-organization-learners_test.js b/api/tests/prescription/learner-management/unit/domain/usecases/replace-sup-organization-learners_test.js index c3153b16677..ca580b74885 100644 --- a/api/tests/prescription/learner-management/unit/domain/usecases/replace-sup-organization-learners_test.js +++ b/api/tests/prescription/learner-management/unit/domain/usecases/replace-sup-organization-learners_test.js @@ -11,6 +11,7 @@ const supOrganizationLearnerImportHeader = new SupOrganizationLearnerImportHeade .join(';'); describe('Unit | UseCase | ReplaceSupOrganizationLearner', function () { + let organizationImportId; const organizationId = 1234; const userId = 333; let organizationImport, @@ -20,7 +21,9 @@ describe('Unit | UseCase | ReplaceSupOrganizationLearner', function () { expectedLearners; beforeEach(function () { + organizationImportId = Symbol('organizationImportId'); organizationImport = new OrganizationImport({ + id: organizationImportId, filename: 'file.csv', organizationId, createdBy: userId, @@ -28,10 +31,11 @@ describe('Unit | UseCase | ReplaceSupOrganizationLearner', function () { }); supOrganizationLearnerRepositoryStub = { replaceStudents: sinon.stub().resolves() }; organizationImportRepositoryStub = { - getLastByOrganizationId: sinon.stub().withArgs(organizationId).resolves(organizationImport), + get: sinon.stub(), save: sinon.stub(), }; + organizationImportRepositoryStub.get.withArgs(organizationImportId).resolves(organizationImport); importStorageStub = { readFile: sinon.stub(), deleteFile: sinon.stub(), @@ -84,7 +88,7 @@ describe('Unit | UseCase | ReplaceSupOrganizationLearner', function () { // when await replaceSupOrganizationLearners({ - organizationId, + organizationImportId, i18n, supOrganizationLearnerRepository: supOrganizationLearnerRepositoryStub, organizationImportRepository: organizationImportRepositoryStub, @@ -110,7 +114,7 @@ describe('Unit | UseCase | ReplaceSupOrganizationLearner', function () { // when await replaceSupOrganizationLearners({ - organizationId, + organizationImportId, i18n, supOrganizationLearnerRepository: supOrganizationLearnerRepositoryStub, organizationImportRepository: organizationImportRepositoryStub, @@ -134,7 +138,7 @@ describe('Unit | UseCase | ReplaceSupOrganizationLearner', function () { // when await replaceSupOrganizationLearners({ - organizationId, + organizationImportId, supOrganizationLearnerRepository: supOrganizationLearnerRepositoryStub, importStorage: importStorageStub, organizationImportRepository: organizationImportRepositoryStub, @@ -149,7 +153,7 @@ describe('Unit | UseCase | ReplaceSupOrganizationLearner', function () { describe('errors case', function () { beforeEach(function () { - organizationImportRepositoryStub.getLastByOrganizationId.withArgs(organizationId).resolves(organizationImport); + organizationImportRepositoryStub.get.withArgs(organizationImportId).resolves(organizationImport); const csvContent = `${supOrganizationLearnerImportHeader} Beatrix;The;Bride;Kiddo;Black Mamba;01/01/1970;thebride@example.net;123456;Assassination Squad;Hattori Hanzo;Deadly Viper Assassination Squad;BAD;BAD; @@ -165,7 +169,7 @@ describe('Unit | UseCase | ReplaceSupOrganizationLearner', function () { // when await catchErr(replaceSupOrganizationLearners)({ - organizationId, + organizationImportId, i18n, supOrganizationLearnerRepository: supOrganizationLearnerRepositoryStub, organizationImportRepository: organizationImportRepositoryStub,