Project

General

Profile

Development #69365

custom_user : migrer les données de user.attributes.phone au format E.164

Added by Paul Marillonnet 5 months ago. Updated about 2 months ago.

Status:
Solution déployée
Priority:
Normal
Category:
-
Target version:
-
Start date:
21 September 2022
Due date:
% Done:

0%

Estimated time:
Patch proposed:
Yes
Planning:
No

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

0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch (6.75 KB) 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch Paul Marillonnet, 22 September 2022 11:36 AM
0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch (11.5 KB) 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch Paul Marillonnet, 22 September 2022 11:36 AM
0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch (10.6 KB) 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch Paul Marillonnet, 22 September 2022 11:36 AM
0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch (7.44 KB) 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch Paul Marillonnet, 06 October 2022 12:00 PM
0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch (11.8 KB) 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch Paul Marillonnet, 06 October 2022 12:00 PM
0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch (10.6 KB) 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch Paul Marillonnet, 06 October 2022 12:00 PM
0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch (7.44 KB) 0003-migrations-set-existing-user-phone-fields-to-E.164-v.patch Paul Marillonnet, 06 October 2022 02:12 PM
0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch (11.8 KB) 0002-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch Paul Marillonnet, 06 October 2022 02:12 PM
0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch (12 KB) 0001-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch Paul Marillonnet, 06 October 2022 02:12 PM
0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch (1.14 KB) 0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch Paul Marillonnet, 24 November 2022 10:04 AM
0002-widgets-use-libphonenumbers-local-formatting-69365.patch (1.01 KB) 0002-widgets-use-libphonenumbers-local-formatting-69365.patch Paul Marillonnet, 24 November 2022 10:04 AM
0003-utils-misc-add-parse_phone_number_utility-69365.patch (1.36 KB) 0003-utils-misc-add-parse_phone_number_utility-69365.patch Paul Marillonnet, 24 November 2022 10:04 AM
0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch (11.2 KB) 0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch Paul Marillonnet, 24 November 2022 10:04 AM
0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch (11.6 KB) 0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch Paul Marillonnet, 24 November 2022 10:04 AM
0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch (1.17 KB) 0001-fields-fix-default-dial-code-retrieval-in-PhoneField.patch Paul Marillonnet, 09 December 2022 12:04 PM
0002-widgets-use-libphonenumbers-local-formatting-69365.patch (1.01 KB) 0002-widgets-use-libphonenumbers-local-formatting-69365.patch Paul Marillonnet, 09 December 2022 12:04 PM
0003-utils-misc-add-parse_phone_number_utility-69365.patch (1.36 KB) 0003-utils-misc-add-parse_phone_number_utility-69365.patch Paul Marillonnet, 09 December 2022 12:04 PM
0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch (11.2 KB) 0004-attribute_kinds-use-custom-PhoneField-for-phone_numb.patch Paul Marillonnet, 09 December 2022 12:04 PM
0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch (12 KB) 0005-csv_import-adapt-user-csv-logic-to-new-phone_number-.patch Paul Marillonnet, 09 December 2022 12:04 PM

Related issues

Related to Publik - Development #49212: Création de compte avec un numéro de téléphone mobileEn cours01 October 2021

Actions

Associated revisions

Revision 0b6f7e1b (diff)
Added by Paul Marillonnet 2 months ago

fields: fix default dial code retrieval in PhoneField (#69365)

Revision 9ebd736a (diff)
Added by Paul Marillonnet 2 months ago

widgets: use libphonenumbers' local formatting (#69365)

Revision 62bb199d (diff)
Added by Paul Marillonnet 2 months ago

utils/misc: add parse_phone_number_utility (#69365)

Revision 2c0443d1 (diff)
Added by Paul Marillonnet 2 months ago

attribute_kinds: use custom PhoneField for phone_number type (#69365)

Revision 3086948b (diff)
Added by Paul Marillonnet 2 months ago

csv_import adapt user csv logic to new phone_number kind (#69365)

History

#1

Updated by Paul Marillonnet 5 months ago

  • Related to Development #49212: Création de compte avec un numéro de téléphone mobile added
#2

Updated by Paul Marillonnet 5 months ago

  • Status changed from Nouveau to En cours
  • Assignee set to Paul Marillonnet
#3

Updated by Paul Marillonnet 5 months ago

0001 on modifie le type d’attribut 'phone_number' pour utiliser le champ PhoneField défini dans #69221.
0002 on modifie la logique d’import csv en conséquence.
0003 on applique la migration des données existantes.

Il reste la partie api à gérer pour conserver ce format E.164, c’est #69430.

#4

Updated by Agate Berriot 5 months 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!

#5

Updated by Paul Marillonnet 5 months 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.

#6

Updated by Paul Marillonnet 5 months 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.

#7

Updated by Benjamin Dauvergne 4 months 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.

#8

Updated by Benjamin Dauvergne 4 months ago

  • Status changed from Solution proposée to Information nécessaire
#9

Updated by Benjamin Dauvergne 4 months 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).

#10

Updated by Paul Marillonnet 4 months 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.

#11

Updated by Paul Marillonnet 4 months ago

  • 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.

#12

Updated by Paul Marillonnet 4 months ago

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.

#13

Updated by Benjamin Dauvergne 4 months ago

  • Status changed from Solution proposée to Solution validée
#14

Updated by Paul Marillonnet 3 months 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

#15

Updated by Paul Marillonnet 3 months 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.

#16

Updated by Paul Marillonnet 3 months 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.

#17

Updated by Benjamin Dauvergne 3 months 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).

#19

Updated by Benjamin Dauvergne 2 months 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

#20

Updated by Paul Marillonnet 2 months 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.

#22

Updated by Benjamin Dauvergne 2 months ago

  • Status changed from Solution proposée to Solution validée
#23

Updated by Paul Marillonnet about 2 months 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)
#24

Updated by Transition automatique about 2 months ago

  • Status changed from Résolu (à déployer) to Solution déployée

Also available in: Atom PDF