nursit / bank

module de paiement bancaire multi prestataires & stockage des transactions pour SPIP
21 stars 22 forks source link

Surcharger affiche_monnaie #49

Closed tcharlss closed 3 years ago

tcharlss commented 4 years ago

J'ai pas l'impression qu'il soit possible de surcharger la fonction affiche_monnaie dans un autre plugin. Si on met un <necessite nom="bank">, on passe après et donc le if (function_exists) n'a aucun effet.

Possible d'en faire une fonction dist ?

Le but c'est d'avoir la bonne devise avec le plugin prix.

Cerdic commented 4 years ago

La bonne devise ?

Oui on peut rendre cette fonction, surchargeable mais si c'est un problème de devise, alors ça risque plutôt d'être piegeux : rien ne va marcher à côté de ça, il y a beaucoup de choses à reprendre pour gérer une devise qui ne soit pas les Euros.

Au moins que la devise affichée soit mauvaise permet de comprendre que derrière ça va surement pas marcher...

(surement en lien avec #48 donc)

rastapopougros commented 4 years ago

Pour l'instant c'est sans question de gérer plusieurs devises dans le même site. Le #48 permet surtout ça, de pouvoir plus tard permettre plusieurs devises, en les passant. Quand il n'y a qu'une seule et unique devise, alors 1) on la configure chez le prestataire bancaire qu'on a choisit (et à peu près tous permettent cela, dire que par défaut les paiements arrivent en dollars, en yens, etc mais config unique) 2) on la configure dans le site SPIP (avec Prix à jour)

Pourquoi "rien ne va marcher" ? Bank ne stocke que des nombres seuls, et il affiche les montants. On veut donc les afficher avec la devise choisie pour le site (qui est unique pour le moment je le répète)

rastapopougros commented 4 years ago

Je précise pour les prestas : tous ceux qui sont pas juste français. Si t'es un site japonais (on a le cas), tu vas pas activer un prestataire franco-français qui sait de base faire que l'euro (mais Stripe ou ce genre).

Cerdic commented 4 years ago

Au hasard parce que pour tout les prestataires on envoie une demande de paiement en précisant que la devise est Euros dans le plugin bank ?

Tu pourras configurer ce que tu veux dans l'interface du prestataire, la demande de paiement se fera toujours en Euros avec le plugin bank.

Je parle même pas de gérer plusieurs devises sur un même site, mais à ce jour le plugin est même pas prévu ni capable de gérer tout dans une seule devise qui serait autre que Euros.

Si on a besoin de ça il faut commencer par faire une revue de code de tout le plugin, gérer la configuration de la devise du site (ie une seule possible), et voir tout ce que ça impacte, notamment pour chaque presta adapter le parametre devise dans la demande de paiement.

Mais je n'ai pas en tête tous les impacts possibles (un des autres impacts est que potentiellement tu n'arrondis pas au même nombre de décimale selon la devise utilisée, toutes n'ont pas des cents)

tcharlss commented 4 years ago

Ah oui, on est complètement passés à côté là :/ On pensait que seul le montant était communiqué au presta lors d'une transaction, sans préciser de devise.

Sans avoir à tout réviser en profondeur, est-ce qu'il n'y aurait pas possibilité de débrayer la devise principale à titre expérimental, en mode "si je la change, j'assume les conséquences" ?

tcharlss commented 4 years ago

Et on peut tenter un PR si ça te semble pas trop casse-gueule

Cerdic commented 4 years ago

Il y a plusieurs choses :

Perso je suis franchement pas pour intégrer une PR bancale ou genre rustine. A chaque fois qu'on laisse une possibilité de faire des bêtises il y aura quelqu'un qui l'utilisera et ça sera un problème.

A minima il faut qu'on traite le problème proprement pour tous les prestas, que l'option soit réellement utilisable (et ça suppose aussi de gérer les cas où un presta ne supporte pas la devise demandée dans la configuration)

rastapopougros commented 4 years ago

ok ok… alors c'est piégeux, parce qu'absolument nulle part ce n'est indiqué dans la doc de Bank que ça ne gère que l'euro en l'envoyant de force. Or comme ça gère certains services qui gèrent parfaitement d'autres devises (stripe, paypal…), et bien sans autre info, on en déduit que c'est sûrement bon tant qu'on utilise ces services là. :)

Pour contexte, là le Diplo a des sites (SPIP évidemment) pour les éditions un peu partout, Japon, Chili, etc, et comme yavait Stripe et autre, tout paraissait pas mal pour payer leurs abos et autres produits avec SPIP.

Du coup avant tout, m'est-avis que la toute toute première chose à faire, c'est : indiquer au début tout en haut de la documentation de Bank sur Contrib, que pour le moment ça ne gère que l'euro en dur ! En gros et gras. Au moins les suivants après nous se feront pas piéger. :)

Cerdic commented 4 years ago

je ne pense pas que tu puisses faire une demande de paiement avec un montant sans indiquer de devise. On est dans du paiement en ligne, quand même, on ne peut pas souffrir le malentendu ni l'ambiguité. Une demande de paiement c'est un montant avec une devise.

Pas certain qu'une seule API considère que la devise est optionnelle...

JuLBLoBuL commented 4 years ago

Bonjour, Pour ma part j'utilise BANK sur des sites dans de nombreux pays ;) J'ai déjà commenté ce sujet, je suis très embêté également car je suis obligé de mettre à jour ma version "custom". Les corrections à apporter sont pourtant mince... J'ai rajouter un menu de sélection de devise, dans la page de configuration de BANK. Puis dans mes prestas, je fais appel à cette configuration, j'utilise Paypal, stripe et des prestas maisons, et ca roule sans soucis. Je serais heureux si nous trouvions une solution pérenne. J'ai déjà tout expliqué dans le forum. Pas de réponse... Merci en tout cas pour ce super plugin. JuL

rastapopougros commented 4 years ago

@JuLBLoBuL on a fait une branche qui normalement modifie de manière exhaustive dans tous les prestas ici, à tester : https://github.com/MuktFr/bank/tree/dev/devises

Il manque juste encore la modif de l'affichage, mais le plus gros est là et bien complet. Dès qu'on aura modifié l'affichage aussi, on la montre en PR.

rastapopougros commented 4 years ago

Pour préciser :

Avec tout ça, normalement c'est bien carré et complet, d'après nous il ne manque rien (sauf l'affichage final comme dit) mais à tester. :)

JuLBLoBuL commented 4 years ago

Merci pour toutes ces infos! Je n'utilises pas le plugin PRIX pour le moment, si je comprends bien, je dois l'installer pour sélectionner les devises à mettre à disposition des prestas?

rastapopougros commented 4 years ago

Si tu veux pouvoir choisir la devise dans un sélecteur oui, pour l'instant c'est uniquement dans Prix (mais c'est encore en réflexion).

Mais si ta devise est toujours la même, tu peux aussi t'inscrire dans le pipeline cité toi-même hein, et fournir le tableau de description de ta devise, et basta.

JuLBLoBuL commented 4 years ago

En fait j'avais essayer d'envoyer une modification de Bank via GIT, incluant un sélecteur piochant dans un fichier XML avec les devises en ISO. C'est ce que je fais pour mes sites et c'est assez pratique :) Il y a t'il une documentation sur l'utilisation ce nouveau pipeline et le format du tableau de description?

rastapopougros commented 4 years ago

Liste du plugin Prix est mieux, c'est une librairie super complète, pas juste les devises mais leurs caractéristiques (arrondis, symboles, langue, etc).

Pour le pipeline c'est là et ça attend comme ce qui est mis pour l'euro par défaut, le code lettre, le code numérique, et l'arrondi : https://github.com/MuktFr/bank/blob/dev/devises/bank_options.php#L188

JuLBLoBuL commented 4 years ago

okay! Je viens d'installer l'API Prix, c'est parfait ;) Je vais tester la version de test de BANK donc, qui est normalement fonctionnel, n'est-ce pas? Je vais publier dans les jours qui viennent un site au canada avec stripe, ca devrait rouler tout seul? Quels sont les manques coté affichage ?

rastapopougros commented 4 years ago

Bah côté affichage c'est… l'objet principal de ce ticket justement :) Càd la fonction fournie par bank, et qu'il utilise dans l'admin, et dans la génération des factures à priori (plugin Factures). Il faudrait déjà l'améliorer dans Bank même pour prendre en compte la devise par défaut de Bank, et il faudrait la rendre surchargeable aussi (car Prix pourrait la surcharger car ça fait mieux et plus complet question affichage).

