fix(api): changed error to log helpful variables and not be too expli…

…cit frontend side
1024pix · Nov 14, 2024 · c2963d5 · c2963d5
1 parent e102081
commit c2963d5
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 22 deletions.
diff --git a/api/src/prescription/learner-management/domain/errors.js b/api/src/prescription/learner-management/domain/errors.js
@@ -41,15 +41,8 @@ class ReconcileCommonOrganizationLearnerError extends DomainError {
   }
 }
 
-class CouldNotDeleteLearnersError extends DomainError {
-  constructor(organizationLearnerIds) {
-    super(`Could not delete the following organization learners : ${organizationLearnerIds}`);
-  }
-}
-
 export {
   AggregateImportError,
-  CouldNotDeleteLearnersError,
   OrganizationDoesNotHaveFeatureEnabledError,
   OrganizationLearnerImportFormatNotFoundError,
   OrganizationLearnersCouldNotBeSavedError,

diff --git a/api/src/prescription/learner-management/domain/models/OrganizationLearnerList.js b/api/src/prescription/learner-management/domain/models/OrganizationLearnerList.js
@@ -1,16 +1,19 @@
-import { CouldNotDeleteLearnersError } from '../errors.js';
+import { logger } from '../../../../shared/infrastructure/utils/logger.js';
 
 class OrganizationLearnerList {
   constructor({ organizationId, organizationLearnerIds } = {}) {
     this.organizationId = organizationId;
     this.organizationLearnerIds = organizationLearnerIds;
   }
-  canDeleteOrganizationLearners(organizationLearnerIdsToValidate) {
+  canDeleteOrganizationLearners(organizationLearnerIdsToValidate, userId) {
     const result = organizationLearnerIdsToValidate.filter((organizationLearnerId) => {
-      return this.organizationLearnerIds.includes(organizationLearnerId);
+      return !this.organizationLearnerIds.includes(organizationLearnerId);
     });
-    if (result.length !== organizationLearnerIdsToValidate.length) {
-      throw new CouldNotDeleteLearnersError(this.organizationLearnerIds);
+    if (result.length !== 0) {
+      logger.error(
+        `User id ${userId} could not delete organization learners because learner id ${result} don't belong to organization id ${this.organizationId} "`,
+      );
+      throw new Error('Could not delete organization learners.');
     }
   }
 }

diff --git a/api/src/prescription/learner-management/domain/usecases/delete-organization-learners.js b/api/src/prescription/learner-management/domain/usecases/delete-organization-learners.js
@@ -18,7 +18,7 @@ const deleteOrganizationLearners = async function ({
     organizationLearnerIds: learnersBelogingToOrganization,
   });
 
-  organizationLearnerList.canDeleteOrganizationLearners(organizationLearnerIds);
+  organizationLearnerList.canDeleteOrganizationLearners(organizationLearnerIds, userId);
   await campaignParticipationRepository.removeByOrganizationLearnerIds({
     organizationLearnerIds,
     userId,

diff --git a/api/tests/prescription/learner-management/unit/domain/models/OrganizationLearnerList_test.js b/api/tests/prescription/learner-management/unit/domain/models/OrganizationLearnerList_test.js
@@ -1,4 +1,3 @@
-import { CouldNotDeleteLearnersError } from '../../../../../../src/prescription/learner-management/domain/errors.js';
 import { OrganizationLearnerList } from '../../../../../../src/prescription/learner-management/domain/models/OrganizationLearnerList.js';
 import { catchErrSync, expect } from '../../../../../test-helper.js';
 
@@ -18,6 +17,7 @@ describe('Unit | Models | OrganizationLearnerListFormat', function () {
 
   describe('#can delete organization learners ', function () {
     it('should throw when lists are different', function () {
+      const userId = Symbol('123');
       //when
       const payload = {
         organizationId: Symbol('organizationId'),
@@ -26,23 +26,25 @@ describe('Unit | Models | OrganizationLearnerListFormat', function () {
 
       const organizationLearnerList = new OrganizationLearnerList(payload);
 
-      const result = catchErrSync(
-        organizationLearnerList.canDeleteOrganizationLearners,
-        organizationLearnerList,
-      )([456, 123]);
+      const result = catchErrSync(organizationLearnerList.canDeleteOrganizationLearners, organizationLearnerList)(
+        [456, 123],
+        userId,
+      );
 
-      expect(result).to.be.instanceof(CouldNotDeleteLearnersError);
+      expect(result).to.be.instanceof(Error);
     });
 
     it('should not throw when lists are identical', function () {
+      const userId = Symbol('123');
+
       const payload = {
         organizationId: Symbol('organizationId'),
         organizationLearnerIds: [123, 345],
       };
 
       expect(() => {
         const organizationLearnerList = new OrganizationLearnerList(payload);
-        organizationLearnerList.canDeleteOrganizationLearners([123, 345]);
+        organizationLearnerList.canDeleteOrganizationLearners([123, 345], userId);
       }).to.not.throw();
     });
   });

diff --git a/...prescription/learner-management/unit/domain/usecases/delete-organization-learners_test.js b/...prescription/learner-management/unit/domain/usecases/delete-organization-learners_test.js
@@ -38,7 +38,7 @@ describe('Unit | UseCase | Organization Learners Management | Delete Organizatio
       organizationLearnerRepository,
     });
 
-    expect(canDeleteStub).to.have.been.calledWith(organizationLearnerIds);
+    expect(canDeleteStub).to.have.been.calledWith(organizationLearnerIds, userId);
 
     expect(organizationLearnerRepository.findOrganizationLearnerIdsByOrganizationId).to.have.been.calledWithExactly({
       organizationId,
@@ -70,7 +70,7 @@ describe('Unit | UseCase | Organization Learners Management | Delete Organizatio
       organizationLearnerRepository,
     });
 
-    expect(canDeleteStub).to.have.been.calledWith(organizationLearnerIdsPayload);
+    expect(canDeleteStub).to.have.been.calledWith(organizationLearnerIdsPayload, userId);
 
     expect(organizationLearnerRepository.findOrganizationLearnerIdsByOrganizationId).to.have.been.calledWithExactly({
       organizationId,