fix(api): prevent calling reward user usecase if user id is not provided

1024pix · Dec 2, 2024 · c07273a · c07273a
1 parent 7ace134
commit c07273a
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 18 deletions.
diff --git a/api/src/evaluation/application/answers/answer-controller.js b/api/src/evaluation/application/answers/answer-controller.js
@@ -11,7 +11,11 @@ const save = async function (request, h, dependencies = { answerSerializer, requ
   const userId = dependencies.requestResponseUtils.extractUserIdFromRequest(request);
   const locale = dependencies.requestResponseUtils.extractLocaleFromRequest(request);
   const createdAnswer = await usecases.correctAnswerThenUpdateAssessment({ answer, userId, locale });
-  if (!config.featureToggles.isAsyncQuestRewardingCalculationEnabled && config.featureToggles.isQuestEnabled) {
+  if (
+    userId &&
+    !config.featureToggles.isAsyncQuestRewardingCalculationEnabled &&
+    config.featureToggles.isQuestEnabled
+  ) {
     await questUsecases.rewardUser({ userId });
   }
 

diff --git a/api/src/quest/domain/usecases/reward-user.js b/api/src/quest/domain/usecases/reward-user.js
@@ -5,6 +5,10 @@ export const rewardUser = async ({
   successRepository,
   rewardRepository,
 }) => {
+  if (!userId) {
+    return;
+  }
+
   const quests = await questRepository.findAll();
 
   if (quests.length === 0) {

diff --git a/api/tests/evaluation/unit/application/answers/answer-controller_test.js b/api/tests/evaluation/unit/application/answers/answer-controller_test.js
@@ -136,7 +136,6 @@ describe('Unit | Controller | answer-controller', function () {
           locale,
         });
       });
-
       it('should serialize the answer', function () {
         // then
         expect(answerSerializerStub.serialize).to.have.been.calledWithExactly(createdAnswer);
@@ -170,9 +169,13 @@ describe('Unit | Controller | answer-controller', function () {
         });
 
         context('when quest feature enabled', function () {
+          beforeEach(function () {
+            config.featureToggles.isQuestEnabled = true;
+            config.featureToggles.isAsyncQuestRewardingCalculationEnabled = false;
+          });
+
           it('should not call rewardUser if async is enabled', async function () {
             // given
-            config.featureToggles.isQuestEnabled = true;
             config.featureToggles.isAsyncQuestRewardingCalculationEnabled = true;
 
             // when
@@ -187,7 +190,6 @@ describe('Unit | Controller | answer-controller', function () {
 
           it('should call rewardUser if async is not enabled', async function () {
             // given
-            config.featureToggles.isQuestEnabled = true;
             config.featureToggles.isAsyncQuestRewardingCalculationEnabled = false;
 
             // when
@@ -199,6 +201,22 @@ describe('Unit | Controller | answer-controller', function () {
             // then
             expect(questUsecases.rewardUser).to.have.been.calledWith({ userId });
           });
+
+          context('when userId is not provided', function () {
+            it('should not call the reward user usecase', async function () {
+              // given
+              requestResponseUtilsStub.extractUserIdFromRequest.returns(null);
+
+              // when
+              await answerController.save(request, hFake, {
+                answerSerializer: answerSerializerStub,
+                requestResponseUtils: requestResponseUtilsStub,
+              });
+
+              // then
+              expect(questUsecases.rewardUser).to.not.have.been.called;
+            });
+          });
         });
       });
     });

diff --git a/...ation-learner/unit/domain/usecases/find-organization-learners-with-participations_test.js b/...ation-learner/unit/domain/usecases/find-organization-learners-with-participations_test.js
@@ -1,7 +1,25 @@
-import { _getCampaignParticipationOverviewsWithoutPagination } from '../../../../../../src/prescription/organization-learner/domain/usecases/find-organization-learners-with-participations.js';
+import {
+  _getCampaignParticipationOverviewsWithoutPagination,
+  findOrganizationLearnersWithParticipations,
+} from '../../../../../../src/prescription/organization-learner/domain/usecases/find-organization-learners-with-participations.js';
 import { expect, sinon } from '../../../../../test-helper.js';
 
 describe('Unit | UseCase | find-organization-learners-with-participations', function () {
+  context('#findOrganizationLearnersWithParticipations', function () {
+    it('should not call libOrganizationLearnerRepository if userIds is null', async function () {
+      // given
+      const libOrganizationLearnerRepository = {
+        findByUserId: sinon.stub(),
+      };
+
+      // when
+      await findOrganizationLearnersWithParticipations({ userIds: null, libOrganizationLearnerRepository });
+
+      // then
+      expect(libOrganizationLearnerRepository.findByUserId).to.not.have.been.called;
+    });
+  });
+
   context('#_getCampaignParticipationOverviewsWithoutPagination', function () {
     it('should return all campaign participations', async function () {
       // given

diff --git a/api/tests/quest/unit/domain/usecases/reward-user_test.js b/api/tests/quest/unit/domain/usecases/reward-user_test.js
@@ -8,6 +8,8 @@ describe('Quest | Unit | Domain | Usecases | RewardUser', function () {
   let successRepository;
   let rewardRepository;
 
+  let dependencies;
+
   beforeEach(function () {
     userId = 1;
 
@@ -22,6 +24,13 @@ describe('Quest | Unit | Domain | Usecases | RewardUser', function () {
     successRepository = { find: sinon.stub() };
 
     rewardRepository = { save: sinon.stub(), getByUserId: sinon.stub() };
+
+    dependencies = {
+      questRepository,
+      eligibilityRepository,
+      successRepository,
+      rewardRepository,
+    };
   });
 
   context('when there are no quests available', function () {
@@ -30,7 +39,21 @@ describe('Quest | Unit | Domain | Usecases | RewardUser', function () {
       questRepository.findAll.resolves([]);
 
       // when
-      await rewardUser({ userId, questRepository, eligibilityRepository });
+      await rewardUser({ userId, ...dependencies });
+      expect(eligibilityRepository.find).to.not.have.been.called;
+    });
+  });
+
+  context('when user id is not provided', function () {
+    it('should not call eligibility repository', async function () {
+      // given
+      const quest = { isEligible: () => false };
+      questRepository.findAll.resolves([quest]);
+      rewardRepository.getByUserId.resolves([]);
+      eligibilityRepository.find.resolves([]);
+
+      // when
+      await rewardUser({ userId: null, ...dependencies });
       expect(eligibilityRepository.find).to.not.have.been.called;
     });
   });
@@ -46,10 +69,7 @@ describe('Quest | Unit | Domain | Usecases | RewardUser', function () {
       // when
       await rewardUser({
         userId,
-        questRepository,
-        eligibilityRepository,
-        successRepository,
-        rewardRepository,
+        ...dependencies,
       });
       expect(successRepository.find).to.not.have.been.called;
     });
@@ -72,10 +92,7 @@ describe('Quest | Unit | Domain | Usecases | RewardUser', function () {
       // when
       await rewardUser({
         userId,
-        questRepository,
-        eligibilityRepository,
-        successRepository,
-        rewardRepository,
+        ...dependencies,
       });
 
       // then
@@ -99,10 +116,7 @@ describe('Quest | Unit | Domain | Usecases | RewardUser', function () {
       // when
       await rewardUser({
         userId,
-        questRepository,
-        eligibilityRepository,
-        successRepository,
-        rewardRepository,
+        ...dependencies,
       });
       expect(successRepository.find).to.not.have.been.called;
     });