Facturation: ranger les campagnes de facturation dans une régie (#74516) #29
No reviewers
pmarillonnet
Labels
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: entrouvert/lingo#29
Loading…
Reference in New Issue
No description provided.
Delete Branch "wip/74516-invoicing-attach-campaign-to-regie"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
0001: ajout de la FK Regie sur le model Campaign, première migration pour virer toutes les données (pas de legacy à garder, on peut tout jeter), migration qui ajoute le champ, minimum de modif dans le code et les tests pour que le commit soit passant
0002: déplacement de juste la vue des éléments non facturés, parce que c'est la seule vue complètement indépendante des autres, donc facile
0003: déplacement de toutes les autres vues, d'un bloc, parce que héritage des templates etc tout dépend de tout
0004: simplification des filtres sur les pages de détail et journal d'un pool, pour supprimer le filtre régie
1f8bf02d99to7bc98843b2Je commence à relire, wish me luck.
Voilà quelques petits trucs attrapés au passage. Je me concentre sur la forme parce que je n’ai pas les billes pour comprendre la logique métier, partie sur laquelle ma relecture est tout à fait complaisante :)
@ -0,0 +1,28 @@from django.db import migrationsOk, je comprends la nécessité du nettoyage ici, mais n’y a-t-il pas des instances en recettes où des données ont déjà été créées à des fins de tests, par Stef ou quelqu’un d’autre, et qu’il faudrait sauvegarder dans un coin avant d’exécuter cette migration ?
non pas de données à conserver, et c'était écrit dans la description du ticket :)
Arf désolé j’ai loupé ça.
@ -0,0 +12,4 @@migrations.AddField(model_name='campaign',name='regie',field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='invoicing.Regie'),Ok, mais il faut alors dans la RegieDeleteView attraper les erreurs (
django.db.ProtectedError) levées à la tentative de suppression alors qu’il existe encore des Campaign liées. Je ne vois ça nulle part (et la DeleteView django de base héritée ne gère pas cela du tout), je loupe un truc ?Si c’est effectivement un oubli, sans doute qu’un petit test en plus serait le bienvenu.
Justement j'ai choisi de mettre PROTECT pour garantir que rien n'est supprimé si ce n'est pas explicitement géré par le code.
Un delete sauvage d'une régie n'est pas possible, l'orm vérifie les relations et balance une erreur avant de tenter la suppression
(l'orm ne délègue pas la gestion des FK et des relations à la DB, il le gère lui-même et se comporte en fonction du on_delete).
Les données de facturation étant sensibles, je préfère lever une erreur et ne pas laisser passer un delete irréfléchi.
(et une campagne n'a pas vocation à être supprimée, on doit conserver les données de facturation)
Ok, à dérouler le code de Lingo puis de la DeleteView de base Django, j’étais dans l’impression que la RegieDeleteView sur une régie qui est encore liée à des campagnes allait lancer une bête trace python au lieu d’une erreur applicative propre. Je loupe un truc ?
ha zut il y a une vue de suppression de régie ? bien vu :)
je m'en vais donc interdire la suppression de régie si campagne ou facture liées.
commit ajouté
¡Maravilloso!
@ -59,1 +61,4 @@<div aria-labelledby="tab-campaigns" hidden="" id="panel-campaigns" role="tabpanel" tabindex="0"><ul class="objects-list single-links">{% for campaign in campaigns %}Dans le gabarit initial que ce bout de gabarit remplace, il y avait un message lorsque cette liste d’objets était vide (“This site doesn't have any campaign yet. […]“). Est-ce volontaire de l’avoir laissé de coté dans cette nouvelle version ?
Dans le template précédent, on avait une liste de campagnes et c'est tout.
Là on est dans un onglet sur la page de détail d'une régie.
Sur la page de détail d'une campagne, dans l'onglet pool il n'y a pas on plus de message lorsque par de pools.
Si tu insistes je peux ajouter ça pour les campagnes et pour les pools (pour que ce soit homogène)
Non, je ne connais pas assez le métier pour insister, je signalais juste ce qui me semblait être une incohérence de forme, mais il s’avère que c’est délibéré. Ok pour moi.