From 30d4362c09726ea5351fe01f079f2e6f69d75381 Mon Sep 17 00:00:00 2001 From: laurentbarontini Date: Tue, 28 Apr 2026 19:23:42 +0200 Subject: [PATCH] Check From To + pricing component domain filter --- AGENTS.md | 9 +- modules/purchase_trade/docs/business-rules.md | 37 ++++++ modules/purchase_trade/pricing.py | 38 +++++- modules/purchase_trade/purchase.py | 24 ++-- modules/purchase_trade/sale.py | 12 +- modules/purchase_trade/tests/test_module.py | 56 +++++++++ notes/business_rules.md | 112 ++++++++++++++++++ 7 files changed, 276 insertions(+), 12 deletions(-) create mode 100644 notes/business_rules.md diff --git a/AGENTS.md b/AGENTS.md index dc8c316..580f77a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,9 +63,14 @@ Guide rapide pour les agents qui codent dans ce repository. - Quand un template affiche les placeholders en brut, verifier que les champs sont bien des placeholders Relatorio dans le XML (pas du texte litteral). - Eviter les apostrophes echappees style `\'` dans placeholders; preferer `"` et `'`. -## 4.bis) Memo templates de session +## 4.bis) Memos metier et templates -- Voir aussi `notes/template_business_rules.md` pour le recap detaille (business rules + decisions templates de la session). +- Regles metier transverses: + - `notes/business_rules.md` +- Regles metier locales `purchase_trade`: + - `modules/purchase_trade/docs/business-rules.md` +- Decisions templates / reports: + - `notes/template_business_rules.md` ## 5) Workflow de modification (obligatoire) diff --git a/modules/purchase_trade/docs/business-rules.md b/modules/purchase_trade/docs/business-rules.md index 1cc1381..8811ec0 100644 --- a/modules/purchase_trade/docs/business-rules.md +++ b/modules/purchase_trade/docs/business-rules.md @@ -485,6 +485,43 @@ Owner technique: `a completer` - Priorite: - `importante` +### BR-PT-014 - Delivery period: From doit rester inferieur ou egal a To + +- Intent: eviter les periodes de livraison incoherentes sur les lignes achat et + vente. +- Description: + - Les champs `from_del` et `to_del` sont presents sur `purchase.line` et + `sale.line`. + - Si les deux dates sont renseignees, `from_del` ne doit jamais etre + posterieur a `to_del`. +- Resultat attendu: + - la sauvegarde d'une `purchase.line` ou `sale.line` est bloquee si + `from_del > to_del` + - une date ouverte reste autorisee si seulement une des deux bornes est + renseignee +- Priorite: + - `importante` + +### BR-PT-015 - Pricing manuel: composant limite a la ligne courante + +- Intent: eviter qu'une ligne de pricing saisie manuellement utilise un + composant rattache a une autre ligne de contrat. +- Description: + - Dans l'onglet `Pricing dates` d'une `purchase.line`, le champ + `pricing.pricing.price_component` doit proposer uniquement les composants + dont `pricing.component.line` est la ligne achat courante. + - Dans l'onglet `Pricing dates` d'une `sale.line`, il doit proposer + uniquement les composants dont `pricing.component.sale_line` est la ligne + vente courante. + - Une ligne de pricing sans composant reste possible pour le mode manuel sans + component. +- Resultat attendu: + - le domaine UI filtre les composants sur la ligne courante + - une validation serveur bloque aussi un composant appartenant a une autre + ligne +- Priorite: + - `importante` + ## 4) Exemples concrets ### Exemple E1 - Augmentation simple diff --git a/modules/purchase_trade/pricing.py b/modules/purchase_trade/pricing.py index 9141c23..fb5cc27 100755 --- a/modules/purchase_trade/pricing.py +++ b/modules/purchase_trade/pricing.py @@ -3,7 +3,8 @@ from trytond.model import fields from trytond.pool import Pool, PoolMeta from trytond.pyson import Bool, Eval, Id -from trytond.model import (ModelSQL, ModelView) +from trytond.model import (ModelSQL, ModelView) +from trytond.exceptions import UserError from trytond.tools import is_full_text, lstrip_wildcard from trytond.transaction import Transaction, inactive_records from decimal import getcontext, Decimal, ROUND_HALF_UP @@ -330,7 +331,14 @@ class Pricing(ModelSQL,ModelView): __name__ = 'pricing.pricing' pricing_date = fields.Date("Date") - price_component = fields.Many2One('pricing.component', "Component")#, domain=[('id', 'in', Eval('line.price_components'))], ondelete='CASCADE') + price_component = fields.Many2One( + 'pricing.component', "Component", + domain=[ + 'OR', + ('line', '=', Eval('line', -1)), + ('sale_line', '=', Eval('sale_line', -1)), + ], + depends=['line', 'sale_line'])#, ondelete='CASCADE') quantity = fields.Numeric("Qt",digits='unit') settl_price = fields.Numeric("Settl. price",digits='unit') fixed_qt = fields.Numeric("Fixed qt",digits='unit', readonly=True) @@ -433,6 +441,32 @@ class Pricing(ModelSQL,ModelView): cls._sync_manual_last(records) cls._sync_eod_price(records) + @staticmethod + def _component_matches_owner(record): + def record_id(value): + return getattr(value, 'id', value) + + component = getattr(record, 'price_component', None) + if not component: + return True + line = getattr(record, 'line', None) + if line: + return record_id(getattr(component, 'line', None)) == record_id(line) + sale_line = getattr(record, 'sale_line', None) + if sale_line: + return ( + record_id(getattr(component, 'sale_line', None)) + == record_id(sale_line)) + return True + + @classmethod + def validate(cls, records): + super(Pricing, cls).validate(records) + for record in records: + if not cls._component_matches_owner(record): + raise UserError( + "Pricing component must belong to the related line.") + @classmethod def _sync_eod_price(cls, records): if not records: diff --git a/modules/purchase_trade/purchase.py b/modules/purchase_trade/purchase.py index 363d0e7..77bf92c 100755 --- a/modules/purchase_trade/purchase.py +++ b/modules/purchase_trade/purchase.py @@ -1094,6 +1094,13 @@ class Line(metaclass=PoolMeta): return configurations[0].pricing_rule or '' return '' + @staticmethod + def _has_invalid_delivery_period(line): + return ( + bool(line.from_del) + and bool(line.to_del) + and line.from_del > line.to_del) + quantity_theorical = fields.Numeric("Contractual Qt", digits='unit', readonly=False) price_type = fields.Selection([ ('cash', 'Cash Price'), @@ -1558,13 +1565,16 @@ class Line(metaclass=PoolMeta): # OpenPosition.delete(op) super(Line, cls).delete(lines) - @classmethod - def validate(cls, lines): - super(Line, cls).validate(lines) - for line in lines: - if line.price_components: - for pc in line.price_components: - if pc.triggers: + @classmethod + def validate(cls, lines): + super(Line, cls).validate(lines) + for line in lines: + if cls._has_invalid_delivery_period(line): + raise UserError( + "Delivery period From date must be before To date.") + if line.price_components: + for pc in line.price_components: + if pc.triggers: for tr in pc.triggers: line.check_from_to(tr) line.check_pricing() diff --git a/modules/purchase_trade/sale.py b/modules/purchase_trade/sale.py index 1ec5447..9a443e9 100755 --- a/modules/purchase_trade/sale.py +++ b/modules/purchase_trade/sale.py @@ -1077,6 +1077,13 @@ class SaleLine(metaclass=PoolMeta): return configurations[0].pricing_rule or '' return '' + @staticmethod + def _has_invalid_delivery_period(line): + return ( + bool(line.from_del) + and bool(line.to_del) + and line.from_del > line.to_del) + del_period = fields.Many2One('product.month',"Delivery Period") lots = fields.One2Many('lot.lot','sale_line',"Lots",readonly=True) fees = fields.One2Many('fee.fee', 'sale_line', 'Fees') @@ -1676,13 +1683,16 @@ class SaleLine(metaclass=PoolMeta): default.setdefault('price_pricing', None) return super().copy(lines, default=default) - @classmethod + @classmethod def validate(cls, salelines): LotQtHist = Pool().get('lot.qt.hist') LotQtType = Pool().get('lot.qt.type') Pnl = Pool().get('valuation.valuation') super(SaleLine, cls).validate(salelines) for line in salelines: + if cls._has_invalid_delivery_period(line): + raise UserError( + "Delivery period From date must be before To date.") if line.price_components: for pc in line.price_components: if pc.triggers: diff --git a/modules/purchase_trade/tests/test_module.py b/modules/purchase_trade/tests/test_module.py index d145439..fef9330 100644 --- a/modules/purchase_trade/tests/test_module.py +++ b/modules/purchase_trade/tests/test_module.py @@ -6,6 +6,7 @@ from decimal import Decimal from unittest.mock import Mock, patch from trytond.pool import Pool +from trytond.pyson import Eval from trytond.tests.test_tryton import ModuleTestCase, with_transaction from trytond.exceptions import UserError from trytond.modules.purchase_trade import valuation as valuation_module @@ -523,6 +524,61 @@ class PurchaseTradeTestCase(ModuleTestCase): self.assertTrue(Pricing.unfixed_qt_price.readonly) self.assertTrue(Pricing.eod_price.readonly) + def test_pricing_component_domain_is_limited_to_owner_line(self): + 'pricing component choices are limited to the purchase or sale line' + Pricing = Pool().get('pricing.pricing') + + self.assertEqual(Pricing.price_component.domain, [ + 'OR', + ('line', '=', Eval('line', -1)), + ('sale_line', '=', Eval('sale_line', -1)), + ]) + + def test_pricing_component_must_belong_to_pricing_owner(self): + 'pricing rows reject components from another purchase or sale line' + Pricing = Pool().get('pricing.pricing') + owner = Mock(id=10) + other = Mock(id=11) + + self.assertTrue(Pricing._component_matches_owner(Mock( + line=owner, + sale_line=None, + price_component=Mock(line=owner, sale_line=None), + ))) + self.assertFalse(Pricing._component_matches_owner(Mock( + line=owner, + sale_line=None, + price_component=Mock(line=other, sale_line=None), + ))) + self.assertTrue(Pricing._component_matches_owner(Mock( + line=None, + sale_line=owner, + price_component=Mock(line=None, sale_line=owner), + ))) + self.assertFalse(Pricing._component_matches_owner(Mock( + line=None, + sale_line=owner, + price_component=Mock(line=None, sale_line=other), + ))) + + def test_purchase_and_sale_delivery_period_reject_from_after_to(self): + 'delivery period dates must be chronological on purchase and sale lines' + PurchaseLine = Pool().get('purchase.line') + SaleLine = Pool().get('sale.line') + invalid = Mock( + from_del=datetime.date(2026, 5, 1), + to_del=datetime.date(2026, 4, 30), + ) + valid = Mock( + from_del=datetime.date(2026, 4, 1), + to_del=datetime.date(2026, 4, 30), + ) + + self.assertTrue(PurchaseLine._has_invalid_delivery_period(invalid)) + self.assertTrue(SaleLine._has_invalid_delivery_period(invalid)) + self.assertFalse(PurchaseLine._has_invalid_delivery_period(valid)) + self.assertFalse(SaleLine._has_invalid_delivery_period(valid)) + def test_pricing_eod_uses_weighted_average_for_manual_rows(self): 'manual pricing eod uses the weighted average of fixed and unfixed legs' Pricing = Pool().get('pricing.pricing') diff --git a/notes/business_rules.md b/notes/business_rules.md new file mode 100644 index 0000000..1f22e89 --- /dev/null +++ b/notes/business_rules.md @@ -0,0 +1,112 @@ +# Business rules + +Memo racine pour les regles metier transverses ou multi-modules. + +Pour les regles propres a un module, preferer la documentation locale quand +elle existe, par exemple: + +- `modules/purchase_trade/docs/business-rules.md` +- `modules/purchase_trade/docs/padding-invoice-accounting.md` + +## Session 2026-04-28 - Devises, TVA, padding et lots + +### `account.move.line` / saisie manuelle en seconde devise + +- `rate` est un vrai champ stocke et editable, plus seulement un champ de + visualisation. +- Le sens du taux suit l'usage Tryton: taux devise seconde / devise societe. + Donc, avec une devise societe a 1, le montant societe se calcule par + `amount_second_currency / rate`. +- Une ligne est consideree comme manuelle si elle n'a ni `origin`, ni + `move_origin`, ni `move.origin`. +- En manuel, si l'utilisateur saisit d'abord `amount_second_currency`, le + systeme reprend le dernier taux disponible pour la devise seconde a la date + de la ligne, puis calcule `debit` ou `credit`. +- Si l'utilisateur veut forcer le taux, il efface d'abord `debit` / `credit`, + saisit `rate`, et le montant societe est recalcule depuis le montant en + devise seconde. +- Si le montant societe et le montant devise seconde sont deja presents, le + taux implicite est `abs(amount_second_currency) / abs(base_amount)`. +- Les lignes generees automatiquement depuis facture doivent aussi renseigner + `rate` quand `amount_second_currency` existe. +- Un fallback central sur `account.move.line.create` calcule le taux manquant + depuis les montants pour eviter les chemins oublies. +- Comme `rate` devient stocke, une mise a jour du module est necessaire; les + anciennes lignes doivent etre regenerees ou backfillees si on veut afficher + le taux historique. + +### Causes racines des bugs devises + +- Deux `on_change_amount_second_currency` existaient; le second ecrasait la + logique de conversion. +- La branche negative testait deux fois `> 0`, donc un montant devise seconde + negatif ne generait pas correctement le credit. +- Le taux initial etait calcule a l'envers par rapport au standard Tryton + attendu. +- Les records transitoires Tryton peuvent ne pas exposer un champ non assigne; + les chemins generiques doivent utiliser `getattr(record, field, None)`. + +### `account.invoice` / taux et devise + +- `on_change_with_rate` depend explicitement de `currency`, `company`, + `invoice_date`, `selection_rate`, `rate_date` et `lines`. +- Ne pas declarer `lines.amount` dans `@fields.depends`: ce champ relationnel + n'existe pas cote definition de vue et provoque un `KeyError`. +- Les move lines de facture, lignes produit, lignes stock/drop et lignes TVA + doivent renseigner `rate` des qu'elles portent `amount_second_currency`. + +### TVA + +- Les taxes sur `invoice.lines[].taxes` alimentent les taxes calculees de la + facture via les lignes taxables. +- Les taxes globales de facture (`invoice.taxes`) peuvent etre automatiques ou + manuelles. +- Les taxes manuelles sont preservees par `update_taxes`; les taxes + automatiques sont synchronisees, supprimees ou recreees selon les lignes. +- Une ligne facture genere des `account.tax.line` de type `base`. +- Une ligne de taxe facture genere une move line comptable avec + `account.tax.line` de type `tax`; si la taxe est manuelle, une ligne `base` + complementaire peut etre ajoutee. +- La seconde devise est portee par `account.move.line` + (`second_currency` / `amount_second_currency`), pas par `account.tax.line` + qui suit la devise comptable de la move line. + +### Padding + +- Les chemins padding doivent verifier que `lot` existe avant de lire + `sale_invoice_line_prov` ou `sale_invoice_line`. +- Le bug observe n'etait pas la logique padding elle-meme mais un acces direct + a un lot absent sur certaines lignes de facture. + +### Payment term + +- Le calcul des echeances doit fonctionner meme si la premiere ligne facture + n'a pas d'`origin`. +- `term_lines` doit etre calcule hors de la branche qui depend du modele + d'origine, avec une ligne metier optionnelle. + +### Periodes comptables + +- `Period.find` utilise un cache par societe/date/etat. +- Une periode creee ou ouverte peut continuer a etre vue comme absente par un + worker tant que le cache ou le serveur n'a pas ete rafraichi. + +### `lot.qt` / validation sale et purchase + +- Cote `sale.line`, la creation de la ligne cree deja le lot virtuel et sa + quantite. +- Le passage initial de `quantity_theorical` de `None` a la quantite de la + ligne ne doit pas etre traite comme un delta physique; sinon `lot.qt` est + double. +- Le correctif sale ignore le delta quand l'ancienne valeur theorique est + `None`; les modifications suivantes restent bien appliquees en delta. +- Cote `purchase.line`, la logique etait deja protegee par une baseline sur la + quantite courante du lot virtuel quand l'ancienne valeur est `None`. + +### Valuation / PNL achat + +- Une purchase line peut ne pas produire de valorisation si elle est finie, + sans lot, filtree par `valuation_type`, sans `lot_price` pour les modes + `priced` / `efp`, sans fees relies a des lots, ou sans derivatives. +- En mode `basis`, sans composant, le fallback metier continue de passer par la + ligne summary quand elle existe.