Projet

Général

Profil

Development #51294

ldap: move messages from hard coded to templates

Ajouté par Loïc Dachary il y a environ 3 ans. Mis à jour il y a presque 3 ans.

Statut:
Nouveau
Priorité:
Normal
Assigné à:
Catégorie:
LDAP
Version cible:
-
Début:
19 février 2021
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Non
Planning:
Non

Description

Les messages d'erreur et d'avertissement sont actuellement en dur dans les sources. Ils sont traduits mais ne peuvent pas être adaptés après l'installation. Pour y remédier il est proposé de charger les messages depuis des templates.


Fichiers

Historique

#1

Mis à jour par Loïc Dachary il y a environ 3 ans

Voici un patch (incomplet) qui illustre l'approche. Est-ce que vous pensez que ça va dans la bonne direction ?

#2

Mis à jour par Loïc Dachary il y a environ 3 ans

Avec le fichier template qui avait été oublié, pour mieux illustrer (ça passe les tests).

#3

Mis à jour par Benjamin Dauvergne il y a environ 3 ans

django.contrib.messages n'accepte pas d'HTML, seulement des chaînes simples, donc ici l'utilisation de templates me semble exagéré, tu peux simplement utiliser un nouveau setting custom_error_message contenant un dico dans LDAP_SETTINGS, ex.:

LDAP_SETTINGS = [
  {
    ....
    'custom_error_messages': {
        'accountLocked': ...
    }
  }
]

à terme ça pourra être remplacé par une interface graphique, quand on pourra configurer un annuaire LDAP directement dans /manage/.

#4

Mis à jour par Loïc Dachary il y a environ 3 ans

Je vais suivre ton conseil, merci :-) Une question subsidiaire: est-ce que tu me confirmes qu'il est possible d'avoir des chaînes localisables dans les settings ? Par exemple:

```
LDAP_SETTINGS = [ {
....
'custom_error_messages': {
'accountLocked': _('tagada {tsoin}'),
}
}
]
```

J'imagine que oui mais il n'est pas impossible qu'il y ait un problèmes de poule et d'oeuf et qu'il faille lire le fichier de config pour initialiser la localisation et que ce soit en fait pas possible.

Si ce n'est pas possible d'avoir des traductions dans les settings alors on pourrait peut-être avoir des templates textes (.txt comme il y en a quelque uns) histoire de rappeler qu'il ne faut pas mettre de HTML dedans.

Qu'en dis-tu ?

#5

Mis à jour par Loïc Dachary il y a environ 3 ans

Alors j'ai fait un joli patch qui sert juste a rien parce que je n'avais pas compris que les settings devaient être sérialisables en JSON. Je recommence.

#6

Mis à jour par Loïc Dachary il y a environ 3 ans

Ce patch ajoute les messages comme suggéré dans LDAP_AUTH_SETTINGS et les tests passent et c'est internationalisé. Mais il y a un soucis pour les messages qui passent par ngettext. Il y a trois possibilités:

  • les messages qui requiert ngettext sont interdits
  • hacker un truc qui contourne le problème tout en laissant les messages dans LDAP_AUTH_SETTINGS
  • mettre les messages dans des templates .txt et profiter du fait que les templates implémentent tout les cas comme il faut

J'aurais tendance a choisir les template .txt parce que, même si c'est un peu overkill, c'est carré.

Qu'en dites vous ?

#7

Mis à jour par Loïc Dachary il y a environ 3 ans

même si c'est un peu overkill,

Je voulais dire "exagéré" et pas "overkill", excuse my english :blush:

#8

Mis à jour par Frédéric Péters il y a environ 3 ans

Qu'en dites vous ?

Perso plutôt non; ça casserait ou compliquerait à outrance le "à terme ça pourra être remplacé par une interface graphique".

#9

Mis à jour par Loïc Dachary il y a environ 3 ans

Ok, merci pour le retour rapide :-) Je vais opter pour supposer que ngettext n'est pas possible. Ca ne va pas être une catastrophe mondiale et ce sera simple.

#10

Mis à jour par Loïc Dachary il y a environ 3 ans

Voici un patch qui est prêt à être revu: il déplace simplement les messages dans LDAP_AUTH_SETTINGS et en permet la traduction. Si les messages sont changés dans settings, il faut faire une opération en plus pour ajouter les traductions, bien sur et c'est la responsabilité de l'admin.

Est-ce que ça pourrait aller ou bien il y a des choses que je n'ai pas vues ?

#11

Mis à jour par Loïc Dachary il y a environ 3 ans

Ping ?

#12

Mis à jour par Loïc Dachary il y a environ 3 ans

Ping ?

#13

Mis à jour par Loïc Dachary il y a environ 3 ans

Rebase sur main, avec résolution les conflits qui empêchait le merge.

#14

Mis à jour par Valentin Deniaud il y a presque 3 ans

  • Assigné à changé de Benjamin Dauvergne à Loïc Dachary

Ça m'a l'air bien, mais attendons #51239 qui touche au même code (d'ailleurs on tolère tout à fait que de tels patches se suivent, genre tu aurais pu soumettre un patch fait pour s'appliquer directement sur #51239).

Formats disponibles : Atom PDF