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

[CW-498] Implémenter le dashboard du panneau d'administration #206

Merged
merged 8 commits into from
Oct 24, 2024

Conversation

notoraptor
Copy link
Contributor

Hello @soline-b ! Voici une PR pour le panneau d'administration !

Quelques captures d'écran:

Screenshot 2024-10-21 at 08-22-55 Clockwork -

Screenshot 2024-10-21 at 08-23-11 Clockwork -

Screenshot 2024-10-21 at 08-23-28 Clockwork -

Screenshot 2024-10-21 at 08-23-56 Clockwork -

(Après les modifications):

Screenshot 2024-10-21 at 08-26-51 Clockwork -

@gyom
Copy link
Collaborator

gyom commented Oct 21, 2024

Merci d'inclure les saisies d'écran dans ta PR comme ça. Ça facilite réellement le feedback.

J'aurais un commentaire purement esthétique: les boutons "manage users" et "save" sont tellement gros qu'on les confond presque pour des en-têtes de sections. Pourrais-tu leur donner une taille comparable à celle que tu donnes aux boutons "edit"?

D'ailleurs, je pense que "save" devrait être accompagné d'un autre bouton pour revenir à la page antérieure. Oui, on pourrait dire que quelqu'un pourrait toujours appuyer sur "back" dans son fureteur, mais avoir un bouton "cancel" ou ça donne une interface un peu plus complète. Ça rassure aussi l'utilisateur que l'interface reconnaît qu'il ne voulait pas changer les valeurs. Bon, ce n'est pas nécessaire, mais je pense que tant qu'à être là, autant mettre ce bouton aussi.

Un autre point pertinent dont on n'avait pas parlé, c'est qu'on pourrait vouloir afficher le statut du compte de la personne. Je ne me souviens plus quel est le nom du champ spécifique ("status" ?), mais si on imagine un scénario dans lequel un utilisateur dit qu'il n'a pas accès, et que le it-support vont voir dans le panneau d'admin, il serait utile qu'ils puissent voir le "status" du même coup en affichant la liste d'utilisateurs. Pas besoin de pouvoir le modifier, mais juste de l'afficher comme autre colonne.

Sur ce sujet, penses-tu, @soline-b, qu'il y aurait d'autres colonnes à ajouter comme ça?

Reduce size for user edition button `save`
Add back button `cancel` to user edition form
@notoraptor
Copy link
Contributor Author

@gyom PR à jour avec tes suggestions !

Captures d´écran

Page d'accueil de l'administration:

Screenshot 2024-10-21 at 10-25-40 Clockwork -

Page d'édition d'un user, avec le bouton "cancel" rajouté pour revenir à la page des users.

PS: En haut de chaque page, il y a aussi des liens cliquables pour revenir à la page précédente (ici, dans Administration panel / Users, on peut cliquer sur Users pour revenir à la page des users, et sur Administration panel pour revenir à la page d'accueil de l'adminstration). Mais ce n'est effectivement pas intuitif !

Screenshot 2024-10-21 at 10-26-12 Clockwork -

Page des users (inchangée):

Screenshot 2024-10-21 at 10-25-52 Clockwork -

@gyom
Copy link
Collaborator

gyom commented Oct 21, 2024

C'est une bonne idée d'avoir fait que les titres en haut sont des hyperliens.

Pourrais-tu mettre "manage users" la moitié de la largeur actuelle (ou même, moins que ça)? Regarde les boutons dans Github et constate combien de vide il y a autour du texte:

image

Pour le "save" et "cancel", on dirait que tu les as placés dans la table avec les attributs et les valeurs. Sémantiquement parlant, ça jure un peu avec le reste du contenu de la table. Ça ne devrait pas être une rangée de la table (comme le suggère le fond gris actuellement), mais plutôt une chose qui vient après la table.

@soline-b
Copy link
Collaborator

Salut,

