Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TECH] Ne pas lever d'exception lors du lancement des seeds si le référentiel contient un acquis sans épreuve fr-fr validée (PIX-13451). #9528

Merged
merged 3 commits into from
Jul 16, 2024

Conversation

HEYGUL
Copy link
Contributor

@HEYGUL HEYGUL commented Jul 15, 2024

🦄 Problème

Lundi 15 juillet, nous avons été dans une situation où le réféerntiel contenait un acquis sans épreuve fr-fr validée.
Bien que cela nee devrait jamais arrivé, cela a mis en lumière que le script de seeds ne savait pas gérer cet état sans lever d'exception.
Cela est embêtant pour les environnements locaux et surtout pour les Review Apps.

Le script de seed s'arrêtait avec le message suivant :

Error while executing "/Users/alexandre/1024Pix/pix/api/db/seeds/seed.js" seed: Cannot read properties of undefined (reading '0')
Error: Error while executing "/Users/alexandre/1024Pix/pix/api/db/seeds/seed.js" seed: Cannot read properties of undefined (reading '0')
    at Seeder._waterfallBatch (/Users/alexandre/1024Pix/pix/api/node_modules/knex/lib/migrations/seed/Seeder.js:118:23)
TypeError: Cannot read properties of undefined (reading '0')
    at Module.findFirstValidatedChallengeBySkillId (file:///Users/alexandre/1024Pix/pix/api/db/seeds/data/common/tooling/learning-content.js:54:45)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async _makeCandidatesComplementaryCertificationCertifiable (file:///Users/alexandre/1024Pix/pix/api/db/seeds/data/common/tooling/session-tooling.js:1018:23)
    at async _makeCandidatesComplementaryCertifiable (file:///Users/alexandre/1024Pix/pix/api/db/seeds/data/common/tooling/session-tooling.js:939:9)
    at async _makeCandidatesCertifiable (file:///Users/alexandre/1024Pix/pix/api/db/seeds/data/common/tooling/session-tooling.js:865:45)
    at async Module.createPublishedSession (file:///Users/alexandre/1024Pix/pix/api/db/seeds/data/common/tooling/session-tooling.js:524:71)
    at async _createPublishedSession (file:///Users/alexandre/1024Pix/pix/api/db/seeds/data/team-certification/data-builder.js:477:3)
    at async teamCertificationDataBuilder (file:///Users/alexandre/1024Pix/pix/api/db/seeds/data/team-certification/data-builder.js:74:3)
    at async Module.seed (file:///Users/alexandre/1024Pix/pix/api/db/seeds/seed.js:67:5)
    at async Seeder._waterfallBatch (/Users/alexandre/1024Pix/pix/api/node_modules/knex/lib/migrations/seed/Seeder.js:115:9)

🤖 Proposition

Faire en sorte que l'acquis ne contenant aucune épreuve fr-fr validée soit ignorée.
Ajouter un message d'avertissement (avec la fonction logger.warn) pour avertir qu'un acquis ne contient pas d'épreuve valide.
Le message est le suivant :

WARN: Skill <skillId> has no validated challenges

🌈 Remarques

En exécutant le script de seed dans le contexte CERTIF uniquement, on constate que celui-ci se termine avec une erreur.
Après investigation, cela est du au fait qu'un AssessmentResult est créé avec comme juryId l'identifiant 9000 qui correspond à un utilisateur créé par le contexte Pix Junior.
Pour garantir l'isolation du contexte CERTIF, on change le juryId par un identifiant créé dans le contexte CERTIF.

💯 Pour tester

Dans un environnement "neuf" (après avoir supprimé les conteneurs), exécuter le script npm run db:seed et vérifier que celui-ci se termine sans erreur.

@HEYGUL HEYGUL requested review from a team July 15, 2024 20:06
@HEYGUL HEYGUL self-assigned this Jul 15, 2024
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

Copy link
Contributor

@Steph0 Steph0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je propose de compléter ensemble la PR car j'avais identifié deux autres ensembles de seeds impactés par ce soucis.

Par ailleurs cette PR n'est pas lié à un ticket, comment j'avais commencé à y regarder de mon côté hier soir dans le train, je propose de se servir du numéro de ticket sur lequel j'avançais : PIX-13451

@HEYGUL HEYGUL changed the title [TECH] Ne pas lever d'exception lors du lancement des seeds si le référentiel contient un acquis sans épreuve fr-fr validée. [TECH] Ne pas lever d'exception lors du lancement des seeds si le référentiel contient un acquis sans épreuve fr-fr validée (PIX-13451). Jul 16, 2024
@HEYGUL HEYGUL removed the request for review from a team July 16, 2024 07:23
@Steph0 Steph0 force-pushed the tech-improve-seed-isolation branch 4 times, most recently from 99d1487 to 27cb16b Compare July 16, 2024 09:40
Copy link
Contributor

@Steph0 Steph0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai proposé un commit supplémentaire

HEYGUL and others added 3 commits July 16, 2024 14:16
@pix-service-auto-merge pix-service-auto-merge merged commit 58a46d1 into dev Jul 16, 2024
6 of 7 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the tech-improve-seed-isolation branch July 16, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-team Toutes les équipes de dev Func Review OK PO validated functionally the PR 🚀 Ready to Merge Tech Review OK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants