[TECH] Refacto des badges des certifications complémentaires (PIX-9862) #7447

aceol · 2023-11-13T15:14:45Z

🦄 Problème

La table complementary-certification-course-results stocke la clé (partner key) du badge acquis. Ce n’est pas une clé étrangère et ne permet pas de faire aisaiment des jointure. Globalement cela prête à confusion

🤖 Proposition

Supprimer la colonne partnerKey qui se refere au champs key de la table badge et la remplacer par une jointure vers la table complementary-certification-badges

🌈 Remarques

Le juryLevel envoyé par le front pour mettre à jour le resultat du jury passe d'une clé (CLEA) a un ID (10002). Je n'ai pas renommé le champs. ca peut se discuter
La PR est tres grosse, elle pourrait etre decoupée mais ca necessiterais de faire plus de tests func...

💯 Pour tester

Creer une session avec une certif droit ou clea et une autre certif edu:

NODE_ENV= npm i [email protected] && LOG_LEVEL=info LOG_FOR_HUMANS=true node ./scripts/data-generation/generate-certif-cli.js 'PRO' 2 '[{"candidateNumber": 1, "key": "CLEA"}, {"candidateNumber": 2, "key": "EDU_2ND_DEGRE"}]'

passer la certif avec les 2 candidats
finaliser
remplir le jury externe pour la certif edu
publier

S'assurer que l'on voit bien l'obtention des certifs coté admin et coté user
Ce cas est passé sur la session suivante: https://admin-pr7447.review.pix.fr/sessions/1000015/certifications

pix-bot-github · 2023-11-13T15:16:25Z

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

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

api/lib/domain/models/ComplementaryCertificationCourseResult.js

P-Jeremy · 2023-11-23T10:46:07Z

api/lib/application/complementary-certification-course-results/index.js

