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

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 });
   }
 

api/src/prescription/organization-learner/domain/usecases/find-organization-learners-with-participations.js

@@ -1,12 +1,22 @@
+import Joi from 'joi';
+
 import { withTransaction } from '../../../../shared/domain/DomainTransaction.js';
 
+const userIdsSchema = Joi.array().items(Joi.number());
+
 const findOrganizationLearnersWithParticipations = withTransaction(async function ({
   userIds,
   campaignParticipationOverviewRepository,
   organizationRepository,
   libOrganizationLearnerRepository,
   tagRepository,
 }) {
+  const validationResult = userIdsSchema.validate(userIds);
+
+  if (validationResult.error) {
+    return [];
+  }
+
   const organizationLearners = (
     await Promise.all(
       userIds.map((userId) => {

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) {

api/tests/evaluation/unit/application/answers/answer-controller_test.js

@@ -18,6 +18,7 @@ describe('Unit | Controller | answer-controller', function () {
       extractLocaleFromRequest: sinon.stub(),
     };
     sinon.stub(usecases, 'correctAnswerThenUpdateAssessment');
+    sinon.stub(questUsecases, 'rewardUser');
   });
 
   describe('#save', function () {
@@ -120,28 +121,39 @@ describe('Unit | Controller | answer-controller', function () {
         usecases.correctAnswerThenUpdateAssessment.resolves(createdAnswer);
         requestResponseUtilsStub.extractUserIdFromRequest.withArgs(request).returns(userId);
         requestResponseUtilsStub.extractLocaleFromRequest.withArgs(request).returns(locale);
+      });
 
+      it('should call the usecase to save the answer', async function () {
         // when
         response = await answerController.save(request, hFake, {
           answerSerializer: answerSerializerStub,
           requestResponseUtils: requestResponseUtilsStub,
         });
-      });
 
-      it('should call the usecase to save the answer', function () {
         // then
         expect(usecases.correctAnswerThenUpdateAssessment).to.have.been.calledWithExactly({
           answer: deserializedAnswer,
           userId,
           locale,
         });
       });
+      it('should serialize the answer', async function () {
+        // when
+        response = await answerController.save(request, hFake, {
+          answerSerializer: answerSerializerStub,
+          requestResponseUtils: requestResponseUtilsStub,
+        });
 
-      it('should serialize the answer', function () {
         // then
         expect(answerSerializerStub.serialize).to.have.been.calledWithExactly(createdAnswer);
       });
-      it('should return the serialized answer', function () {
+      it('should return the serialized answer', async function () {
+        // when
+        response = await answerController.save(request, hFake, {
+          answerSerializer: answerSerializerStub,
+          requestResponseUtils: requestResponseUtilsStub,
+        });
+
         // then
         expect(response.source).to.deep.equal(serializedAnswer);
         expect(response.statusCode).to.equal(201);
@@ -150,7 +162,6 @@ describe('Unit | Controller | answer-controller', function () {
       context('quests', function () {
         beforeEach(function () {
           sinon.stub(config, 'featureToggles');
-          sinon.stub(questUsecases, 'rewardUser');
         });
 
         context('when quest feature is not enabled', function () {
@@ -199,6 +210,22 @@ describe('Unit | Controller | answer-controller', function () {
             // then
             expect(questUsecases.rewardUser).to.have.been.calledWith({ userId });
           });
+
+          it('should not call the reward user usecase if userId is not provided', async function () {
+            // given
+            config.featureToggles.isQuestEnabled = true;
+            config.featureToggles.isAsyncQuestRewardingCalculationEnabled = false;
+            requestResponseUtilsStub.extractUserIdFromRequest.withArgs(request).returns(null);
+
+            // when
+            await answerController.save(request, hFake, {
+              answerSerializer: answerSerializerStub,
+              requestResponseUtils: requestResponseUtilsStub,
+            });
+
+            // then
+            expect(questUsecases.rewardUser).to.not.have.been.called;
+          });
         });
       });
     });

api/tests/prescription/organization-learner/unit/domain/usecases/find-organization-learners-with-participations_test.js

@@ -1,7 +1,30 @@
-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 { DomainTransaction } from '../../../../../../src/shared/domain/DomainTransaction.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 does not have the expected format', async function () {
+      // given
+      sinon.stub(DomainTransaction, 'execute').callsFake((lambda) => lambda());
+      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

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;
     });