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] CampaignLearningContent hérite de LearningContent (PIX-16195) #11206

Merged

Conversation

alicegoarnisson
Copy link
Contributor

@alicegoarnisson alicegoarnisson commented Jan 23, 2025

🥞 Problème

Beaucoup d'attributs communs entre CampaignLearningContent et LearningContent

🥓 Proposition

On crée une relation d'héritage entre les deux classes. CampaignLearningContent hérite de Learning Content (💸 ).

🧃 Remarques

😋 Pour tester

  • Faire un export de résultats sur les campagnes :

  • vérifier en particulier qu'aucune colonne liée aux domaines et aux compétences ne manque par rapport à l'onglet Analyse -> Résultat par compétence

  • Vérifier que les domaines et les compétences soient bien dans l'ordre (on fait bientôt un ticket pour réparer l'ordre des acquis).

  • Aller sur la page Analyse d'une campagne et vérifier que tout est correct ( Assessment / Collect de profile )

  • Vérifier le fonctionnement de pickCertificationChallengesForPixPlus @1024pix/team-certification

@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 :

@alicegoarnisson alicegoarnisson marked this pull request as ready for review January 23, 2025 14:54
@alicegoarnisson alicegoarnisson requested a review from a team as a code owner January 23, 2025 14:54
@alicegoarnisson alicegoarnisson added 👀 Func Review Needed Need PO validation for this functionally 👀 Tech Review Needed labels Jan 23, 2025
@xav-car xav-car force-pushed the pix-16195/campaign-learning-content-refacto branch from bc61718 to c8d9dbc Compare January 24, 2025 09:35
@xav-car xav-car requested a review from a team January 24, 2025 13:34
@xav-car xav-car force-pushed the pix-16195/campaign-learning-content-refacto branch from c8d9dbc to d044398 Compare January 24, 2025 13:44
findArea(areaId) {
return this._learningContent.findArea(areaId);
get competences() {
return super.competences.sort((a, b) => a.index.localeCompare(b.index));
Copy link
Member

Choose a reason for hiding this comment

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

Question: Pourquoi ne pas avoir mis en place ce tri dans l'objet parent LearningContent ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Je ne sais pas si LearningContent est utilisé ailleurs, les autres scopes ne veulent peut-être pas implémenter ce tri

Copy link
Contributor

Choose a reason for hiding this comment

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

Notre besoin était lié au context des export de résultats d'une campagne ou l'on avait auparavant un tri définit. Maintenant qu'il ne l'est plus. nous avons préféré le mettre dans notre model plutôt qu'au global.

N'ayant pas eu de retour sur ce besoin de tri dans d'autre context. Nous avons préféré rester "isolé"


findSkill(skillId) {
return this._learningContent.findSkill(skillId);
return super.areas.sort((a, b) => a.code.localeCompare(b.code));
Copy link
Member

Choose a reason for hiding this comment

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

Question: Pourquoi ne pas avoir mis en place ce tri dans l'objet parent LearningContent ?

@alicegoarnisson alicegoarnisson added Func Review OK PO validated functionally the PR 🚀 Ready to Merge and removed 👀 Func Review Needed Need PO validation for this functionally 🚀 Ready to Merge labels Jan 29, 2025
@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-16195/campaign-learning-content-refacto branch from fc20a35 to 5fa42d0 Compare January 29, 2025 09:18
@pix-service-auto-merge pix-service-auto-merge merged commit 119cddc into dev Jan 29, 2025
8 of 10 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-16195/campaign-learning-content-refacto branch January 29, 2025 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants