nursit/bank

Ajouter un champ devise sur spip_transactions

Closed this issue · 6 comments

cf #52 (comment)

La devise devrait être stockée dans la table spip_transactions

En lisant 17335c1 je pense qu'on a un problème structurel :

je pense que la fonction bank_devise_defaut() ne devrait pas être appelée quand on prépare le formulaire de paiement, pour chaque prestataire, mais quand on créé la transaction, la devise devant alors être stockée avec le montant qui n'a pas de sens si il n'est pas associé à une devise.

A partir du moment où la devise peut changer, il faut donc qu'elle figure dans la table des transactions, car

  • la devise a pu changer entre le moment où l'on a créé la transaction et le moment où l'on présente le formulaire de paiement
  • au retour de paiement, on doit bien vérifier que le montant et la devises correspondent à ce qu'on avait demandé au moment où la transaction a été créée - la devise par défaut ayant pu changer depuis, notamment sur les paiements lents de type chèque/virement

Hello, plutôt que dans une PR fermée donc cachée, c'est mieux de discuter dans un ticket ouvert. 😄

Donc je remets ici le questionnement que je posais dans la PR. Par contre, le point que je soulève vaut pour les trois tickets, en fait pour toute la milestone "Support des devises", pas juste pour ce ticket là.

Cependant j'ai une question pour le premier point, enfin les deux premiers.
La fonction bank_devise_defaut fournit (doit fournir) toutes les infos nécessaires, mais ça ne les fournit que pour la devise en cours. Or si on stocke la devise dans chaque transaction, on a donc potentiellement plusieurs devises au fil du temps dans le site (2, 3… peu importe), mais seulement son identifiant à priori (soit le code alpha, soit le code num), pas l'ensemble des informations dont on a besoin suivant les contextes. Sauf… à toujours tout stocker à chaque fois en méga duplication ? Bof bof. Car que ce soit pour l'affichage dans les interfaces longtemps après, ou pour les échanges avec le presta, on a besoin de toutes les infos : parfois le code alpha, parfois la décimale, etc.

Donc comment faire au mieux ? Concrètement le vrai mieux serait que Bank utilise aussi l'excellente librairie maintenue dans la commu Drupal, que l'on a intégré pour le moment au plugin Prix. Mais on sait que ça ne doit pas être son emplacement final ! Car d'autres plugins (comme Bank ou d'autres) en ont forcément besoin si on ne gère plus seulement l'euro. Est-ce que Bank devrait l'intégrer aussi en double ? Est-ce qu'on devrait faire un plugin du genre "Montants" pour l'intégrer seule, et les autres en dépendent, donc y compris Bank ? (ça ne me choque pas qu'il dépende d'un plugin aussi central comme ça)
https://git.spip.net/spip-contrib-extensions/prix/src/branch/master/vendor/commerceguys/intl

Je disais dans mon commentaire précédent qu'il fallait la liste complète des devises existantes (et leurs infos, noms humains, codes etc) "pour le premier point, enfin les deux premiers".

Mais en fait aussi pour le troisième ! Puisque si on doit pouvoir configurer (donc dans une interface, dans un select à priori) la devise dans la config de chaque ajout de type "chèque", alors forcément il faut aussi cette liste complète (avec code et label humain) pour ce point aussi !

Donc au final : il faut absolument la liste que fournit la super lib tenue à jour et qui fournit toutes les infos nécessaires pour l'ensemble des trois points que tu demandes.

Donc il faut vraiment faire un choix d'où mettre cette lib :

  • en doublon dans le plugin Bank ?
  • dans un plugin "Devises" ou "Montants" sur la commu, nécessité obligatoirement par Bank ?
  • je sais pas si possible : dans un plugin "Devises" ou "Montants" sur la commu utilisé seulement par Bank si présent, et si pas présent, il s'en sort avec les euros par défaut partout ?

Si vraiment dans ta tête, ta conception, tu préfères que le plugin Bank ne dépende jamais de rien, on peut tenter le dernier… Mais là comme ça je me dis quand même que quand on parle d'échange d'argent, c'est toujours sur une devise précise, et ça paraitrait pas mal qu'à partir de cette amélioration, le cœur du système prenne bien en compte explicitement tout ça, et que du coup ce ne soit pas "optionnel". Je sais pas ce que t'en pense…
En tout cas si c'est pas optionnel, c'est forcément dans un plugin commun, car c'est sûr et certains que d'autres plugins ont besoin de cette liste de devises maintenues, même sans utiliser Bank.
(Et oui en vrai yorait pas à se poser cette question si on pouvait nécessiter chacun des libs Composer 😛 )

Le plugin peut avoir par défaut une liste de devises qui se limite à l'euro, et qui assure donc les fonctionnalités à l'identique, et si on a le plugin devises avec la lib qui va bien et l'api qui va bien, il y récupère la liste complète et extensive des devises possibles.

Et du coup le plugin devises n'est nécessaire que si on veut vraiment faire des devises étrangères hors euro, ça me parait un bon compromis

Yes ok, j'allais bien dans ce sens pour le moment. :)

Le petit plus quand même de l'avoir vraiment de base, c'est que la lib ne fait pas que fournir les devises : elle fournit aussi le formatage officiel des montants dans chaque langues/pays suivant chaque devise (utilisé désormais dans Prix au lieu d'un truc bidouillé en interne). Et du coup ça aurait permis d'avoir le même affichage uniformisé partout.

Mais on devrait pouvoir faire un truc similaire pour l'affichage aussi : utiliser le code actuel sans changement par défaut, et si ya le plugin Devises et sa lib, ça redirige l'affichage vers la super fonction complète mutualisée qui gère tout mieux.

alors c'est un bon compromis mais ça ébranle un peu la solidité, enfin on se repose un peu plus sur les épaules des admins pour pas tout casser, sans que ça ne l'interdise techniquement

je m'explique : si c'est un truc optionnel qui est permis parce qu'on a un plugin en plus, ça veut dire qu'on peut le désactiver à tout moment sans que Bank ne râle puisqu'il ne le nécessite pas

or… c'est tout l'objet de ce ticket, les devises ne vont plus être un truc utile seulement à un instant T où on a besoin : elles vont être gardées à l'infini en mémoire dans chaque transaction. Du coup si on utilise d'autres devises (enfin au moins une autre), et que ça permet de l'enlever, on se retrouve avec des transactions qui ont "USD" par ex car c'est l'identifiant unique officiel et que ça suffit, mais… plus sans les infos complètes de cette devise, dont on a peut-être besoin à un moment malgré tout (normalement non mais on peut pas tout prévoir). Alors bon quand on sait qu'on a utilisé un jour d'autres devises, c'est débile de désinstaller l'autre plugin ok, mais ça ne l'interdit pas, contrairement à un necessite, alors qu'on peut dès lors être embêté si plus là.

mais bon, on peut vivre avec ce risque, qui serait vraiment très rare… :)

C'est intégré, et le risque de casse en cas de desactivation du plugin intl est limité, notamment par 357205b