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

Enhance legacy non migrated token display #1394

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

Un3x
Copy link
Contributor

@Un3x Un3x commented Dec 15, 2023

No description provided.

@Un3x Un3x force-pushed the enhance-legacy-non-migrated-token-display branch from 0bf37c7 to 0dc775b Compare December 15, 2023 13:45
@Un3x Un3x requested a review from skelz0r December 15, 2023 13:45
Copy link
Contributor

@DorineLam DorineLam left a comment

Choose a reason for hiding this comment

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

Merci @Un3x, on est d'accord que quand on clique ça va ouvrir ça :
image

Copy link
Member

@skelz0r skelz0r left a comment

Choose a reason for hiding this comment

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

C'est quand même compliqué d'un point de vue métier.
Y'a des tests qui traînent pour tester toutes les branches ?

D'ailleurs j'ai l'impression qu'il est possible de ne pas avoir de show du jeton (quasiment sûr btw avec le compact qui traîne). Pourquoi ?

Et pourquoi on n'affiche pas le show quelque soit l'état du jeton ?

Vu que globalement c'est quasiment que du refacto (modulo le no_action) j'accepte la PR mais je trouve ça très compliqué en voyant cette PR, imo y'a potentiellement de la simplification à faire d'un point de vue métier.

@Un3x
Copy link
Contributor Author

Un3x commented Dec 15, 2023

L'idée de cette colone c'est de pouvoir voir en un clin d'oeil quelles sont les actions en attentes concernant chacune des habilitations et le token. Les actions possibles sont le show et le prolong, mais les labels diffèrent selon la situation.

Le cas qui est ajouté dans cette PR (le remplacement du token legacy), disparaîtra en même temps que les tokens legacy.

@Un3x
Copy link
Contributor Author

Un3x commented Dec 15, 2023

Concernant les tests, il y a des tests pour l'affichage du show quand il y a un blacklisting, du prolong et du no action.

Il n'y a pas de test dans le cas du show pour une première utilisation, ni pour le cas rajouté ici de changement de jetons legacy (après normalement ce n'est qu'un label qui change)

1 similar comment
@Un3x
Copy link
Contributor Author

Un3x commented Dec 15, 2023

Concernant les tests, il y a des tests pour l'affichage du show quand il y a un blacklisting, du prolong et du no action.

Il n'y a pas de test dans le cas du show pour une première utilisation, ni pour le cas rajouté ici de changement de jetons legacy (après normalement ce n'est qu'un label qui change)

@Un3x
Copy link
Contributor Author

Un3x commented Dec 15, 2023

@DorineLam oui c'est ca qu'ils vont voir. Sauf s'ils sont contact métier, auquel cas il y aura le contenu de la popup version contact métier.

@DorineLam
Copy link
Contributor

Merci Thomas, ça me semble super.
@skelz0r le fait de ne pas voir le jeton dans la colonne action attendue si il n'y a pas d'action "indispensable" c'est justement pour quel les usagers voient en un coup d'oeil les habilitations qui nécessitent une action.
C'est plus utile pour les usagers avec beaucoup d'habilitations.

@Un3x Un3x force-pushed the enhance-legacy-non-migrated-token-display branch from 0dc775b to 99a62c4 Compare December 18, 2023 08:57
@Un3x Un3x merged commit 1d25fde into develop Dec 19, 2023
5 checks passed
@Un3x Un3x deleted the enhance-legacy-non-migrated-token-display branch December 19, 2023 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants