Skip to content

Commit

Permalink
[BUGFIX] Nouveaux repositories learning content : résultats en doublon (
Browse files Browse the repository at this point in the history
  • Loading branch information
pix-service-auto-merge authored Dec 10, 2024
2 parents 6ed7f34 + 6aa705c commit 289e174
Show file tree
Hide file tree
Showing 17 changed files with 679 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export async function getByName(name) {

export async function findByRecordIds(ids) {
if (!config.featureToggles.useNewLearningContent) return oldFrameworkRepository.findByRecordIds(ids);
const frameworkDtos = await getInstance().loadMany(ids);
const frameworkDtos = await getInstance().getMany(ids);
return frameworkDtos
.filter((frameworkDto) => frameworkDto)
.sort(byName)
Expand Down
2 changes: 1 addition & 1 deletion api/lib/infrastructure/repositories/thematic-repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export async function findByCompetenceIds(competenceIds, locale) {

export async function findByRecordIds(ids, locale) {
if (!config.featureToggles.useNewLearningContent) return oldThematicRepository.findByRecordIds(ids, locale);
const thematicDtos = await getInstance().loadMany(ids);
const thematicDtos = await getInstance().getMany(ids);
return thematicDtos
.filter((thematic) => thematic)
.map((thematicDto) => toDomain(thematicDto, locale))
Expand Down
3 changes: 2 additions & 1 deletion api/lib/infrastructure/repositories/tube-repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const TABLE_NAME = 'learningcontent.tubes';
const ACTIVE_STATUS = 'actif';

export async function get(id) {
if (!config.featureToggles.useNewLearningContent) return oldTubeRepository.get(id);
const tubeDto = await getInstance().load(id);
if (!tubeDto) {
throw new LearningContentResourceNotFound();
Expand All @@ -34,7 +35,7 @@ export async function findByNames({ tubeNames, locale }) {

export async function findByRecordIds(ids, locale) {
if (!config.featureToggles.useNewLearningContent) return oldTubeRepository.findByRecordIds(ids, locale);
const tubeDtos = await getInstance().loadMany(ids);
const tubeDtos = await getInstance().getMany(ids);
return toDomainList(
tubeDtos.filter((tubeDto) => tubeDto),
locale,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const TABLE_NAME = 'learningcontent.tutorials';
export async function findByRecordIdsForCurrentUser({ ids, userId, locale }) {
if (!config.featureToggles.useNewLearningContent)
return oldTutorialRepository.findByRecordIdsForCurrentUser({ ids, userId, locale });
let tutorialDtos = await getInstance().loadMany(ids);
let tutorialDtos = await getInstance().getMany(ids);
tutorialDtos = tutorialDtos.filter((tutorialDto) => tutorialDto);
if (locale) {
const lang = extractLangFromLocale(locale);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export async function findByFrameworkId({ frameworkId, locale }) {

export async function findByRecordIds({ areaIds, locale }) {
if (!config.featureToggles.useNewLearningContent) return oldAreaRepository.findByRecordIds({ areaIds, locale });
const areaDtos = await getInstance().loadMany(areaIds);
const areaDtos = await getInstance().getMany(areaIds);
return areaDtos
.filter((areaDto) => areaDto)
.sort(byId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,15 @@ const findValidatedBySkillId = async function (skillId, locale) {
};

export async function getManyTypes(ids) {
const challenges = await challengeDatasource.getMany(ids);
return Object.fromEntries(challenges.map(({ id, type }) => [id, type]));
try {
const challenges = await challengeDatasource.getMany(ids);
return Object.fromEntries(challenges.map(({ id, type }) => [id, type]));
} catch (err) {
if (err instanceof LearningContentResourceNotFound) {
throw new NotFoundError();
}
throw err;
}
}

function byId(challenge1, challenge2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export async function getCompetenceName({ id, locale }) {
export async function findByRecordIds({ competenceIds, locale }) {
if (!config.featureToggles.useNewLearningContent)
return oldCompetenceRepository.findByRecordIds({ competenceIds, locale });
const competenceDtos = await getInstance().loadMany(competenceIds);
const competenceDtos = await getInstance().getMany(competenceIds);
return competenceDtos
.filter((competenceDto) => competenceDto)
.sort(byId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,21 +66,35 @@ export class LearningContentRepository {
/**
* Loads one entity by ID.
* @param {string|number} id
* @returns {Promise<object>}
* @returns {Promise<object|null>}
*/
async load(id) {
if (!id) return null;
return this.#dataloader.load(id);
}

/**
* Gets several entities by ID.
* Deduplicates ids and removes nullish ids before loading.
* @param {string[]|number[]} ids
* @returns {Promise<(object|null)[]>}
*/
async getMany(ids) {
const idsToLoad = new Set(ids);
idsToLoad.delete(undefined);
idsToLoad.delete(null);

const dtos = await this.loadMany(Array.from(idsToLoad));

return dtos;
}

/**
* Loads several entities by ID.
* @param {string[]|number[]} ids
* @returns {Promise<object[]>}
* @returns {Promise<(object|null)[]>}
*/
async loadMany(ids) {
const notNullIds = ids.filter((id) => id);
return this.#dataloader.loadMany(notNullIds);
return this.#dataloader.loadMany(ids);
}

/**
Expand Down Expand Up @@ -123,7 +137,7 @@ export class LearningContentRepository {
* @param {string|number|undefined} id
*/
clearCache(id) {
logger.debug({ tableName: this.#tableName, id }, 'trigerring cache clear');
logger.debug({ tableName: this.#tableName, id }, 'triggering cache clear');

if (id) {
this.#dataloader.clear(id);
Expand Down
21 changes: 12 additions & 9 deletions api/src/shared/infrastructure/repositories/skill-repository.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { knex } from '../../../../db/knex-database-connection.js';
import { config } from '../../config.js';
import { NotFoundError } from '../../domain/errors.js';
import { Skill } from '../../domain/models/Skill.js';
Expand Down Expand Up @@ -64,17 +65,19 @@ export async function findOperativeByCompetenceId(competenceId) {
export async function findOperativeByCompetenceIds(competenceIds) {
if (!config.featureToggles.useNewLearningContent)
return oldSkillRepository.findOperativeByCompetenceIds(competenceIds);
const skills = [];
for (const competenceId of competenceIds) {
const skillsForCompetence = await findOperativeByCompetenceId(competenceId);
skills.push(...skillsForCompetence);
}
return skills.sort(byId);
const ids = await knex
.pluck('id')
.from(TABLE_NAME)
.whereIn('competenceId', competenceIds)
.whereIn('status', OPERATIVE_STATUSES)
.orderBy('id');
const skillDtos = await getInstance().loadMany(ids);
return skillDtos.map(toDomain);
}

export async function findOperativeByIds(ids) {
if (!config.featureToggles.useNewLearningContent) return oldSkillRepository.findOperativeByIds(ids);
const skillDtos = await getInstance().loadMany(ids);
const skillDtos = await getInstance().getMany(ids);
return skillDtos
.filter((skillDto) => skillDto && OPERATIVE_STATUSES.includes(skillDto.status))
.sort(byId)
Expand All @@ -83,7 +86,7 @@ export async function findOperativeByIds(ids) {

export async function findByRecordIds(ids) {
if (!config.featureToggles.useNewLearningContent) return oldSkillRepository.findByRecordIds(ids);
const skillDtos = await getInstance().loadMany(ids);
const skillDtos = await getInstance().getMany(ids);
return skillDtos
.filter((skillDto) => skillDto)
.sort(byId)
Expand All @@ -92,7 +95,7 @@ export async function findByRecordIds(ids) {

export async function findActiveByRecordIds(ids) {
if (!config.featureToggles.useNewLearningContent) return oldSkillRepository.findActiveByRecordIds(ids);
const skillDtos = await getInstance().loadMany(ids);
const skillDtos = await getInstance().getMany(ids);
return skillDtos
.filter((skillDto) => skillDto && skillDto.status === ACTIVE_STATUS)
.sort(byId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { catchErr, databaseBuilder, domainBuilder, expect, mockLearningContent }
const { ENGLISH_SPOKEN } = LOCALE;

describe('Integration | Repository | tutorial-repository', function () {
testTurorialRepository(); // eslint-disable-line mocha/no-setup-in-describe
testTutorialRepository(); // eslint-disable-line mocha/no-setup-in-describe

describe('when using old learning content', function () {
beforeEach(function () {
Expand All @@ -24,10 +24,10 @@ describe('Integration | Repository | tutorial-repository', function () {
config.featureToggles.useNewLearningContent = true;
});

testTurorialRepository(); // eslint-disable-line mocha/no-setup-in-describe
testTutorialRepository(); // eslint-disable-line mocha/no-setup-in-describe
});

function testTurorialRepository() {
function testTutorialRepository() {
describe('#findByRecordIdsForCurrentUser', function () {
it('should find tutorials by ids', async function () {
// given
Expand Down Expand Up @@ -72,6 +72,49 @@ describe('Integration | Repository | tutorial-repository', function () {
expect(tutorials).to.deep.include.members(tutorialsList);
});

it('should avoid duplicates and nulls', async function () {
// given
const tutorialsList = [
{
duration: '00:00:54',
format: 'video',
link: 'https://tuto.fr',
source: 'tuto.fr',
title: 'tuto0',
id: 'recTutorial0',
skillId: undefined,
userSavedTutorial: undefined,
tutorialEvaluation: undefined,
},
{
duration: '00:01:54',
format: 'page',
link: 'https://tuto.com',
source: 'tuto.com',
title: 'tuto1',
id: 'recTutorial1',
skillId: undefined,
userSavedTutorial: undefined,
tutorialEvaluation: undefined,
},
];

await mockLearningContent({
tutorials: tutorialsList,
});

// when
const tutorials = await tutorialRepository.findByRecordIdsForCurrentUser({
ids: ['recTutorial0', 'recTutorial1', 'recCOUCOUPAPA', 'recTutorial1'],
userId: null,
});

// then
expect(tutorials).to.have.lengthOf(2);
expect(tutorials[0]).to.be.instanceof(TutorialForUser);
expect(tutorials).to.deep.include.members(tutorialsList);
});

it('should associate userSavedTutorial when it exists for provided user', async function () {
// given
const userId = databaseBuilder.factory.buildUser().id;
Expand Down Expand Up @@ -856,6 +899,79 @@ describe('Integration | Repository | tutorial-repository', function () {
tutorialId: 'tuto4',
});
});

it('should avoid nulls and duplicates', async function () {
// given
databaseBuilder.factory.buildKnowledgeElement({
skillId: 'recSkill3',
userId,
status: KnowledgeElement.StatusType.INVALIDATED,
source: KnowledgeElement.SourceType.DIRECT,
createdAt: new Date('2000-01-01'),
});
databaseBuilder.factory.buildKnowledgeElement({
skillId: 'recSkill3',
userId,
status: KnowledgeElement.StatusType.INVALIDATED,
source: KnowledgeElement.SourceType.DIRECT,
createdAt: new Date('2000-01-02'),
});
databaseBuilder.factory.buildKnowledgeElement({
skillId: 'recSkillWithoutTuto',
userId,
status: KnowledgeElement.StatusType.INVALIDATED,
source: KnowledgeElement.SourceType.DIRECT,
createdAt: new Date('2000-01-02'),
});
const userSavedTutorialId = databaseBuilder.factory.buildUserSavedTutorial({
userId,
tutorialId: 'tuto4',
skillId: 'recSkill3',
}).id;
const tutorialEvaluationId = databaseBuilder.factory.buildTutorialEvaluation({
tutorialId: 'tuto4',
userId,
}).id;
await databaseBuilder.commit();
await mockLearningContent({
tutorials: [
{
id: 'tuto4',
locale: 'fr-fr',
},
],
skills: [
{
id: 'recSkill3',
tutorialIds: ['tuto4'],
status: 'actif',
},
{
id: 'recSkillWithoutTuto',
tutorialIds: [],
status: 'actif',
},
],
});

// when
const { results } = await tutorialRepository.findPaginatedFilteredRecommendedByUserId({ userId });

// then
expect(results.length).to.equal(1);
expect(results[0].tutorialEvaluation).to.include({
id: tutorialEvaluationId,
userId,
tutorialId: 'tuto4',
status: TutorialEvaluation.statuses.LIKED,
});
expect(results[0].userSavedTutorial).to.include({
id: userSavedTutorialId,
userId,
skillId: 'recSkill3',
tutorialId: 'tuto4',
});
});
});

describe('when tutorials amount exceed page size', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,27 @@ describe('Integration | Repository | area-repository', function () {
]);
});
});

it('should ignore null and duplicates', async function () {
// when
const areas = await areaRepository.findByRecordIds({
areaIds: ['recArea2', 'recArea0', 'recCOUCOUMAMAN', 'recArea0'],
});

// then
expect(areas).to.deepEqualArray([
domainBuilder.buildArea({
...areaData0,
title: areaData0.title_i18n.fr,
competences: [],
}),
domainBuilder.buildArea({
...areaData2,
title: areaData2.title_i18n.fr,
competences: [],
}),
]);
});
});

context('when no areas found for given ids', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ describe('Integration | Repository | framework-repository', function () {
expect(frameworks).to.deepEqualArray([expectedFramework0, expectedFramework2]);
});

it('should return frameworks it managed to find', async function () {
it('should return frameworks it managed to find once', async function () {
// when
const frameworks = await frameworkRepository.findByRecordIds(['recId2', 'recIdCAVA']);
const frameworks = await frameworkRepository.findByRecordIds(['recId2', 'recIdCAVA', 'recId2']);

// then
const expectedFramework2 = domainBuilder.buildFramework({ ...frameworkData2, areas: [] });
Expand Down
Loading

0 comments on commit 289e174

Please sign in to comment.