From ade4870d2f2c35746057948d5033ccbdfada627c Mon Sep 17 00:00:00 2001 From: Guillaume Lagorce Date: Thu, 28 Nov 2024 12:06:48 +0100 Subject: [PATCH] bugfix(api): handle certification test when both answer and live alert exists on same challenge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Yannick François Co-authored-by: Steph0 --- .../domain/usecases/get-next-challenge.js | 11 ++-- .../repositories/answer-repository.js | 11 ++++ .../usecases/get-next-challenge_test.js | 40 +++++++++---- .../repositories/answer-repository_test.js | 59 +++++++++++++++++++ 4 files changed, 107 insertions(+), 14 deletions(-) diff --git a/api/src/certification/evaluation/domain/usecases/get-next-challenge.js b/api/src/certification/evaluation/domain/usecases/get-next-challenge.js index 2381a972f43..c2e2497ce08 100644 --- a/api/src/certification/evaluation/domain/usecases/get-next-challenge.js +++ b/api/src/certification/evaluation/domain/usecases/get-next-challenge.js @@ -44,16 +44,19 @@ const getNextChallenge = async function ({ }) { const certificationCourse = await certificationCourseRepository.get({ id: assessment.certificationCourseId }); - const allAnswers = await answerRepository.findByAssessment(assessment.id); - const alreadyAnsweredChallengeIds = allAnswers.map(({ challengeId }) => challengeId); - const validatedLiveAlertChallengeIds = await _getValidatedLiveAlertChallengeIds({ assessmentId: assessment.id, certificationChallengeLiveAlertRepository, }); - const excludedChallengeIds = [...alreadyAnsweredChallengeIds, ...validatedLiveAlertChallengeIds]; + const allAnswers = await answerRepository.findByAssessmentExcludingChallengeIds({ + assessmentId: assessment.id, + excludedChallengeIds: validatedLiveAlertChallengeIds, + }); + + const alreadyAnsweredChallengeIds = allAnswers.map(({ challengeId }) => challengeId); + const excludedChallengeIds = [...alreadyAnsweredChallengeIds, ...validatedLiveAlertChallengeIds]; const lastNonAnsweredCertificationChallenge = await sessionManagementCertificationChallengeRepository.getNextChallengeByCourseIdForV3( assessment.certificationCourseId, diff --git a/api/src/shared/infrastructure/repositories/answer-repository.js b/api/src/shared/infrastructure/repositories/answer-repository.js index a982244199b..59eb4abe783 100644 --- a/api/src/shared/infrastructure/repositories/answer-repository.js +++ b/api/src/shared/infrastructure/repositories/answer-repository.js @@ -81,6 +81,16 @@ const findByAssessment = async function (assessmentId) { return _toDomainArray(answerDTOsWithoutDuplicate); }; +const findByAssessmentExcludingChallengeIds = async function ({ assessmentId, excludedChallengeIds = [] }) { + const answerDTOs = await knex + .select(COLUMNS) + .from('answers') + .where({ assessmentId }) + .whereNotIn('challengeId', excludedChallengeIds); + + return _toDomainArray(answerDTOs); +}; + const findChallengeIdsFromAnswerIds = async function (ids) { return knex.distinct().pluck('challengeId').from('answers').whereInArray('id', ids); }; @@ -108,6 +118,7 @@ const saveWithKnowledgeElements = async function (answer, knowledgeElements) { }; export { findByAssessment, + findByAssessmentExcludingChallengeIds, findByChallengeAndAssessment, findChallengeIdsFromAnswerIds, get, diff --git a/api/tests/certification/evaluation/unit/domain/usecases/get-next-challenge_test.js b/api/tests/certification/evaluation/unit/domain/usecases/get-next-challenge_test.js index 40cea9cfe98..b1b747a546d 100644 --- a/api/tests/certification/evaluation/unit/domain/usecases/get-next-challenge_test.js +++ b/api/tests/certification/evaluation/unit/domain/usecases/get-next-challenge_test.js @@ -26,7 +26,7 @@ describe('Unit | Domain | Use Cases | get-next-challenge', function () { getMostRecentBeforeDate: sinon.stub(), }; answerRepository = { - findByAssessment: sinon.stub(), + findByAssessmentExcludingChallengeIds: sinon.stub(), }; challengeRepository = { get: sinon.stub(), @@ -79,7 +79,9 @@ describe('Unit | Domain | Use Cases | get-next-challenge', function () { .withArgs(v3CertificationCourse.getStartDate()) .resolves(flashAlgorithmConfiguration); - answerRepository.findByAssessment.withArgs(assessment.id).resolves([]); + answerRepository.findByAssessmentExcludingChallengeIds + .withArgs({ assessmentId: assessment.id, excludedChallengeIds: [] }) + .resolves([]); certificationChallengeLiveAlertRepository.getLiveAlertValidatedChallengeIdsByAssessmentId .withArgs({ assessmentId: assessment.id }) .resolves([]); @@ -170,7 +172,9 @@ describe('Unit | Domain | Use Cases | get-next-challenge', function () { .withArgs(v3CertificationCourse.getStartDate()) .resolves(flashAlgorithmConfiguration); - answerRepository.findByAssessment.withArgs(assessment.id).resolves([]); + answerRepository.findByAssessmentExcludingChallengeIds + .withArgs({ assessmentId: assessment.id, excludedChallengeIds: [] }) + .resolves([]); certificationChallengeLiveAlertRepository.getLiveAlertValidatedChallengeIdsByAssessmentId .withArgs({ assessmentId: assessment.id }) .resolves([]); @@ -259,7 +263,9 @@ describe('Unit | Domain | Use Cases | get-next-challenge', function () { id: nonAnsweredCertificationChallenge.challengeId, }); - answerRepository.findByAssessment.withArgs(assessment.id).resolves([]); + answerRepository.findByAssessmentExcludingChallengeIds + .withArgs({ assessmentId: assessment.id, excludedChallengeIds: [] }) + .resolves([]); certificationChallengeLiveAlertRepository.getLiveAlertValidatedChallengeIdsByAssessmentId .withArgs({ assessmentId: assessment.id }) .resolves([]); @@ -319,8 +325,8 @@ describe('Unit | Domain | Use Cases | get-next-challenge', function () { const answerStillValid = domainBuilder.buildAnswer({ challengeId: alreadyAnsweredChallenge.id }); const answerWithOutdatedChallenge = domainBuilder.buildAnswer({ challengeId: outdatedChallenge.id }); - answerRepository.findByAssessment - .withArgs(assessment.id) + answerRepository.findByAssessmentExcludingChallengeIds + .withArgs({ assessmentId: assessment.id, excludedChallengeIds: [] }) .resolves([answerStillValid, answerWithOutdatedChallenge]); certificationChallengeLiveAlertRepository.getLiveAlertValidatedChallengeIdsByAssessmentId @@ -418,7 +424,12 @@ describe('Unit | Domain | Use Cases | get-next-challenge', function () { .withArgs(v3CertificationCourse.getStartDate()) .resolves(flashAlgorithmConfiguration); - answerRepository.findByAssessment.withArgs(assessment.id).resolves([]); + answerRepository.findByAssessmentExcludingChallengeIds + .withArgs({ + assessmentId: assessment.id, + excludedChallengeIds: [nonAnsweredCertificationChallenge.challengeId], + }) + .resolves([]); challengeRepository.findActiveFlashCompatible.withArgs({ locale }).resolves([nextChallenge, lastSeenChallenge]); challengeRepository.getMany.withArgs([]).resolves([]); @@ -514,7 +525,12 @@ describe('Unit | Domain | Use Cases | get-next-challenge', function () { .withArgs(v3CertificationCourse.getStartDate()) .resolves(flashAlgorithmConfiguration); - answerRepository.findByAssessment.withArgs(assessment.id).resolves([]); + answerRepository.findByAssessmentExcludingChallengeIds + .withArgs({ + assessmentId: assessment.id, + excludedChallengeIds: [nonAnsweredCertificationChallenge.challengeId], + }) + .resolves([]); challengeRepository.findActiveFlashCompatible .withArgs() .resolves([challengeWithLiveAlert, challengeWithOtherSkill, challengeWithLiveAlertedSkill]); @@ -600,7 +616,9 @@ describe('Unit | Domain | Use Cases | get-next-challenge', function () { .withArgs(v3CertificationCourse.getStartDate()) .resolves(flashAlgorithmConfiguration); - answerRepository.findByAssessment.withArgs(assessment.id).resolves([answer]); + answerRepository.findByAssessmentExcludingChallengeIds + .withArgs({ assessmentId: assessment.id, excludedChallengeIds: [] }) + .resolves([answer]); certificationChallengeLiveAlertRepository.getLiveAlertValidatedChallengeIdsByAssessmentId .withArgs({ assessmentId: assessment.id }) .resolves([]); @@ -679,7 +697,9 @@ describe('Unit | Domain | Use Cases | get-next-challenge', function () { const assessment = domainBuilder.buildAssessment(); const locale = 'fr-FR'; - answerRepository.findByAssessment.withArgs(assessment.id).resolves([]); + answerRepository.findByAssessmentExcludingChallengeIds + .withArgs({ assessmentId: assessment.id, excludedChallengeIds: [] }) + .resolves([]); certificationChallengeLiveAlertRepository.getLiveAlertValidatedChallengeIdsByAssessmentId .withArgs({ assessmentId: assessment.id }) .resolves([]); diff --git a/api/tests/shared/integration/infrastructure/repositories/answer-repository_test.js b/api/tests/shared/integration/infrastructure/repositories/answer-repository_test.js index 91370125c26..5a6d86b84b1 100644 --- a/api/tests/shared/integration/infrastructure/repositories/answer-repository_test.js +++ b/api/tests/shared/integration/infrastructure/repositories/answer-repository_test.js @@ -253,6 +253,65 @@ describe('Integration | Repository | answerRepository', function () { }); }); + describe('#findByAssessmentExcludingChallenges', function () { + context('when assessment does not exist', function () { + it('should return an empty array', async function () { + // given + databaseBuilder.factory.buildAssessment({ id: 123 }); + databaseBuilder.factory.buildAnswer({ assessmentId: 123 }); + await databaseBuilder.commit(); + + // when + const foundAnswers = await answerRepository.findByAssessmentExcludingChallengeIds({ assessmentId: 456 }); + + // then + expect(foundAnswers).to.be.empty; + }); + }); + context('when assessment exists', function () { + context('when excludingChallengeIds is not provided', function () { + it('should return all answers', async function () { + // given + databaseBuilder.factory.buildAssessment({ id: 123 }); + const expectedAnswer = domainBuilder.buildAnswer({ assessmentId: 123 }); + databaseBuilder.factory.buildAnswer({ ...expectedAnswer, result: expectedAnswer.result.status }); + await databaseBuilder.commit(); + + // when + const foundAnswers = await answerRepository.findByAssessmentExcludingChallengeIds({ assessmentId: 123 }); + + // then + expect(foundAnswers).to.deep.equal([expectedAnswer]); + }); + }); + + context('when excludingChallengeIds is provided', function () { + it('should return answers except the ones excluded', async function () { + // given + databaseBuilder.factory.buildAssessment({ id: 123 }); + const expectedAnswer = domainBuilder.buildAnswer({ assessmentId: 123 }); + const excludedAnswer = domainBuilder.buildAnswer({ + id: 456, + assessmentId: 123, + challengeId: 'excludedChallengeId', + }); + databaseBuilder.factory.buildAnswer({ ...expectedAnswer, result: expectedAnswer.result.status }); + databaseBuilder.factory.buildAnswer({ ...excludedAnswer, result: excludedAnswer.result.status }); + await databaseBuilder.commit(); + + // when + const foundAnswers = await answerRepository.findByAssessmentExcludingChallengeIds({ + assessmentId: 123, + excludedChallengeIds: [excludedAnswer.challengeId], + }); + + // then + expect(foundAnswers).to.deep.equal([expectedAnswer]); + }); + }); + }); + }); + describe('#findChallengeIdsFromAnswerIds', function () { context('when provided answerIds collection is empty', function () { it('should return an empty array', async function () {