JuLBLoBuL commented 4 years ago

Ok. Merci pour ces infos. J'essaie d'intégrer l'API PRIX à mon plugin perso (j'utilise pas FACTURES) et la première chose dont je me rends compte que dans la page de config de l'API PRIX, on peut choisir la devise (super) par contre pour dans "Langues et formatage des prix", je n'ai que trois choix (francais, fr canadien et fr suisse), quelque soit la devise. C'est normal? Merci

tcharlss commented 4 years ago

@JuLBLoBuL Je réponds ici mais je suggère de continuer ensuite dans la tracker du plugin prix, sinon on ne va plus s'y retrouver.

Oui c'est voulu qu'il n'y ait que ces 3 options : on fait en sorte de ne mettre que celles qui correspondent aux langues configurées sur le site ("langue principale du site" + menu multilinguisme). Je suppose que c'est le français dans ton cas, donc paf, 3 options. Si tu actives d'autres langues, tu auras autant d'options correspondantes.

Je ne vois pas de cas où il y aurait besoin d'utiliser une locale qui n'a rien à voir avec les langues configurées : par ex. si mon site est en anglais est-ce qu'il aurait une raison de vouloir formater par défaut les prix dans une autre langue ?

En fait il faudrait revoir un peu le wording de cette partie de la conf : il ne s'agit pas de configurer des langues mais plutôt des locales. Ou disons plutôt : choisir des locales selon les langues de spip configurées.

JuLBLoBuL commented 4 years ago

Bonjour, Finalement j'ai laissé tombé cette API (Prix), car elle ne semble pas surcharger BANK, et la configuration\utilisation n'est très clair. Je suis revenu sur ma version custom de BANK qui est plus simple. Je ne perds rien car je dispose aussi des même infos. Mon fichier XML, dispose également des arrondis et autre. Cela me semble être une solution plus simple et plus élégante. En ce qui concerne la localisation, mes sites sont tous en français (parfois multilingue) mais les paiements se font en monnaie locale. Je serais heureux de proposer ma solution de manière pérenne si cela est possible...

rastapopougros commented 4 years ago

Je ne suis pas sûr d'avoir tout saisi.

Tu voudrais que Prix surcharge quoi de Bank ? Si tu parles uniquement de afficher_montant… bah c'est l'objet de ce ticket justement, donc tant qu'il est pas résolu, évidemment que ça ne peut pas encore le faire…

Je ne sais pas pour quels besoins tu utilises Bank, mais pour ce qui est de la vente de choses ayant un prix, le plugin est l'API central dans la communauté, et il est nécessité par tout le reste (paniers, commandes, etc). Enfin bon, si ya un truc à dire, comme dit par tcharlss plus haut, c'est dans les tickets de la communauté plutôt qu'ici.

Par ailleurs je vois pas en quoi ce serait plus pérenne, Prix intègre une librairie PHP extrêmement maintenue, qui contient toutes les devises ET traduites dans toutes les langues ET avec des fonctions de formatage dédiées à chaque langue (les montants ne sont pas du tout affichés pareil suivant les langues !), donc c'est la seule manière d'être vraiment pérenne. Et comme on l'a déjà dit je ne sais plus où, cette lib va sûrement partir du plugin Prix pour aller dans un plugin séparé car doit pouvoir être utilisé pour d'autres montants que des prix (les prix étant un cas particulier des montants).

JuLBLoBuL commented 4 years ago

Merci de vos lumières. Bon, je vais juste attendre donc la prochaine version stable de BANK et PRIX permettant la gestion des devises étrangères. Si malgré tout, il serait envisageable de surcharger "affiche_monnaie", ce serait déjà un grand progrès. Je suis à votre disposition si je peux aider à quelque chose :)

Cerdic commented 3 years ago

Ce problème est donc résolu par 030836792295d14add474a91fa90f60ee1cb52f8