Project

General

Profile

Development #51294

ldap: move messages from hard coded to templates

Added by Loïc Dachary about 3 years ago. Updated almost 3 years ago.

Status:
Nouveau
Priority:
Normal
Assignee:
Category:
LDAP
Target version:
-
Start date:
19 February 2021
Due date:
% Done:

0%

Estimated time:
Patch proposed:
No
Planning:
No

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.


Files

History

#1

Updated by Loïc Dachary about 3 years ago

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

#2

Updated by Loïc Dachary about 3 years ago

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

#3

Updated by Benjamin Dauvergne about 3 years ago

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

Updated by Loïc Dachary about 3 years ago

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

Updated by Loïc Dachary about 3 years ago

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

Updated by Loïc Dachary about 3 years ago

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

Updated by Loïc Dachary about 3 years ago

même si c'est un peu overkill,

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

#8

Updated by Frédéric Péters about 3 years ago

Qu'en dites vous ?

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

#9

Updated by Loïc Dachary about 3 years ago

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

Updated by Loïc Dachary about 3 years ago

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

Updated by Loïc Dachary about 3 years ago

Ping ?

#12

Updated by Loïc Dachary about 3 years ago

Ping ?

#13

Updated by Loïc Dachary about 3 years ago

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

#14

Updated by Valentin Deniaud almost 3 years ago

  • Assignee changed from Benjamin Dauvergne to 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).

Also available in: Atom PDF