@@ -13,7 +13,7 @@ const register = async function (server) {
          payload: Joi.object({
            data: {
              attributes: {
-                juryLevel: Joi.string().required(),
+                juryLevel: Joi.required(),


Question: Pourquoi le jury level peut être désormais un id ? 🤔

La reponse est vaguement dans les remarque de PR.
Les resultats de certifs comp stockent maintenant non plus une clé de badge mais un id.
On remplace donc ici aussi. On aurait pu choisir de garder la key (mais ca aurait ete un peu etrange), de prendre le level justement (mais il faudrait ensuite retrouver le bon id derriere) ou bien de prendre cet id

Du coup je comprends mieux mais ça serait peut être mieux de renommer la variable, juryLevel en juryLevelComplementaryCertificationBadgeId ou juryComplementaryCertificationBadgeId maintenant

aceol · 2023-11-23T19:39:09Z

Je propose de

renommer juryLevel en juryComplementaryCertificationBadgeId (comme suggéré par @P-Jeremy )
limiter cette prop via Joi à Number | REJECTED | UNSET

aceol · 2023-11-24T12:44:23Z

Ajout d'un commit pour ajouter un check joi sur le format du JuryLevel
✨ api: Limit juryLevel to id or specific states

AndreiaPena · 2023-11-27T14:28:11Z

Vu ensemble: on ne supprime pas la colonne partnerKey de complementary-certification-course-results ici.
A faire dans une autre PR.

api/tests/integration/infrastructure/repositories/jury-certification-repository_test.js

api/lib/infrastructure/repositories/certification-result-repository.js

AndreiaPena · 2023-11-27T15:38:46Z

api/db/migrations/20231106161726_add-cc-course-results-to-cc-badges-reference.js

+      .withKeyName('cccresults-ccbadgeId_foreignkey');
+  });
+
+  await knex(TABLE_NAME).update({


Pas trop d'inquiétude à se faire niveau perf de cette migration ?

On a moins de 3000 lignes à mettre à jour, c'est OK

api/lib/domain/events/handle-complementary-certifications-scoring.js

AndreiaPena · 2023-11-27T16:29:28Z

api/lib/application/complementary-certification-course-results/index.js

+                    identifiersType.complementaryCertificationBadgeId,
+                    Joi.string().valid(juryOptions.REJECTED).valid(juryOptions.UNSET),


Ah oui le juryLevel ça peut soit être un id soit une string dans le cas d'un jury échoué ou en attente ?

AndreiaPena · 2023-11-27T16:35:49Z

api/lib/domain/models/ComplementaryCertificationCourseResult.js

-  static buildFromJuryLevel({ complementaryCertificationCourseId, juryLevel, pixPartnerKey }) {
-    if (juryLevel === juryOptions.REJECTED) {
+  static buildFromJuryLevel({ complementaryCertificationCourseId, complementaryCertificationBadgeId, juryLevel }) {
+    if (juryLevel === 'REJECTED') {


Question : Je ne sais pas dans quel contexte on se trouve ici mais il n'y a pas un risque d'avoir un juryLevel à UNSET ici et qu'il finisse à la ligne 42 ?

api/lib/domain/models/CertificationResult.js

AndreiaPena · 2023-11-27T16:56:46Z

api/lib/infrastructure/repositories/certificate-repository.js

@@ -71,7 +71,7 @@ export { getPrivateCertificate, findPrivateCertificatesByUserId, getShareableCer
 async function _getCertifiedBadges(certificationCourseId) {
  const complementaryCertificationCourseResults = await knex
    .select(
-      'complementary-certification-course-results.partnerKey',
+      'badges.key as partnerKey',


histoire d'être iso avec le nom de colonne de cette table, mais ça suggère plus de changement dans le code

Suggested change

'badges.key as partnerKey',

'badges.key as key',

J'ai pas osé (pour limiter les changements effectivement)

api/tests/acceptance/application/certifications/index_test.js

AndreiaPena · 2023-11-27T17:09:43Z

Je propose de

renommer juryLevel en juryComplementaryCertificationBadgeId (comme suggéré par @P-Jeremy )

limiter cette prop via Joi à Number | REJECTED | UNSET

Je suis pas sur pour la proposition dujuryComplementaryCertificationBadgeId Car ce paramètre peut ne pas être une ComplementaryCertificationBadgeId mais REJECTED ou UNSET

…ication

…geId

aceol added Development in progress team-certif labels Nov 13, 2023

aceol self-assigned this Nov 13, 2023

aceol force-pushed the pix-9913-refacto-complementaries branch 3 times, most recently from 3afbf90 to b8d9468 Compare November 13, 2023 22:59

pix-service-auto-merge force-pushed the pix-9913-refacto-complementaries branch from 9c4f665 to ee19ef6 Compare November 14, 2023 10:06

Base automatically changed from pix-9913-refacto-complementaries to dev November 14, 2023 10:13

aceol force-pushed the pix-9862-refacto-complementary-certification-course-results-partnerKey branch 6 times, most recently from a6fff66 to 46c15ce Compare November 17, 2023 08:28

aceol force-pushed the pix-9862-refacto-complementary-certification-course-results-partnerKey branch 4 times, most recently from eb52bc9 to f396d3c Compare November 22, 2023 14:06

aceol marked this pull request as ready for review November 23, 2023 08:37

aceol added 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally and removed Development in progress labels Nov 23, 2023

P-Jeremy reviewed Nov 23, 2023

View reviewed changes

api/lib/domain/models/ComplementaryCertificationCourseResult.js Show resolved Hide resolved

P-Jeremy reviewed Nov 23, 2023

View reviewed changes

aceol force-pushed the pix-9862-refacto-complementary-certification-course-results-partnerKey branch from f396d3c to d6a0ece Compare November 23, 2023 19:39

aceol force-pushed the pix-9862-refacto-complementary-certification-course-results-partnerKey branch 2 times, most recently from fac0ad8 to c9ee2b0 Compare November 27, 2023 07:39

AndreiaPena added Func Review OK PO validated functionally the PR and removed 👀 Func Review Needed Need PO validation for this functionally labels Nov 27, 2023

AndreiaPena reviewed Nov 27, 2023

View reviewed changes

api/lib/domain/models/CertificationResult.js Show resolved Hide resolved

AndreiaPena reviewed Nov 27, 2023

View reviewed changes

api/tests/acceptance/application/certifications/index_test.js Outdated Show resolved Hide resolved

AndreiaPena approved these changes Nov 29, 2023

View reviewed changes

aceol force-pushed the pix-9862-refacto-complementary-certification-course-results-partnerKey branch from c9ee2b0 to de707e7 Compare November 29, 2023 14:48

aceol added 🚀 Ready to Merge and removed 👀 Tech Review Needed labels Nov 29, 2023

pix-service-auto-merge force-pushed the pix-9862-refacto-complementary-certification-course-results-partnerKey branch 2 times, most recently from 86a621b to bb29a09 Compare November 29, 2023 14:50

aceol added 5 commits November 29, 2023 16:06

✅ api: Split test as there can be only one complementary per certifif…

33b7818

…ication

♻️ api: Simplify repo for single complementary per certification

58909bf

🗃️ api: Add ccc-results foreign key cc-badges

6226051

♻️ api: add complementaryCertificationBadgeId to result

1287cb3

♻️ api: Replace join from partnerKey to complementaryCertificationBad…

8b95d88

…geId

aceol force-pushed the pix-9862-refacto-complementary-certification-course-results-partnerKey branch from bb29a09 to 9bade71 Compare November 29, 2023 15:08

aceol added 6 commits November 29, 2023 16:08

♻️ api: Update accpetance tests with complementary data

0785cf8

♻️ api: Remove readSync

86e3c86

♻️ api: Replace partnerKey by complementaryCertificationBadgeId

ab718c2

♻️ api: JuryLevel is now either an id or a string

b6728f7

♻️ api: Get jury level by complementary certification badge id

d2e14ab

✨ api: Limit juryLevel to id or specific states

caafd89

aceol force-pushed the pix-9862-refacto-complementary-certification-course-results-partnerKey branch from 9bade71 to caafd89 Compare November 29, 2023 15:09

pix-service-auto-merge merged commit 07f35de into dev Nov 29, 2023
3 checks passed

pix-service-auto-merge deleted the pix-9862-refacto-complementary-certification-course-results-partnerKey branch November 29, 2023 15:15

aceol mentioned this pull request Dec 11, 2023

[TECH] Supprime la colonne partnerKey (PIX-10162 ) #7659

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TECH] Refacto des badges des certifications complémentaires (PIX-9862) #7447

[TECH] Refacto des badges des certifications complémentaires (PIX-9862) #7447

aceol commented Nov 13, 2023 •

edited

Loading

pix-bot-github commented Nov 13, 2023

P-Jeremy Nov 23, 2023

aceol Nov 23, 2023

P-Jeremy Nov 23, 2023 •

edited

Loading

aceol commented Nov 23, 2023

aceol commented Nov 24, 2023

AndreiaPena commented Nov 27, 2023

AndreiaPena Nov 27, 2023

aceol Nov 29, 2023

AndreiaPena Nov 27, 2023

AndreiaPena Nov 27, 2023

AndreiaPena Nov 27, 2023 •

edited

Loading

aceol Nov 28, 2023

AndreiaPena commented Nov 27, 2023 •

edited

Loading

		identifiersType.complementaryCertificationBadgeId,
		Joi.string().valid(juryOptions.REJECTED).valid(juryOptions.UNSET),

[TECH] Refacto des badges des certifications complémentaires (PIX-9862) #7447

[TECH] Refacto des badges des certifications complémentaires (PIX-9862) #7447

Conversation

aceol commented Nov 13, 2023 • edited Loading

🦄 Problème

🤖 Proposition

🌈 Remarques

💯 Pour tester

pix-bot-github commented Nov 13, 2023

P-Jeremy Nov 23, 2023

Choose a reason for hiding this comment

aceol Nov 23, 2023

Choose a reason for hiding this comment

P-Jeremy Nov 23, 2023 • edited Loading

Choose a reason for hiding this comment

aceol commented Nov 23, 2023

aceol commented Nov 24, 2023

AndreiaPena commented Nov 27, 2023

AndreiaPena Nov 27, 2023

Choose a reason for hiding this comment

aceol Nov 29, 2023

Choose a reason for hiding this comment

AndreiaPena Nov 27, 2023

Choose a reason for hiding this comment

AndreiaPena Nov 27, 2023

Choose a reason for hiding this comment

AndreiaPena Nov 27, 2023 • edited Loading

Choose a reason for hiding this comment

aceol Nov 28, 2023

Choose a reason for hiding this comment

AndreiaPena commented Nov 27, 2023 • edited Loading

aceol commented Nov 13, 2023 •

edited

Loading

P-Jeremy Nov 23, 2023 •

edited

Loading

AndreiaPena Nov 27, 2023 •

edited

Loading

AndreiaPena commented Nov 27, 2023 •

edited

Loading