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.

Sign up for GitHub

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

Jump to bottom

[FEATURE] Créer une alerte en cas de non détection de l'extension Companion lors de la certification sur Pix App (PIX-14727). #10303

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

[FEATURE] Créer une alerte en cas de non détection de l'extension Companion lors de la certification sur Pix App (PIX-14727). #10303

wants to merge 5 commits into from

Conversation

AndreiaPena
Copy link
Member

@AndreiaPena AndreiaPena commented Oct 11, 2024 •
edited
Loading

🦄 Problème

En certification, lorsque Pix App ne détecte plus l'extension, la page Blocker s'affiche mais ça n'entraîne rien de plus.
On souhaiterait pouvoir bloquer l'utilisateur sur la question en cours tant que le surveillant n'a pas confirmé l'activation de l'extension. Pour cela nous avons besoin d'enregistrer une alerte en BDD.

🤖 Proposition

Créer une alerte spécifique à Companion.

🌈 Remarques

Création d'une nouvelle table 'certfication-companion-live-alerts'

💯 Pour tester

  • Activer l'extension Companion
  • Se connecter sur App avec certif-success@example.net
  • Remplir le formulaire d'entrée en session de certif (7402 firstname0... 01/01/2000)
  • Entrer en session
  • Désactiver l'extension
  • Vérifier en BDD l'enregistrement d'une alerte en cours ('ongoing')

@AndreiaPena AndreiaPena requested review from a team as code owners October 11, 2024 12:29
@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 :

@AndreiaPena AndreiaPena force-pushed the pix-14727-companion-live-alert branch 2 times, most recently from 10f6a86 to 62e7c60 Compare October 11, 2024 15:21
@nlepage nlepage force-pushed the pix-14727-companion-live-alert branch from 62e7c60 to 3656f36 Compare October 11, 2024 15:25
@nlepage nlepage removed the request for review from a team October 14, 2024 07:33
@P-Jeremy
Copy link
Contributor

Test func ✅

Après désactivation de l'extension

image

image

Steph0
Steph0 reviewed Oct 14, 2024
View reviewed changes
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 fais pas mal de retour sur la definition des model, et du coup ca risque d'impacter la partie Front qu'il faudrait que je relise.

api/db/migrations/20241011074058_create-certification-companion-live-alert-table.js Outdated Show resolved Hide resolved
api/db/migrations/20241011074058_create-certification-companion-live-alert-table.js Show resolved Hide resolved
api/db/migrations/20241011074058_create-certification-companion-live-alert-table.js Show resolved Hide resolved
api/src/certification/session-management/domain/models/CertificationCompanionLiveAlert.js Outdated Show resolved Hide resolved
api/src/certification/session-management/domain/usecases/create-companion-alert.js Outdated Show resolved Hide resolved
api/src/certification/session-management/domain/usecases/create-companion-alert.js Outdated Show resolved Hide resolved
...n/session-management/infrastructure/repositories/certification-companion-alert-repository.js Outdated Show resolved Hide resolved
...s/certification/session-management/acceptance/application/companion-live-alert-route_test.js Outdated Show resolved Hide resolved
mon-pix/app/adapters/assessment.js Show resolved Hide resolved
@nlepage nlepage force-pushed the pix-14727-companion-live-alert branch from 9fb9b04 to 5efe444 Compare October 14, 2024 10:18
@nlepage nlepage force-pushed the pix-14727-companion-live-alert branch 2 times, most recently from 3a484e9 to 9dd057a Compare October 14, 2024 14:18
Steph0
Steph0 approved these changes Oct 14, 2024
View reviewed changes
Copy link
Contributor

@Steph0 Steph0 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Je mets le Approve en anticipation du deplacement vers evaluation (vu que on est en live sur le meet) 😊 !

Test func avec un warning

Cela fonctionne mais warning sur l'enum qui est pas toujorus enregistre avec la meme casse
image

api/db/migrations/20241011074058_create-certification-companion-live-alert-table.js Outdated
await knex.schema.createTable(TABLE_NAME, function (table) {
table.increments().primary().notNullable();
table.integer('assessmentId').notNullable().unsigned().references('assessments.id').index();
table.string('status').notNullable().defaultTo('ONGOING');
Copy link
Contributor

Choose a reason for hiding this comment

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

Merci pour le commentaire sur la table, DATA est aussi interesse sur la definition fonctionnelle aussi des colonnes. Typiquement que veut dire cleared et ongoing. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

j'ai tenté ! à confirmer :)

api/src/certification/session-management/domain/usecases/create-companion-alert.js Outdated
async function ({ assessmentId, certificationCompanionAlertRepository }) {
const companionAlert = new CertificationCompanionLiveAlert({
assessmentId,
status: 'ONGOING',
Copy link
Contributor

Choose a reason for hiding this comment

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

CertificationCompanionLiveAlertStatus.ONGOING (autant utiliser l'enumeration cree pour l'occaz ce qui evite les magic strings).

Autre version que j'aime bien, mais ptet overkill, une factory (j'ai pas cherche d'article en node) : CertificationCompanionLiveAlert.createOngoing()

...s/certification/session-management/acceptance/application/companion-live-alert-route_test.js Outdated Show resolved Hide resolved
@AndreiaPena AndreiaPena force-pushed the pix-14727-companion-live-alert branch 2 times, most recently from 13e9288 to 75d4edf Compare October 14, 2024 16:51
nlepage and others added 2 commits October 14, 2024 19:05
@nlepage @AndreiaPena
feat(mon-pix): companion blocker accepts onBlock callback
81d1358 
Co-authored-by: Andreia Pena Ferreira <andreia.pena@pix.fr>
@nlepage @AndreiaPena
✨ mon-pix: call companion-alert endpoint when companion is not detected
bd63f5b 
Co-authored-by: Andreia Pena Ferreira <andreia.pena@pix.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nlepage nlepage nlepage left review comments

@Steph0 Steph0 Steph0 approved these changes

@mcampourcy mcampourcy mcampourcy approved these changes

@P-Jeremy P-Jeremy P-Jeremy approved these changes

Assignees

@nlepage nlepage

@AndreiaPena AndreiaPena

Labels
👀 Func Review Needed 👀 Tech Review Needed team-certif team-companion
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@AndreiaPena @pix-bot-github @P-Jeremy @Steph0 @mcampourcy @nlepage