#6159 - Feature: allow FranceConnect to be configured in a procedure #6272
Conversation
| fci.associate_user! | ||
|
|
||
| if fci.user && !fci.user.can_france_connect? | ||
| unless fci.user&.can_france_connect? |
There was a problem hiding this comment.
Nous sommes un peu allergiques aux unless dans l'équipe. Je suis surpris qu’il n’y ait pas de règle Rubocop pour ça.
There was a problem hiding this comment.
Il y a des règles concernant unless, mais elles ont plutôt pour objectif de le favoriser (1) en cas de condition négative, ou de lui préférer if (2) en présence d'une clause else.
C'est vraiment la norme dans la communauté Ruby d'utiliser ce mot clé du langage pour exprimer une condition de la manière la plus proche possible d'un langage naturel. Ça clarifie aussi très souvent l'intention que l'on met derrière une condition. Dans le cas ci-dessus, le ! se fait très discret et peut facilement échapper à l'œil d'un développeur ou d'un relecteur, faussant ainsi la compréhension de l'intention de l'auteur de cette ligne de code.
Ceci étant dit, je me plierai volontiers aux conventions adoptées par l'équipe ;)
|
Bonjour et merci pour le taff. Je sais qu'actuellement on a du pain sur une autre planche, mais je mets mes remarques pour plus tard. Peux-tu découper cette pr en plus petit et faire des plus petits commits (https://github.com/betagouv/demarches-simplifiees.fr/wiki/Pratiques-de-dev) Merci |
akarzim
force-pushed
the
6159_feature_france-connect
branch
10 times, most recently
from
f81d09f to
ab9a02a
Compare
|
@LeSim je fais face à un dilemme : GitGuardian râle parce que des secrets de test (pas du tout secrets) sont divulgués dans le fichier Merci d'avance ! |
akarzim
force-pushed
the
6159_feature_france-connect
branch
2 times, most recently
from
62ac122 to
ebce6e5
Compare
|
Oups, je me rends compte qu'on a laissé une question en suspens ici. Les secrets dans config.yml dont se plaint Gitgardian, c'est des secrets bidons utilisés dans l'environnement de test. Donc pas de souci. On n'a pas encore trouvé moyen de dire à gitguardian d'ignorer ces lignes (mais on peut le bypasser ponctuellement, et normalement c'est que quand il y a un diff qu'il se déclenche). Si vous trouvez une manière de définitivement couper le sifflet à gitguardian concernant ces lignes, cool - et sinon on devra ignorer le warning au cas par cas. |
Ce que je vois là comme ça, ce serait de chiffrer les secrets à l'aide de
|
akarzim
force-pushed
the
6159_feature_france-connect
branch
from
ebce6e5 to
547944c
Compare
|
Ah si, je l'ai : en fait il faut fusionner les commits ec51ce6 et 9d5d6d9 – sinon Gitguardian voit apparaitre le secret une fois (et il râle), et ensuite il voit apparaitre le secret à nouveau dans le diff du second commit (et il râle à nouveau). Mais si vous fusionnez les commits tout devrait être bon. |
|
Excusez moi d'intervenir comme un cheveu sur la soupe, mais sur le fond de cette pr, il me semble qu'on avait convergé avec @mfaure pour ne pas l'intégrer dans notre code. Un problème étant, si on fait des jetons par démarche, et qu'un utilisateur cherche a se connecter avec FC par la page https://www.demarches-simplifiees.fr/users/sign_in quel jeton on prend ? Avez-vous résolu ce souci ? |
|
@LeSim oui on peut fermer. Nous rouvrirons quand les planètes seront mieux alignées. En parallèle, on note quelque part que le code existe bel et bien ? |
|
PR fermée Le code reste disponible sur le dépôt de l'ADULLACT. |
Fixed #6159 "ETQ administrateur, je souhaite utiliser un jeton FranceConnect pour chaque démarche"
Travail en cours.
Quand @akarzim de @synbioz aura finalisé cette PR et que l'ADULLACT aura fait la recette fonctionnelle,
nous compléterons la description de cette PR et retirerons le mode "Draft".