feat(api): add idempotent call to insert feature

1024pix · Dec 4, 2024 · 93c2cc9 · 93c2cc9
1 parent 2f8e5c1
commit 93c2cc9
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 53 deletions.
diff --git a/api/src/organizational-entities/application/http-error-mapper-configuration.js b/api/src/organizational-entities/application/http-error-mapper-configuration.js
@@ -1,7 +1,6 @@
 import { HttpErrors } from '../../shared/application/http-errors.js';
 import { DomainErrorMappingConfiguration } from '../../shared/application/models/domain-error-mapping-configuration.js';
 import {
-  AlreadyExistingOrganizationFeatureError,
   DpoEmailInvalid,
   FeatureNotFound,
   FeatureParamsNotProcessable,
@@ -16,10 +15,6 @@ const organizationalEntitiesDomainErrorMappingConfiguration = [
     name: UnableToAttachChildOrganizationToParentOrganizationError.name,
     httpErrorFn: (error) => new HttpErrors.ConflictError(error.message, error.code, error.meta),
   },
-  {
-    name: AlreadyExistingOrganizationFeatureError.name,
-    httpErrorFn: (error) => new HttpErrors.ConflictError(error.message, error.code, error.meta),
-  },
   {
     name: OrganizationNotFound.name,
     httpErrorFn: (error) => new HttpErrors.UnprocessableEntityError(error.message, error.code, error.meta),

diff --git a/api/src/organizational-entities/domain/errors.js b/api/src/organizational-entities/domain/errors.js
@@ -12,18 +12,6 @@ class UnableToAttachChildOrganizationToParentOrganizationError extends DomainErr
   }
 }
 
