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

[TECH] Export de la liste blanche SCO #10593

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

[TECH] Export de la liste blanche SCO #10593

wants to merge 7 commits into from

Conversation

Steph0
Copy link
Contributor

@Steph0 Steph0 commented Nov 19, 2024

🍂 Problème

Dans la suite des episodes "on en peut plus de gerer la liste blanche par variable d'environnement", et pour pouvoir importer une liste blanche, il faut pouvoir aussi l'exporter.

🌰 Proposition

  • Creer un bouton d'export
  • Compatibilite entre le fichier d'export et d'import

🎃 Remarques

  • J'ai ajoute aussi une petite securite sur l'import pour ne pas pouvoir importer une liste invalide

🪵 Pour tester

  • Sur Pix Admin, aller dans Administration > Certification > Liste blanche centres SCO
    • soit /administration/certification
  • Cliquer sur le bouton d'export
    • Pas d'erreur, et recuperation d'un fichier CSV avec un en-tete "externalId"
  • Reperer, ou creer, un centre SCO, et y mettre un externalId
    • Par exemple CERTIFICATION_SCO_MANAGING_STUDENTS_EXTERNAL_ID est un bon candidat
  • Editer le fichier CSV export, y mettre l'externalId
  • Cliquer sur le bouton d'import et importer le fichier sauvegarde
    • Pas d'erreur
  • Refaire un export, et voir que le nouveau fichier d'export contient l'externalId
  • Editer le fichier CSV export, y mettre un external id "faux", genre "PAS_MOI"
  • Tenter d'exporter un fichier avec ce mauvais id
    • Constater un message d'erreur
    • Tenter un export et voir que la liste blanche n'a pas bouge (on a refuse la liste avec des data invalides)

@Steph0 Steph0 requested a review from a team as a code owner November 19, 2024 16:07
@Steph0 Steph0 self-assigned this Nov 19, 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 :

AndreiaPena
AndreiaPena reviewed Nov 19, 2024
View reviewed changes
admin/app/components/administration/certification/sco-whitelist-configuration.gjs
const token = this.session.data.authenticated.access_token;
await this.fileSaver.save({ url, fileName, token });
} catch (error) {
this.notifications.error(this.intl.t('pages.administration.certification.sco-whitelist.export.error'));
Copy link
Member

@AndreiaPena AndreiaPena Nov 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

si jamais ma PR passe avant (mais j'y crois moyen 😅)

Suggested change
this.notifications.error(this.intl.t('pages.administration.certification.sco-whitelist.export.error'));
this.pixToast.sendErrorNotification({message:this.intl.t('pages.administration.certification.sco-whitelist.export.error')});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Est-ce que je peux le faire avant que ta PR merge ? Sinon je vais devoir laisser ainsi

Copy link
Member

Choose a reason for hiding this comment

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

Tu pourrais avant mais la notif n'apparaîtra pas tant que ma PR n'est pas mergé :)

Copy link
Member

Choose a reason for hiding this comment

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

Ma PR à été mergé, le commentaire plus haut est à corriger :)

@AndreiaPena
Copy link
Member

AndreiaPena commented Nov 19, 2024 •
edited
Loading

Question : Avec l'ajout de cette PR, l'import/export de la whitelist est terminé, non ?

Si oui il y a un bandeau Feature en cours à retirer coté front :)
Capture d’écran 2024-11-19 à 18 04 36

Sinon, pas terminé la review, je poursuis demain :)

@Steph0 Steph0 force-pushed the tech-times-certif-whitelist-SCO branch 2 times, most recently from c45966a to aeab8dd Compare November 20, 2024 08:29
@Steph0 Steph0 force-pushed the tech-times-certif-whitelist-SCO branch from 1ab82f7 to a5d0b65 Compare November 20, 2024 08:33
@Steph0
Copy link
Contributor Author

Steph0 commented Nov 20, 2024 •
edited
Loading

Question : Avec l'ajout de cette PR, l'import/export de la whitelist est terminé, non ?

Si oui il y a un bandeau Feature en cours à retirer coté front :) Capture d’écran 2024-11-19 à 18 04 36

Sinon, pas terminé la review, je poursuis demain :)

C'est fait, j'ai retire le bandeau car la partie Pix Admin est en effet finie.
La feature dans sa totalite n'est par contre pas encore terminee, il me reste a maintenant brancher l'usage de la nouvelle colone en lieu et place de la variable d'environnement (mais ce sera un prochain tech times)

@AndreiaPena
Copy link
Member

AndreiaPena commented Nov 20, 2024 •
edited
Loading

  • Quand je télécharge, j'ai l'entête "externalId" ✅

  • Puis je renseigne un externalId (EX123, l'externalId du Centre de certif Contenu ) sous l'entête, ça échoue ✅

  • => c'est un externalId PRO

  • => Je suis pas bien réveillé du coup j'ai eu du mal à comprendre pourquoi il ne passait pas à l'origine. Je me demande si on ne développerait pas le message d'erreur (à voir si cette fonctionnalité mérite qu'on développe un peu plus finement les erreurs)

  • Puis je réessaye avec CERTIFICATION_SCO_MANAGING_STUDENTS_EXTERNAL_ID ça fonctionne ✅

  • J'exporte la liste, je le retrouve bien dedans ✅

  • J'importe une nouvelle liste avec plusieurs externalIds SCO uniquement, la notif me dit que c'est bon ✅

"externalId"
"CERTIFICATION_SCO_MANAGING_STUDENTS_EXTERNAL_ID","SCOSCO","SCO_123"

  • J'exporte de nouveau la liste, je ne trouve pas mes 3 externalIds, uniquement le premier 🔴

  • Je recommence en me disant que j'ai probablement mal renseigné les ids

"externalId"
"CERTIFICATION_SCO_MANAGING_STUDENTS_EXTERNAL_ID"
"SCOSCO"
"SCO_123"
  • J'exporte de nouveau la liste et j'ai les 3 ✅

@AndreiaPena
Copy link
Member

Simulé une erreur lors de l'export, j'ai bien la notif d'erreur ✅

Capture d’écran 2024-11-20 à 10 56 35

@yaf
Copy link
Contributor

yaf commented Nov 21, 2024

Sur Pix Admin, aller dans Administration > Certification > Liste blanche centres SCO

question : quand je fais ce cheminement, je n'arrive pas à « liste blanche centres SCO » il y a une subtilité que je n'ai pas ?

image

@yaf
Copy link
Contributor

yaf commented Nov 21, 2024

OK, c'était dans admin ....

@P-Jeremy
Copy link
Contributor

P-Jeremy commented Nov 21, 2024 •
edited
Loading

Test fun cas passant OK ✅

Test func mauvais id 🤔
J'ai bien la notif (trop lent pour l'avoir dans ma capture)

⚠️ Par contre j'ai une erreur 500 (testé avec un external id non sco)
image
image
Capture d’écran 2024-11-21 à 11 22 57

AndreiaPena
AndreiaPena reviewed Nov 21, 2024
View reviewed changes
api/src/certification/configuration/domain/usecases/import-sco-whitelist.js
const numberOfUpdatedLines = await centerRepository.addToWhitelistByExternalIds({ externalIds });

if (externalIds.length !== numberOfUpdatedLines) {
throw new RangeError('Some externalIds are not valid, please verify whitelist');
Copy link
Member

Choose a reason for hiding this comment

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

Cette erreur n'est pas catché ce qui donne la 500

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndreiaPena AndreiaPena AndreiaPena approved these changes

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

Assignees

@Steph0 Steph0

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

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