[TECH] Sauvegarder l'état de l'import dans tous les cas (PIX-14200) (#…

…10088)
1024pix · Sep 16, 2024 · 9f05896 · 9f05896
1 parent 21c5630
commit 9f05896
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 51 deletions.
diff --git a/...rc/prescription/learner-management/domain/usecases/add-or-update-organization-learners.js b/...rc/prescription/learner-management/domain/usecases/add-or-update-organization-learners.js
@@ -13,6 +13,7 @@ async function addOrUpdateOrganizationLearners({
   organizationLearnerRepository,
   organizationImportRepository,
   importStorage,
+  logger,
   chunkSize = ORGANIZATION_LEARNER_CHUNK_SIZE,
 }) {
   const errors = [];
@@ -64,8 +65,11 @@ async function addOrUpdateOrganizationLearners({
       organizationImport.process({ errors });
       await organizationImportRepository.save(organizationImport);
       loggerForImport("IMPORT_LOG -> <<addOrUpdateOrganizationLearners>> Aprés le save de l'organizationImport");
-
-      await importStorage.deleteFile({ filename: organizationImport.filename });
+      try {
+        await importStorage.deleteFile({ filename: organizationImport.filename });
+      } catch (error) {
+        logger.error(error);
+      }
 
       loggerForImport('IMPORT_LOG -> <<addOrUpdateOrganizationLearners>> Fin du usecase');
     }

diff --git a/api/src/prescription/learner-management/domain/usecases/index.js b/api/src/prescription/learner-management/domain/usecases/index.js
@@ -5,6 +5,7 @@ import * as userReconciliationService from '../../../../../lib/domain/services/u
 import * as campaignRepository from '../../../../../lib/infrastructure/repositories/campaign-repository.js';
 import * as membershipRepository from '../../../../../lib/infrastructure/repositories/membership-repository.js';
 import { logErrorWithCorrelationIds } from '../../../../../src/shared/infrastructure/monitoring-tools.js';
+import { logger } from '../../../../../src/shared/infrastructure/utils/logger.js';
 import * as organizationFeatureApi from '../../../../organizational-entities/application/api/organization-features-api.js';
 import * as organizationRepository from '../../../../shared/infrastructure/repositories/organization-repository.js';
 import { injectDependencies } from '../../../../shared/infrastructure/utils/dependency-injection.js';
@@ -50,6 +51,7 @@ const dependencies = {
   logErrorWithCorrelationIds,
   userReconciliationService,
   organizationFeatureRepository: repositories.organizationFeatureRepository,
+  logger,
 };
 
 const path = dirname(fileURLToPath(import.meta.url));

diff --git a/...cription/learner-management/infrastructure/repositories/organization-import-repository.js b/...cription/learner-management/infrastructure/repositories/organization-import-repository.js
@@ -1,7 +1,6 @@
 import { knex } from '../../../../../db/knex-database-connection.js';
 import { DomainTransaction } from '../../../../shared/domain/DomainTransaction.js';
 import { ApplicationTransaction } from '../../../shared/infrastructure/ApplicationTransaction.js';
-import { IMPORT_STATUSES } from '../../domain/constants.js';
 import { OrganizationImport } from '../../domain/models/OrganizationImport.js';
 import { OrganizationImportDetail } from '../../domain/read-models/OrganizationImportDetail.js';
 
@@ -50,18 +49,13 @@ function _stringifyErrors(errors) {
 }
 
 const save = async function (organizationImport) {
-  let knexConn = DomainTransaction.getConnection();
-
   const attributes = { ...organizationImport, errors: _stringifyErrors(organizationImport.errors) };
-  if (attributes.errors || attributes.status === IMPORT_STATUSES.UPLOADING) {
-    // if there is errors, we don't want to use the given transaction
-    knexConn = knex;
-  }
+
   if (organizationImport.id) {
-    const updatedRows = await knexConn('organization-imports').update(attributes).where({ id: organizationImport.id });
+    const updatedRows = await knex('organization-imports').update(attributes).where({ id: organizationImport.id });
     if (updatedRows === 0) throw new Error();
   } else {
-    await knexConn('organization-imports').insert(attributes);
+    await knex('organization-imports').insert(attributes);
   }
 };
 

diff --git a/...management/integration/infrastructure/repositories/organization-import-repository_test.js b/...management/integration/infrastructure/repositories/organization-import-repository_test.js
@@ -2,7 +2,7 @@ import { IMPORT_STATUSES } from '../../../../../../src/prescription/learner-mana
 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, withTransaction } from '../../../../../../src/shared/domain/DomainTransaction.js';
+import { DomainTransaction } from '../../../../../../src/shared/domain/DomainTransaction.js';
 import { catchErr, databaseBuilder, expect, sinon } from '../../../../../test-helper.js';
 
 describe('Integration | Repository | Organization Learner Management | Organization Import', function () {
@@ -65,45 +65,6 @@ describe('Integration | Repository | Organization Learner Management | Organizat
 
       expect(error).to.be.ok;
     });
-
-    it('should use domainTransaction', async function () {
-      const organizationId = databaseBuilder.factory.buildOrganization().id;
-      const userId = databaseBuilder.factory.buildUser().id;
-      await databaseBuilder.commit();
-
-      const organizationImport = OrganizationImport.create({ organizationId, createdBy: userId });
-      organizationImport.upload({ filename: 'test.csv', encoding: 'utf8' });
-      try {
-        await DomainTransaction.execute(async (domainTransaction) => {
-          await organizationImportRepository.save(organizationImport, domainTransaction);
-          throw new Error();
-        });
-        // eslint-disable-next-line no-empty
-      } catch (e) {}
-
-      const savedImport = await organizationImportRepository.getLastByOrganizationId(organizationId);
-      expect(savedImport).to.be.null;
-    });
-
-    it('should use ApplicationTransaction', async function () {
-      const organizationId = databaseBuilder.factory.buildOrganization().id;
-      const userId = databaseBuilder.factory.buildUser().id;
-      await databaseBuilder.commit();
-
-      const organizationImport = OrganizationImport.create({ organizationId, createdBy: userId });
-      organizationImport.upload({ filename: 'test.csv', encoding: 'utf8' });
-
-      try {
-        await withTransaction(async () => {
-          await organizationImportRepository.save(organizationImport);
-          throw new Error();
-        })();
-        // eslint-disable-next-line no-empty
-      } catch (e) {}
-
-      const savedImport = await organizationImportRepository.getLastByOrganizationId(organizationId);
-      expect(savedImport).to.be.null;
-    });
   });
 
   describe('#get', function () {

diff --git a/...ption/learner-management/unit/domain/usecases/add-or-update-organization-learners_test.js b/...ption/learner-management/unit/domain/usecases/add-or-update-organization-learners_test.js
@@ -14,6 +14,7 @@ describe('Unit | UseCase | add-or-update-organization-learners', function () {
   let parserStub;
   let streamerSymbol;
   let organizationImportStub;
+  let loggerStub;
 
   beforeEach(function () {
     organizationId = Symbol('organizationId');
@@ -54,6 +55,9 @@ describe('Unit | UseCase | add-or-update-organization-learners', function () {
       addOrUpdateOrganizationOfOrganizationLearners: sinon.stub(),
       disableAllOrganizationLearnersInOrganization: sinon.stub().resolves(),
     };
+    loggerStub = {
+      error: sinon.stub(),
+    };
   });
 
   it('should save learners', async function () {
@@ -107,4 +111,19 @@ describe('Unit | UseCase | add-or-update-organization-learners', function () {
     expect(organizationImportStub.process).to.have.been.called;
     expect(organizationImportRepositoryStub.save).to.have.been.called;
   });
+
+  it('should save import state even if deleteFile crash but log error for tracking', async function () {
+    const s3DeleteError = new Error('s3 delete error');
+    importStorageStub.deleteFile.rejects(s3DeleteError);
+    await addOrUpdateOrganizationLearners({
+      organizationImportId,
+      importStorage: importStorageStub,
+      organizationImportRepository: organizationImportRepositoryStub,
+      organizationLearnerRepository: organizationLearnerRepositoryStub,
+      logger: loggerStub,
+      chunkSize: 2,
+    });
+    expect(loggerStub.error).to.have.been.calledWith(s3DeleteError);
+    expect(organizationImportRepositoryStub.save).to.have.been.calledWithExactly(organizationImportStub);
+  });
 });