add unique constraint on email_role

[lpofredc]

Ajout d'une contrainte d'unicité sur le champ email de la table utilisateurs.t_roles comme évoqué sur cette issue de UsersHub (PnX-SI/UsersHub#122)

fix PnX-SI/UsersHub#122

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0d269c8) 37.67% compared to head (992619f) 37.67%.
Report is 55 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop      #56   +/-   ##
========================================
  Coverage    37.67%   37.67%           
========================================
  Files           12       12           
  Lines          698      698           
========================================
  Hits           263      263           
  Misses         435      435           

Flag	Coverage Δ
pytest	37.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[camillemonchicourt]

Peut-être que la migration doit gérer le cas où 2 utilisateurs ont le même email ?
L'indiquer clairement et s’arrêter ?
Ou alors supprimer les emails en doublon, mais ça peut être problématique car on supprimerait des emails sans l'indiquer ?

[mvergez]

Je rebondis sur cette PR dans le cadre d'une prestation pour l'Agence Régionale de la Biodiversité en île de France.

Je ne pense pas qu'il faille supprimer les emails en doublon mais plutôt avertir l'administrateur. Si on applique cette migration, on a une erreur du type :

sqlalchemy.exc.IntegrityError: (psycopg2.errors.UniqueViolation) could not create unique index "t_roles_email_un"

Mais sous une backtrace énorme.

Donc :

• Soit on laisse cette backtrace, tout en informant les administrateurs dans la release pour qu'ils fassent le ménage avant
• Soit on exécute une requête dans la base pour récupérer tous les doublons et on raise une erreur plus lisible s'il y en a ?
• Soit on fait un try except ici : https://github.com/PnX-SI/UsersHub-authentification-module/pull/56/files#diff-c452483e96fabcd77375a32d265703a0b3b45b78f78a2e21fd7c3d72d7c39f20R19-R24 avec également une erreur plus lisible.
• Autres idées ?

[camillemonchicourt]

Pour moi, c'est un soucis que les migrations puissent échouer, et que les administrateurs doivent comprendre pourquoi, et ensuite lancer des commandes Alembic qu'ils ne maîtrisent pas.

[mvergez]

Alors il faut les prévenir dans une note de version de GeoNature, je ne vois pas comment on pourrait faire autrement. Parce que même si on met un message le plus clair possible dans alembic, ils ne vont pas savoir comment relancer les migrations.

Qu'en penses-tu ?

[camillemonchicourt]

Oui