Development #69365
custom_user : migrer les données de user.attributes.phone au format E.164
0%
Description
Dans le cadre de #49212 :
Dans #65173 on introduit un nouveau champ user.phone et on met en place la double écriture implicite user.phone <-> user.attributes.phone.
Dans #69221 on s’assure que le téléphone mentionné à l’authentification atterrit au format E.164, il en sera de même lors de l’enregistrement du compte de l’usager.
Pour que ça n’explose pas en plein vol il faut que tous les user.attributes.phone partout soient au format E.164. Une migration est nécessaire.
Fichiers
Demandes liées
Révisions associées
widgets: use libphonenumbers' local formatting (#69365)
utils/misc: add parse_phone_number_utility (#69365)
attribute_kinds: use custom PhoneField for phone_number type (#69365)
csv_import adapt user csv logic to new phone_number kind (#69365)
Historique
Mis à jour par Paul Marillonnet il y a plus d'un an
- Lié à Development #49212: Création de compte avec un numéro de téléphone mobile ajouté
Mis à jour par Paul Marillonnet il y a plus d'un an
- Statut changé de Nouveau à En cours
- Assigné à mis à Paul Marillonnet
Mis à jour par Paul Marillonnet il y a plus d'un an
- Fichier 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch ajouté
- Fichier 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch ajouté
- Fichier 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch ajouté
- Statut changé de En cours à Solution proposée
- Patch proposed changé de Non à Oui
Mis à jour par A. Berriot il y a plus d'un an
Dans ton 0001, je vois un ['33', '01a01'], qui est passé d'un test de clean négatif à positif, c'est normal ? (@tests/test_fields.py)
À part ça, ça me parait top!
Mis à jour par Paul Marillonnet il y a plus d'un an
Agate Berriot a écrit :
Dans ton 0001, je vois un
['33', '01a01'],qui est passé d'un test de clean négatif à positif, c'est normal ? (@tests/test_fields.py)À part ça, ça me parait top!
Yes, bien vu, il manque la même validation que pour le champ DRF déjà en place pour ce type d’attribut phone_number, je regarde.
Mis à jour par Paul Marillonnet il y a plus d'un an
Paul Marillonnet a écrit :
Yes, bien vu, il manque la même validation que pour le champ DRF déjà en place pour ce type d’attribut
phone_number, je regarde.
Yes, dans #69221, dans la méthode compress du champ multi-valeurs il manquait l’appel à authentic2.attribute_kinds.clean_number(). C’est fait et ça permet ici de lever une erreur de validation sur ce numéro qui n’en est pas un. C’est visible dans la branche.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
Je me rappelle maintenant pourquoi ces histoires de E.164 me perturbent, la dernière fois que je l'ai proposé c'était pour l'intégration de la recherche sur le numéro de téléphone, principalement pour les accueils guichet et via plateforme téléphonique, il faudrait s'assurer avant de migrer quoi que ce soit que la recherche via l'API (/api/users/q=..., pour le moteur de recherche coté combo et l'accueil) et via le BO (/manager/users/) vont retrouver les numéros de téléphone au format E.164 quand on rentre un format court local. Par ailleurs il faudrait aussi qu'en sortie (affichage) le format court adapté soit utilisé et pour cela il faudrait configurer un country code par défaut au niveau d'authentic. En sortie ça devrait donner :
if phone_number.country_code in settings.PHONENUMBER_LOCAL_COUNTRY_CODES: (par défaut on met la liste des préfixes pour la France et les DOM-TOM) format = phonenumbers.PhoneNumberFormat.NATIONAL) else: format = phonenumbers.PhoneNumberFormat.INTERNATIONAL) return phonenumbers.format_number(x, format)
Ne pas casser les intégrations avec PABX est la priorité ici.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
- Statut changé de Solution proposée à Information nécessaire
Mis à jour par Benjamin Dauvergne il y a plus d'un an
Au passage je relis un peu :
- je vois un changement pour le form_field, et concernant le champ DRF ? On ne normalise pas vers E.164 ?
- dans le code CSV, si le numéro ne parse pas, la valeur est perdue et aucune errer n'est remontée, je donnerai deux choix: péter une erreur, ou stocker ça de toute façon dans phone_1 et sans préfixe, en considérant ça comme un numéro local et tant pis (avec un test)
- même souci dans la migration, il faudrait conserver la valeur existante même si elle est pourrie (sans le +33)
Quand ces numéros pourris arrivent sur la page d'édition du compte il faudrait que ça s'affiche, mais qu'on ne puisse pas soumettre (si la valeur ne parse pas, phone_0 = vide et phone_1 = la valeur bidon).
Mis à jour par Paul Marillonnet il y a plus d'un an
- Statut changé de Information nécessaire à En cours
Benjamin Dauvergne a écrit :
Je me rappelle maintenant pourquoi ces histoires de E.164 me perturbent, la dernière fois que je l'ai proposé c'était pour l'intégration de la recherche sur le numéro de téléphone, principalement pour les accueils guichet et via plateforme téléphonique, il faudrait s'assurer avant de migrer quoi que ce soit que la recherche via l'API (/api/users/q=..., pour le moteur de recherche coté combo et l'accueil) et via le BO (/manager/users/) vont retrouver les numéros de téléphone au format E.164 quand on rentre un format court local. Par ailleurs il faudrait aussi qu'en sortie (affichage) le format court adapté soit utilisé et pour cela il faudrait configurer un country code par défaut au niveau d'authentic. En sortie ça devrait donner :
[...]Ne pas casser les intégrations avec PABX est la priorité ici.
Ok bien vu, j’avais complètement zappé cette recherche qui doit pouvoir fonctionner avec des numéros locaux. J’ai créé #69906 et #69907 respectivement pour l’api et la recherche dans le BO.
–––––
Au passage je relis un peu :
- je vois un changement pour le form_field, et concernant le champ DRF ? On ne normalise pas vers E.164 ?
Yes, on le fait aussi, c’est le patche posé dans #69430.
- dans le code CSV, si le numéro ne parse pas, la valeur est perdue et aucune errer n'est remontée, je donnerai deux choix: péter une erreur, ou stocker ça de toute façon dans phone_1 et sans préfixe, en considérant ça comme un numéro local et tant pis (avec un test)
Ok, j’avais zappé l’absence d’erreur remontée, c’est un oubli de ma part, je regarde.
- même souci dans la migration, il faudrait conserver la valeur existante même si elle est pourrie (sans le +33)
Ok, faisons comme ça.
Quand ces numéros pourris arrivent sur la page d'édition du compte il faudrait que ça s'affiche, mais qu'on ne puisse pas soumettre (si la valeur ne parse pas, phone_0 = vide et phone_1 = la valeur bidon).
Ça marche, fair enough.
Mis à jour par Paul Marillonnet il y a plus d'un an
- Fichier 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch ajouté
- Fichier 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch ajouté
- Fichier 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch ajouté
- Statut changé de En cours à Solution proposée
- dans le code CSV, si le numéro ne parse pas, la valeur est perdue et aucune errer n'est remontée, je donnerai deux choix: péter une erreur, ou stocker ça de toute façon dans phone_1 et sans préfixe, en considérant ça comme un numéro local et tant pis (avec un test)
Ok, j’avais zappé l’absence d’erreur remontée, c’est un oubli de ma part, je regarde.
Corrigé ici, ça remonte une erreur quand la valeur n’est pas valide.
- même souci dans la migration, il faudrait conserver la valeur existante même si elle est pourrie (sans le +33)
Ok, faisons comme ça.
Corrigé ici.
Quand ces numéros pourris arrivent sur la page d'édition du compte il faudrait que ça s'affiche, mais qu'on ne puisse pas soumettre (si la valeur ne parse pas, phone_0 = vide et phone_1 = la valeur bidon).
J’ai regardé un peu plusieurs façons de faire, la façon la plus propre c’est de gérer ça directement dans la méthode decompress du widget, a priori simplement un
diff --git a/src/authentic2/forms/widgets.py b/src/authentic2/forms/widgets.py
index 06e5d4261..8abc857a7 100644
--- a/src/authentic2/forms/widgets.py
+++ b/src/authentic2/forms/widgets.py
@@ -414,4 +414,4 @@ class PhoneWidget(MultiWidget):
# retrieve the string representation from pn.national_number integer
raw_number = '0' * (pn.number_of_leading_zeros or 0) + str(pn.national_number)
return [code, raw_number]
- return ['', '']
+ return [settings.DEFAULT_COUNTRY_CODE, value]
que je vais m’empresser d’aller corriger directement dans #69221 accompagné d’un test.
Mis à jour par Paul Marillonnet il y a plus d'un an
- Fichier 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch ajouté
- Fichier 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch ajouté
- Fichier 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch ajouté
Paul Marillonnet a écrit :
J’ai regardé un peu plusieurs façons de faire, la façon la plus propre c’est de gérer ça directement dans la méthode
decompressdu widget, a priori simplement un
[...]
que je vais m’empresser d’aller corriger directement dans #69221 accompagné d’un test.
J’ai fait la modif directement dans le commit de #69221 qui ajoute le MultiWidget, et j’ai ajouté un test ici dans le 0001. Je remets les trois patches dans leur version à jour.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
- Statut changé de Solution proposée à Solution validée
Mis à jour par Paul Marillonnet il y a plus d'un an
- Statut changé de Solution validée à En cours
Discussions sur le salon ce matin :
je pense qu'il faut revoir le plan, garder les données qu'on a et convertir lors des échanges de données uniquement.
donc si la réunion est pas configuré avec +262 comme préfixe par défaut tant pis ça fera 02... -> +33 2... -> 02 ... de authentic vers un champ pré-rempli vers w.c.s. c'est pas pire que maintenant.
quand on aura configuré un default_country_code ça fera 02... -> +262 ... -> 02 ..
si on force un country code maintenant en modifiant toutes les données stockées et qu'on change d'avis on pourra pas revenir en arrière, même avec un default_country_code faut rien faire dans le stockage, ce qui n'empêche pas d'être plus strict pour les nouvelles données
Mis à jour par Paul Marillonnet il y a plus d'un an
Ce qui m’embête ici est que le format international globalement unique permettait de faire du numéro de téléphone une information identifiante pour l’usager.
C’était une anticipation de la situation où des comptes seraient créés sans courriel.
Si on ne migre pas les numéros existants, je n’ai aucune idée de comment on peut prévoir cette situation.
Mis à jour par Paul Marillonnet il y a plus d'un an
Benj, tu avais un plan précis en tête lorsque tu pensais la migration des données pas nécessaire voire nuisible ici ?
Par exemple, sans migration, est-ce qu’on considère que c’est au widget d’être capable de prendre en entrée des numéros en des formats variés et de néanmoins tenter de les afficher correctement ?
Est-ce qu’on essaie aussi de continuer à gérer une certaine unicité des numéros de téléphone sur une instance ?
J’ai en tête le cas où on se passe complètement d’email pour identifier l’usager, et où on ne lui demande pas de définir un username non plus.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
Paul Marillonnet a écrit :
Ce qui m’embête ici est que le format international globalement unique permettait de faire du numéro de téléphone une information identifiante pour l’usager.
C’était une anticipation de la situation où des comptes seraient créés sans courriel.
Si on ne migre pas les numéros existants, je n’ai aucune idée de comment on peut prévoir cette situation.
Je dirai bien que les comptes ayant un numéro de téléphone qui n'est pas au format E164 ont les contraintes :
1. ils ne peuvent pas avoir le flag phone_verified (normalement c'est automatique, on ne touche à rien donc phone_verified is NULL), toute validation par changement du numéro actuel forcera la validation et le format E164
2. ces comptes sont exclus des recherches d'unicité de fait (puisqu'on ne cherche que le format E164), mais ce n'est pas grave ils ne sont pas vérifiés
Et là en réfléchissant je me dis qu'il y a peut-être des comptes déjà au format E164 ce qui casse 2., je dirai donc que les comptes ayant une email non-vérifié sont exclus de la recherche d'unicité. On ne pourra avoir qu'un seul compte avec un numéro de téléphone vérifié en toute circonstance, c'est donc, vaguement, un identifiant.
Il faut voir qu'on a une notion un peu lâche de ce qu'est un identifiant ici, ça veut juste dire un truc unique à l'inscription, nul part on ne suppose que les emails ou numéro de téléphone sont uniques ailleurs dans Publik sauf rare cas SSO OIDC entre deux publics qui n'existe que sur GLC ou vaguement dans des créations de compte pilotées depuis un formulaire (avec ?get_or_create=email).
Mis à jour par Paul Marillonnet il y a plus d'un an
- Fichier 0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch 0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch ajouté
- Fichier 0002-widgets-use-libphonenumbers-local-formatting-69365.patch 0002-widgets-use-libphonenumbers-local-formatting-69365.patch ajouté
- Fichier 0003-utils-misc-add-parse_phone_number_utility-69365.patch 0003-utils-misc-add-parse_phone_number_utility-69365.patch ajouté
- Fichier 0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch 0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch ajouté
- Fichier 0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch 0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch ajouté
- Statut changé de En cours à Solution proposée
Mis à jour par Benjamin Dauvergne il y a plus d'un an
0001: ici je vois
dial = (
settings.PHONE_COUNTRY_CODES.get(country_code, {}).get('lang', None)
or settings.DEFAULT_COUNTRY_CODE
)
mais dans 0003 :
settings.PHONE_COUNTRY_CODES[settings.DEFAULT_COUNTRY_CODE]['lang'],
J'ai un peu du mal à voir le type de donnée qu'il y a dans settings.DEFAULT_COUNTRY_CODE, on dirait que ce n'est pas la même chose que dans lang or dans le premier patch on fait comme si c'était pareil.
0005: ce qui est fait pour le champ phone devrait être fait pour tous les champs de type 'phone_number' vu que tu as changé la correspondance dans 0004
Mis à jour par Paul Marillonnet il y a plus d'un an
- Statut changé de Solution proposée à En cours
Benjamin Dauvergne a écrit :
0001: ici je vois [...]
mais dans 0003 :
[...]
J'ai un peu du mal à voir le type de donnée qu'il y a dans settings.DEFAULT_COUNTRY_CODE, on dirait que ce n'est pas la même chose que dans lang or dans le premier patch on fait comme si c'était pareil.
Oui, en effet, c’est la structure de settings.PHONE_COUNTRY_CODES qui a changé et je me suis planté dans le fallback sur la valeur par défaut dans 0001. Je corrige.
0005: ce qui est fait pour le champ phone devrait être fait pour tous les champs de type 'phone_number' vu que tu as changé la correspondance dans 0004
Oui ok bien vu, je fais ça.
Mis à jour par Paul Marillonnet il y a plus d'un an
- Fichier 0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch 0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch ajouté
- Fichier 0002-widgets-use-libphonenumbers-local-formatting-69365.patch 0002-widgets-use-libphonenumbers-local-formatting-69365.patch ajouté
- Fichier 0003-utils-misc-add-parse_phone_number_utility-69365.patch 0003-utils-misc-add-parse_phone_number_utility-69365.patch ajouté
- Fichier 0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch 0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch ajouté
- Fichier 0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch 0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch ajouté
- Statut changé de En cours à Solution proposée
Les deux soucis identifiés ici, corrigés dans cette nouvelle version.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
- Statut changé de Solution proposée à Solution validée
Mis à jour par Paul Marillonnet il y a plus d'un an
- Statut changé de Solution validée à Résolu (à déployer)
commit 3086948b0ebbb321c68191c8036a887889968dc1
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Wed Sep 21 15:09:39 2022 +0200
csv_import adapt user csv logic to new phone_number kind (#69365)
commit 2c0443d1bf94d5cdc2b92361875bb6e928abf8bc
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Wed Sep 21 11:38:11 2022 +0200
attribute_kinds: use custom PhoneField for phone_number type (#69365)
commit 62bb199d56b3da01e161388ec9fc914bb09d7196
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Thu Oct 20 11:34:00 2022 +0200
utils/misc: add parse_phone_number_utility (#69365)
commit 9ebd736adbd8d245ab945030e7d3797a1ec0aec7
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Wed Nov 2 12:31:08 2022 +0100
widgets: use libphonenumbers' local formatting (#69365)
commit 0b6f7e1b804c69ba8f036dd9160183a63b5385c5
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Thu Oct 20 11:58:44 2022 +0200
fields: fix default dial code retrieval in PhoneField (#69365)
Mis à jour par Transition automatique il y a plus d'un an
- Statut changé de Résolu (à déployer) à Solution déployée
fields: fix default dial code retrieval in PhoneField (#69365)