-
Notifications
You must be signed in to change notification settings - Fork 52
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] Documenter les colonnes de notre base de données #9457
base: dev
Are you sure you want to change the base?
Conversation
…te knex migration" This reverts commit 32ff8c7.
Une fois les applications déployées, elles seront accessibles via les liens suivants :
Les variables d'environnement seront accessibles via les liens suivants : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Est-ce que écrire notre documentation technique de BDD en français est une bonne chose (vu que l'on code en anglais) ? J'y vois un pas qui ne va pas dans le sens de faire rayonner pix à une échelle + internationale.
Autre point : cette PR sort des informations qui étaient jusqu'à présent non visibles à l'extérieur de Pix. Peut-être faire valider avec le Produit de faire sortir cela ?
Excellente idée par contre de documenter notre BDD :)
'COMMENT ON COLUMN "assessments"."userId" IS \'Identifiant technique de l’utilisateur passant le test, obligatoire\'', | ||
); | ||
await knex.raw( | ||
'COMMENT ON COLUMN "assessments"."type" IS \'Options:\n\n- PREVIEW\n\n\n- DEMO\n\n\n- PLACEMENT (vieux type)\n\n\n- CAMPAIGN\n\n\n- COMPETENCE_EVALUATION\n\n\n- CERTIFICATION\n\n\n\nobligatoire\'', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Est-ce que les \n\n\n\n\n
ont un intérêt côté PG ? Ou serait-il intéressant de les enlever ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ils ont été forcé justement si tu regardes le script qui fait le parsing, car PG les comprends bien. Tu peux essayer en faisant \d+ assessments
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Effectivement, ils sont bien interprétés. Mais je ne comprends pas trop l'intérêt d'en avoir plusieurs ceci dit :)
'COMMENT ON COLUMN "authentication-methods"."identityProvider" IS \'6 options:\n\n- PIX\n\n\n- GAR: SSO externe\n\n\n- POLE_EMPLOI: SSO externe\n\n\n- CNAV: SSO externe\n\n\n- PAYSDELALOIRE : SSO externe\n\n\n- FWB : SSO externe\'', | ||
); | ||
await knex.raw( | ||
'COMMENT ON COLUMN "authentication-methods"."authenticationComplement" IS \'- Si “PIX”: \n\n- “password": mot de passe chiffrée\n\n\n- “shouldChangePassword": true/false - si true, alors le mot de passe stockée a été generée par un membre d\'\'un établissement scolaire, depuis Pix Orga, et ce mot de passe est temporaire. Lors de la prochaine connexion de cet élève, il devra saisir un nouveau mot de passe.\n\n\n\n\n- Si “GAR”: \n\n- “lastName"\n\n\n- “firstName"\n\n\n\n\n- Si “POLE_EMPLOI”:\n\n- “accessToken": lien créé entre compte PE et compte Pix, créé à partir de l’identifiant PE\n\n\n- “expiredDate":\n\n\n- “refreshToken":\n\n\n\n\n- Si “CNAV”:\n\n- le champ est VIDE\n\n\n\n\n- Si “FWB”:\n\n- "employeeNumber"\n\n\n- "population”\'', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tout comme les \n
serait-ce intéressant de clean les caractères HTML ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 pour les caractères HTML qu'on retrouve tel quels quand on récupère la description de la table.
Pour le code en français, j'estime que le boulot a déjà été fait en français donc autant en profiter. Et c'est très facile pour des étrangers d'utiliser un traducteur. Quid des PRs, et des descriptions des endpoints dans ce cas … :)
Les descriptions sont déjà implicites dans notre code (nommage des colonnes et Modèles du domaine). Ici, on les rends juste plus explicites et accessibles. |
🦄 Problème
Actuellement, la documentation est faite manuellement dans Confluence. C'est loin du code, ce qui fait personne y pense et c'est par conséquent pas à jour. De plus, notre data catalog n'en bénéficie pas ainsi que les personnes réutilisant la base de code.
🤖 Proposition
Ajouter les commentaires directement sur les colonnes.
La migration a été généré grâce au script présent dans le premier commit et supprimé dans le troisième car c'est un one-shot. Il est ajouté et supprimé uniquement pour le retrouver dans l'historique. Si vous êtes curieux ou curieuse de voir de l'AST, n'hésitez pas.
🌈 Remarques
Pour info les commentaires sont disponibles sur pgcli par exemple en faisant
\d+ <nom_de_table>
A venir (prochain point tech par exemple): il faut créer une règle Eslint pour forcer les développeurs à commenter les colonnes à chaque nouvelle migration
💯 Pour tester
Constater que les commentaires sont biens présents avec
\d+ <nom_de_table>
Tester la migration down en utilisant la CLI avec
run npm run db:rollback:latest
run npm db:migrate