-class AlreadyExistingOrganizationFeatureError extends DomainError {
-  constructor({
-    code = 'ALREADY_EXISTING_ORGANIZATION_FEATURE',
-    message = 'Unable to add feature to organization',
-    meta,
-  } = {}) {
-    super(message);
-    this.code = code;
-    this.meta = meta;
-  }
-}
-
 class DpoEmailInvalid extends DomainError {
   constructor({ code = 'DPO_EMAIL_INVALID', message = 'DPO email invalid', meta } = {}) {
     super(message);
@@ -72,7 +60,6 @@ class FeatureParamsNotProcessable extends DomainError {
 }
 
 export {
-  AlreadyExistingOrganizationFeatureError,
   DpoEmailInvalid,
   FeatureNotFound,
   FeatureParamsNotProcessable,

diff --git a/...rc/organizational-entities/infrastructure/repositories/organization-feature-repository.js b/...rc/organizational-entities/infrastructure/repositories/organization-feature-repository.js
@@ -3,7 +3,7 @@
  */
 import * as knexUtils from '../../../../src/shared/infrastructure/utils/knex-utils.js';
 import { DomainTransaction } from '../../../shared/domain/DomainTransaction.js';
-import { AlreadyExistingOrganizationFeatureError, FeatureNotFound, OrganizationNotFound } from '../../domain/errors.js';
+import { FeatureNotFound, OrganizationNotFound } from '../../domain/errors.js';
 import { OrganizationFeatureItem } from '../../domain/models/OrganizationFeatureItem.js';
 
 /**
@@ -13,21 +13,18 @@ import { OrganizationFeatureItem } from '../../domain/models/OrganizationFeature
  * @typedef {import('../../domain/models/OrganizationFeatureItem.js').OrganizationFeatureItem} OrganizationFeatureItem
  */
 
-const DEFAULT_BATCH_SIZE = 100;
-
 /**
  **
  * @param {OrganizationFeature[]} organizations
  */
-async function saveInBatch(organizationFeatures, batchSize = DEFAULT_BATCH_SIZE) {
+async function saveInBatch(organizationFeatures) {
   try {
     const knexConn = DomainTransaction.getConnection();
-    await knexConn.batchInsert('organization-features', organizationFeatures, batchSize);
+    await knexConn('organization-features')
+      .insert(organizationFeatures)
+      .onConflict(['featureId', 'organizationId'])
+      .ignore();
   } catch (err) {
-    if (knexUtils.isUniqConstraintViolated(err)) {
-      throw new AlreadyExistingOrganizationFeatureError();
-    }
-
     if (knexUtils.foreignKeyConstraintViolated(err) && err.constraint.includes('featureid')) {
       throw new FeatureNotFound();
     }

diff --git a/api/src/prescription/learner-management/application/api/learners-api.js b/api/src/prescription/learner-management/application/api/learners-api.js
@@ -1,3 +1,4 @@
+import { withTransaction } from '../../../../shared/domain/DomainTransaction.js';
 import { usecases } from '../../domain/usecases/index.js';
 
 /**
@@ -27,7 +28,7 @@ export const hasBeenLearner = async ({ userId }) => {
  * @returns {Promise<void>}
  * @throws TypeError - Throw when params.userId or params.organizationId is not defined
  */
-export const deleteOrganizationLearnerBeforeImportFeature = async ({ userId, organizationId }) => {
+export const deleteOrganizationLearnerBeforeImportFeature = withTransaction(async ({ userId, organizationId }) => {
   if (!userId) {
     throw new TypeError('userId is required');
   }
@@ -39,4 +40,4 @@ export const deleteOrganizationLearnerBeforeImportFeature = async ({ userId, org
   const organizationLearnerIds = await usecases.findOrganizationLearnersBeforeImportFeature({ organizationId });
 
   return usecases.deleteOrganizationLearners({ userId, organizationId, organizationLearnerIds });
-};
+});
diff --git a/...nizational-entities/integration/application/organization/organization.admin.route.test.js b/...nizational-entities/integration/application/organization/organization.admin.route.test.js
@@ -1,7 +1,6 @@
 import { organizationAdminController } from '../../../../../src/organizational-entities/application/organization/organization.admin.controller.js';
 import * as organizationAdminRoutes from '../../../../../src/organizational-entities/application/organization/organization.admin.route.js';
 import {
-  AlreadyExistingOrganizationFeatureError,
   DpoEmailInvalid,
   FeatureNotFound,
   FeatureParamsNotProcessable,
@@ -93,20 +92,6 @@ describe('Integration | Organizational Entities | Application | Route | Admin |
         expect(response.statusCode).to.equal(422);
       });
     });
-
-    context('when trying to add already existing feature on organization', function () {
-      it('returns a 409 HTTP status code', async function () {
-        organizationAdminController.addOrganizationFeatureInBatch.rejects(
-          new AlreadyExistingOrganizationFeatureError(),
-        );
-
-        // when
-        const response = await httpTestServer.request(method, url, payload);
-
-        // then
-        expect(response.statusCode).to.equal(409);
-      });
-    });
   });
 
   describe('POST /api/admin/organizations/update-organizations', function () {

diff --git a/...-entities/integration/infrastructure/repositories/organization-feature-repository_test.js b/...-entities/integration/infrastructure/repositories/organization-feature-repository_test.js
@@ -1,8 +1,4 @@
-import {
-  AlreadyExistingOrganizationFeatureError,
-  FeatureNotFound,
-  OrganizationNotFound,
-} from '../../../../../src/organizational-entities/domain/errors.js';
+import { FeatureNotFound, OrganizationNotFound } from '../../../../../src/organizational-entities/domain/errors.js';
 import { OrganizationFeature } from '../../../../../src/organizational-entities/domain/models/OrganizationFeature.js';
 import { OrganizationFeatureItem } from '../../../../../src/organizational-entities/domain/models/OrganizationFeatureItem.js';
 import * as organizationFeatureRepository from '../../../../../src/organizational-entities/infrastructure/repositories/organization-feature-repository.js';
@@ -54,7 +50,7 @@ describe('Integration | Repository | Organization-for-admin', function () {
       expect(result[1].params).to.deep.equal(organizationFeatures[1].params);
     });
 
-    it('throws an error if organization feature already exists', async function () {
+    it('should passe even if organization feature already exists', async function () {
       databaseBuilder.factory.buildOrganizationFeature({ organizationId: organization.id, featureId: feature.id });
       await databaseBuilder.commit();
 
@@ -67,9 +63,7 @@ describe('Integration | Repository | Organization-for-admin', function () {
       ];
 
       // when
-      const error = await catchErr(organizationFeatureRepository.saveInBatch)(organizationFeatures);
-
-      expect(error).to.be.instanceOf(AlreadyExistingOrganizationFeatureError);
+      expect(await organizationFeatureRepository.saveInBatch(organizationFeatures)).to.not.throws;
     });
 
     it('throws an error if organization does not exists', async function () {

diff --git a/api/tests/prescription/learner-management/unit/application/api/learners-api_test.js b/api/tests/prescription/learner-management/unit/application/api/learners-api_test.js
@@ -3,6 +3,7 @@ import {
   hasBeenLearner,
 } from '../../../../../../src/prescription/learner-management/application/api/learners-api.js';
 import { usecases } from '../../../../../../src/prescription/learner-management/domain/usecases/index.js';
+import { DomainTransaction } from '../../../../../../src/shared/domain/DomainTransaction.js';
 import { catchErr, expect, sinon } from '../../../../../test-helper.js';
 
 describe('Unit | Prescription | learner management | Api | learners', function () {
@@ -37,6 +38,10 @@ describe('Unit | Prescription | learner management | Api | learners', function (
     let findOrganizationLearnersBeforeImportFeatureStub, deleteOrganizationLearnersStub;
 
     beforeEach(function () {
+      sinon.stub(DomainTransaction, 'execute').callsFake((callback) => {
+        return callback();
+      });
+
       findOrganizationLearnersBeforeImportFeatureStub = sinon
         .stub(usecases, 'findOrganizationLearnersBeforeImportFeature')
         .rejects();