Développement #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.
Files
Related issues
Associated revisions
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)
History
Updated by Paul Marillonnet about 2 years ago
- Related to Développement #49212: Création de compte avec un numéro de téléphone mobile added
Updated by Paul Marillonnet about 2 years ago
- Status changed from Nouveau to En cours
- Assignee set to Paul Marillonnet
Updated by Paul Marillonnet about 2 years ago
- File 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch added
- File 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch added
- File 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch added
- Status changed from En cours to Solution proposée
- Patch proposed changed from No to Yes
Updated by A. B. about 2 years ago
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!
Updated by Paul Marillonnet about 2 years ago
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.
Updated by Paul Marillonnet about 2 years ago
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.
Updated by Benjamin Dauvergne about 2 years ago
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.
Updated by Benjamin Dauvergne about 2 years ago
- Status changed from Solution proposée to Information nécessaire
Updated by Benjamin Dauvergne about 2 years ago
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).
Updated by Paul Marillonnet about 2 years ago
- Status changed from Information nécessaire to 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.
Updated by Paul Marillonnet about 2 years ago
- File 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch added
- File 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch added
- File 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch added
- Status changed from En cours to 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.
Updated by Paul Marillonnet about 2 years ago
- File 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch added
- File 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch added
- File 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch added
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
decompress
du 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.
Updated by Benjamin Dauvergne about 2 years ago
- Status changed from Solution proposée to Solution validée
Updated by Paul Marillonnet about 2 years ago
- Status changed from Solution validée to 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
Updated by Paul Marillonnet about 2 years ago
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.
Updated by Paul Marillonnet about 2 years ago
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.
Updated by Benjamin Dauvergne about 2 years ago
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
).
Updated by Paul Marillonnet about 2 years ago
- File 0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch 0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch added
- File 0002-widgets-use-libphonenumbers-local-formatting-69365.patch 0002-widgets-use-libphonenumbers-local-formatting-69365.patch added
- File 0003-utils-misc-add-parse_phone_number_utility-69365.patch 0003-utils-misc-add-parse_phone_number_utility-69365.patch added
- File 0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch 0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch added
- File 0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch 0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch added
- Status changed from En cours to Solution proposée
Updated by Benjamin Dauvergne about 2 years ago
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
Updated by Paul Marillonnet about 2 years ago
- Status changed from Solution proposée to 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.
Updated by Paul Marillonnet about 2 years ago
- File 0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch 0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch added
- Status changed from En cours to Solution proposée
- File 0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch 0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch added
- File 0002-widgets-use-libphonenumbers-local-formatting-69365.patch 0002-widgets-use-libphonenumbers-local-formatting-69365.patch added
- File 0003-utils-misc-add-parse_phone_number_utility-69365.patch 0003-utils-misc-add-parse_phone_number_utility-69365.patch added
- File 0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch 0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch added
Les deux soucis identifiés ici, corrigés dans cette nouvelle version.
Updated by Benjamin Dauvergne about 2 years ago
- Status changed from Solution proposée to Solution validée
Updated by Paul Marillonnet about 2 years ago
- Status changed from Solution validée to 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)
Updated by Transition automatique almost 2 years ago
- Status changed from Résolu (à déployer) to Solution déployée
fields: fix default dial code retrieval in PhoneField (#69365)