Facturation - trouver le payeur (sans garde alternée) (#74498) #31

Merged
lguerin merged 8 commits from wip/74498-invoicing-payer-id into main 2023-03-16 10:55:43 +01:00
Owner
No description provided.
lguerin force-pushed wip/74498-invoicing-payer-id from 78121eec41 to a3bd556147 2023-02-28 12:19:29 +01:00 Compare
lguerin force-pushed wip/74498-invoicing-payer-id from a3bd556147 to 6d85bd3bbc 2023-02-28 17:04:59 +01:00 Compare
lguerin force-pushed wip/74498-invoicing-payer-id from 6d85bd3bbc to ec3c510eaf 2023-02-28 18:26:18 +01:00 Compare
lguerin changed title from WIP: Facturation - trouver le payeur (sans garde alternée) (#74498) to Facturation - trouver le payeur (sans garde alternée) (#74498) 2023-02-28 18:26:33 +01:00
Author
Owner

0001: du détail, changement de l'affichage des extra variables au niveau d'un modèle de grille
0002: définition des variables du payeur

J'ai choisi de les poser sur le modèle de grille (Pricing), car les variables utilisées pour les critères y sont définies aussi, et utiliseront des filtres de requête, comme pour les variables du payer. Ça dépend donc des modèles de fiche en face. On aura certainement des modèles de grilles famille qui utilisent des modèles de fiche famille, et non des modèles de fiches d'une autre app.

J'aurais pu les poser sur la régie, mais je me dis que ça ne regarde pas le régisseur.

0003: split du champ user_name en user_first_name et user_last_name, ajout des champs payer* sur les models Line
0004: harmonisation, renommage des champs adult* en payer*
0005: ajout des champs payer* sur les models Invoice
0006: stockage des infos du payer sur chaque ligne et chaque facture à la génération

J'utilise un cache pour les infos du payeur, pour avoir les mêmes nom/prénom/etc partout pour une campagne donnée. Ca m'étonnerait qu'on aille chercher le nom d'un payer en date du (filtre de requête avec un |value_at), donc ça m'a semblé ok, et c'est bien pour les perfs.
(un payer_external_id doit être de la forme <key>:<id> - idéalement key a un rapport avec le modèle de fiche, exemple: adult:42, donc pas de risque de collision avec un payer trouvé sur des modèles de fiche différents)

0007: la mécanique pour calculer les valeur des variables de payeur
0008: affichage propre des erreurs sur les infos du payeur sur le journal

0001: du détail, changement de l'affichage des extra variables au niveau d'un modèle de grille 0002: définition des variables du payeur J'ai choisi de les poser sur le modèle de grille (Pricing), car les variables utilisées pour les critères y sont définies aussi, et utiliseront des filtres de requête, comme pour les variables du payer. Ça dépend donc des modèles de fiche en face. On aura certainement des modèles de grilles famille qui utilisent des modèles de fiche famille, et non des modèles de fiches d'une autre app. J'aurais pu les poser sur la régie, mais je me dis que ça ne regarde pas le régisseur. 0003: split du champ user_name en user_first_name et user_last_name, ajout des champs payer* sur les models Line 0004: harmonisation, renommage des champs adult* en payer* 0005: ajout des champs payer* sur les models Invoice 0006: stockage des infos du payer sur chaque ligne et chaque facture à la génération J'utilise un cache pour les infos du payeur, pour avoir les mêmes nom/prénom/etc partout pour une campagne donnée. Ca m'étonnerait qu'on aille chercher le nom d'un payer en date du (filtre de requête avec un |value_at), donc ça m'a semblé ok, et c'est bien pour les perfs. (un payer_external_id doit être de la forme `<key>:<id>` - idéalement key a un rapport avec le modèle de fiche, exemple: `adult:42`, donc pas de risque de collision avec un payer trouvé sur des modèles de fiche différents) 0007: la mécanique pour calculer les valeur des variables de payeur 0008: affichage propre des erreurs sur les infos du payeur sur le journal
lguerin added 8 commits 2023-03-03 10:49:22 +01:00
lguerin changed target branch from wip/74585-use-gadjo-styles to main 2023-03-10 08:53:55 +01:00
lguerin added 8 commits 2023-03-10 09:42:10 +01:00
Owner

Merci pour le rebase, je commence à relire aujourd’hui.

Merci pour le rebase, je commence à relire aujourd’hui.
pmarillonnet reviewed 2023-03-15 15:33:24 +01:00
pmarillonnet left a comment
Owner

Le code et les tests sont clairs pour un relecteur comme moi qui connais mal (voire pas du tout) la logique métier. Juste quelques petites remarques au passage, dis-moi ce que tu en penses.

Le code et les tests sont clairs pour un relecteur comme moi qui connais mal (voire pas du tout) la logique métier. Juste quelques petites remarques au passage, dis-moi ce que tu en penses.
@ -41,3 +41,3 @@
<td>{{ line.unit_amount }}</td>
<td>{{ line.total_amount }}</td>
<td>{{ line.user_name }} ({{ line.user_external_id }})</td>
<td>{{ line.user_first_name }} {{ line.user_last_name }} ({{ line.user_external_id }})</td>
Owner

Ok, juste une question de forme, je pensais qu’on avait pris la peine de définir une


@property
def user_name(self):

justement pour ne pas avoir à taper des modifications dans ce genre de gabarits malgré la modification du modèle.

Ok, juste une question de forme, je pensais qu’on avait pris la peine de définir une <pre><code class="python"> @property def user_name(self): </code></pre> justement pour ne pas avoir à taper des modifications dans ce genre de gabarits malgré la modification du modèle.
Author
Owner

En effet, mais pour cette vue j'ai écrit une union de querysets srur deux models différents (InvoiceLine et InjectedLine), avec des annotations pour compléter les champs manquants dans l'un des models, et un values pour que ça fonctionne. Donc pas possible d'utiliser les méthodes des models dans le template.

Mais je vais tricher et ajouter une ligne dans la méthode get_context_data de la vue, pour utiliser cette property (comme j'ai déjà triché pour d'autres trucs)

En effet, mais pour cette vue j'ai écrit une union de querysets srur deux models différents (InvoiceLine et InjectedLine), avec des annotations pour compléter les champs manquants dans l'un des models, et un values pour que ça fonctionne. Donc pas possible d'utiliser les méthodes des models dans le template. Mais je vais tricher et ajouter une ligne dans la méthode `get_context_data` de la vue, pour utiliser cette property (comme j'ai déjà triché pour d'autres trucs)
@ -64,6 +71,17 @@ def get_invoice_lines_for_user(agendas, agendas_pricings, user_external_id, user
return agenda_pricing
raise AgendaPricingNotFound
def get_payer_data(request, payer_external_id, agenda_pricing=None, payer_data=None):
Owner

Ok, juste une question de forme ici j’aurais bien vu un def get_cached_payer_data comme nom de fonction, pour que quelqu’un qui (comme moi) ne connaît pas du tout le code comprenne plus directement, à la lecture du code qui l’utilise plus loin, l’appel à première vue un peu mystérieux :

payer_data = get_payer_data(…, payer_data=payer_data)
Ok, juste une question de forme ici j’aurais bien vu un `def get_cached_payer_data` comme nom de fonction, pour que quelqu’un qui (comme moi) ne connaît pas du tout le code comprenne plus directement, à la lecture du code qui l’utilise plus loin, l’appel à première vue un peu mystérieux : <pre> payer_data = get_payer_data(…, payer_data=payer_data) </pre>
Author
Owner

fait, merci

fait, merci
@ -224,0 +273,4 @@
try:
value = Template(tplt).render(context)
if not value:
if key not in bool_keys:
Owner

Je ne comprends pas ici en quoi le fait d’avoir une clé associée à une valeur autre que booléenne fait qu’on interdit un résultat vide :/

Je ne comprends pas ici en quoi le fait d’avoir une clé associée à une valeur autre que booléenne fait qu’on interdit un résultat vide :/
Author
Owner

Les champs payer de type bool (demat, direct_debit) ont une valeur par défaut, "False".
Les autres champs n'ont pas de valeur par défaut (nom, prénom, impossible de définir une valeur par défaut), donc on ne veut pas de résultat vide.
C'est un peu arbitraire, je me suis dit qu'on pouvait ne pas définir de template pour les champs bool et ça passe quand même, alors que pour les autres champs c'est plus embêtant pour la suite (édition d'une facture etc) d'avoir une valeur vide.
Peut-être que je me suis embêtée pour rien et qu'on pourrait péter une erreur pour les bools aussi.
On verra à l'usage :)

Les champs payer de type bool (demat, direct_debit) ont une valeur par défaut, "False". Les autres champs n'ont pas de valeur par défaut (nom, prénom, impossible de définir une valeur par défaut), donc on ne veut pas de résultat vide. C'est un peu arbitraire, je me suis dit qu'on pouvait ne pas définir de template pour les champs bool et ça passe quand même, alors que pour les autres champs c'est plus embêtant pour la suite (édition d'une facture etc) d'avoir une valeur vide. Peut-être que je me suis embêtée pour rien et qu'on pourrait péter une erreur pour les bools aussi. On verra à l'usage :)
@ -39,0 +39,4 @@
<dl>
{% for key in object.get_extra_variables_keys %}
<dt><b>{% blocktrans %}{{ key }}:{% endblocktrans %}</b></dt>
<dd><pre>{{ object.extra_variables|get:key }}</pre></dd>
Owner

Là juste question d’affichage, je me dis qu’avec <pre/> il peut y avoir des soucis d’affichage si la valeur est trop longue et dépasse la largeur allouée dans le conteneur. Je ne sais si ça peut arriver, mais j’imagine qu’on peut gérer cela par la suite, dans une autre PR, si ça survenait.

Là juste question d’affichage, je me dis qu’avec `<pre/>` il peut y avoir des soucis d’affichage si la valeur est trop longue et dépasse la largeur allouée dans le conteneur. Je ne sais si ça peut arriver, mais j’imagine qu’on peut gérer cela par la suite, dans une autre PR, si ça survenait.
Author
Owner

Effectivement ça dépasse facilement, en local j'ai un template minimal pour aller chercher un champ dans une fiche et ça dépasse de l'écran.
Mais j'ai voulu faire un premier truc rapide et pas forcément joli, pour avancer; on pourra faire un ticket joliesse pour améliorer tout ça.

Effectivement ça dépasse facilement, en local j'ai un template minimal pour aller chercher un champ dans une fiche et ça dépasse de l'écran. Mais j'ai voulu faire un premier truc rapide et pas forcément joli, pour avancer; on pourra faire un ticket joliesse pour améliorer tout ça.
@ -0,0 +3,4 @@
{% block breadcrumb %}
{{ block.super }}
<a href="{% url 'lingo-manager-pricing-payer-edit' object.pk %}">{% trans "Payer ariables definition" %}</a>
Owner

Petite typo ici dans la chaîne internationalisée (ariables -> variables).

Petite typo ici dans la chaîne internationalisée (ariables -> variables).
Author
Owner

corrigé

corrigé
@ -0,0 +15,4 @@
{% csrf_token %}
{{ form.management_form }}
<table>
<tbody>
Owner

J’avoue que là je n’ai pas essayé de checkout la branche pour voir l’affichage du formset en tableau avec un formulaire par ligne comme ça. Totale confiance en l’auteure de la PR là-dessus :)

J’avoue que là je n’ai pas essayé de checkout la branche pour voir l’affichage du formset en tableau avec un formulaire par ligne comme ça. Totale confiance en l’auteure de la PR là-dessus :)
Author
Owner

copier/coller d'un autre template déjà existant, et on doit avoir des trucs similaires dans combo et passerelle :)

copier/coller d'un autre template déjà existant, et on doit avoir des trucs similaires dans combo et passerelle :)
lguerin added 8 commits 2023-03-16 09:25:11 +01:00
lguerin requested review from pmarillonnet 2023-03-16 09:27:28 +01:00
pmarillonnet approved these changes 2023-03-16 10:54:36 +01:00
pmarillonnet left a comment
Owner

Merci pour les réponses sur les choses qui m’échappaient encore, et pour les modifs demandées. Le reste est clair pour moi. Ack.

Merci pour les réponses sur les choses qui m’échappaient encore, et pour les modifs demandées. Le reste est clair pour moi. Ack.
lguerin merged commit 869cc6ca22 into main 2023-03-16 10:55:43 +01:00
lguerin deleted branch wip/74498-invoicing-payer-id 2023-03-16 10:55:43 +01:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/lingo#31
No description provided.