Pardon pour le retard :)

  • Concernant le statut, je pense qu'il pourra s'agir d'une PR ultérieure, celle-ci incluant déjà la modification des noms d'utilisateurs et une première version du dashboard en lui-même (techniquement, deuxième version, mais elle ajoute son premier bouton ! 🎂 )

  • Concernant le visuel, je pense que je peux te laisser les rênes, car il s'agit du panneau d'administration et que le design m'importe pour l'instant peu. Si tu penses cependant qu'il s'agit d'une chose importante, je ne suis pas fermée à accorder plus de réflexion de mon côté à l'avenir :)

@notoraptor
Copy link
Contributor Author

@gyom Comme ceci ?

Screenshot 2024-10-21 at 12-07-21 Clockwork -

Screenshot 2024-10-21 at 12-07-36 Clockwork -

@gyom
Copy link
Collaborator

gyom commented Oct 21, 2024

Merci @notoraptor! Pour moi ça convient bien. Je suis aussi d'accord avec @soline-b comme quoi c'est pas le temps de passer trop de temps sur le look pour tout changer alors que d'autres éléments vont s'ajouter bientôt. Mais il y avait quelques choses qui me faisaient juste trop grincer des dents. :)

@soline-b
Copy link
Collaborator

Voilà pour mes remarques :

Utilisation dynamique des noms d'utilisateurs associés aux clusters

Ici, les champs mila_cluster_username et cc_account_username sont hardcodés : l'idée serait de se baser sur le champ account_field renseigné pour chaque cluster dans le fichier de configuration. Pour cela, il est possible de s'aider de la fonction get_account_fields dans le fichier clockwork/clockwork_web/core/clusters_helper.py.

Traduction des nouveaux messages de l'UI

Il serait bien d'ajouter au fichier clockwork/clockwork_web/static/locales/fr/LC_MESSAGES/messages.po qui n'existent pas déjà. L'idée serait de checker les traductions de :

  • Missing argument username
  • Cannot find username: {mila_email_username}
  • Administration panel
  • Manage users
  • Users (je pense que celle-ci existe)
  • Cancel
  • Save
  • edit

N'hésite pas à me poser des questions à ce sujet, sinon la doc pour faire ça est ici : https://github.com/mila-iqia/clockwork/blob/master/docs/clockwork_dev_guide/internationalization.md
(NB : la dernière étape (ie création des fichiers .mo) devrait être faite automatiquement, donc pas besoin de s'en soucier)

Cas d'un utilisateur qui n'a de base pas accès à un cluster

Le champ "None" étant prérempli dans le cas où l'utilisateur n'a pas de compte associé au cluster, celui-ci est modifié pour "None" quand non updaté. Cela peut être testé avec l'utilisateur [email protected].

Si tu as des questions, n'hésite pas :)
Merci beaucoup !

@notoraptor
Copy link
Contributor Author

@soline-b PR mise à jour !

  • Les champs des noms d'utilisateurs sont gérés dynamiquement
  • Des traductions ont été mises en place.
    • J'ai ajouté des traductions pour les noms de champs spécifiques mila_cluster_username et cc_account_username en anglais et en français.
  • La valeur None est correctement prise en charge: elle est affichée comme une chaîne vide.
    • Dans le formulaire, si un champ est vide, il est enregistré comme None.
Captures

Capture d’écran de 2024-10-21 16-13-59

Capture d’écran de 2024-10-21 16-14-04

Capture d’écran de 2024-10-21 16-14-12

Capture d’écran de 2024-10-21 16-14-29

Capture d’écran de 2024-10-21 16-14-33

Capture d’écran de 2024-10-21 16-14-42

Capture d’écran de 2024-10-21 16-14-46

Capture d’écran de 2024-10-21 16-14-55

Capture d’écran de 2024-10-21 16-15-11

Capture d’écran de 2024-10-21 16-15-14

Capture d’écran de 2024-10-21 16-15-18

Capture d’écran de 2024-10-21 16-15-26

Capture d’écran de 2024-10-21 16-15-41

@soline-b soline-b merged commit f61e2be into mila-iqia:master Oct 24, 2024
2 checks passed
@notoraptor notoraptor deleted the cw-498-admin branch November 19, 2024 16:05
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