python-drafthorse icon indicating copy to clipboard operation
python-drafthorse copied to clipboard

`MonetarySummation.tax_total` is parsed incorrectly

Open notEvil opened this issue 1 year ago • 2 comments

Hi,

I noticed that MonetarySummation.tax_total is parsed incorrectly due to MonetarySummation.tax_total_other_currency. They share the tag TaxTotalAmount and therefore the amount string gets split by

https://github.com/pretix/python-drafthorse/blob/5ed87a6f99919156609408b2ecf619f2c82e9b31/drafthorse/models/container.py#L60

For instance, <ram:TaxTotalAmount>0.00</ram:TaxTotalAmount> will end up as <ram:TaxTotalAmount currencyID=".">0</ram:TaxTotalAmount>.

Models: https://github.com/pretix/python-drafthorse/blob/5ed87a6f99919156609408b2ecf619f2c82e9b31/drafthorse/models/accounting.py#L148

notEvil avatar Jul 30 '24 10:07 notEvil

Uh, interesting! Do you have capacity to create a PR? We currently use drafthorse only for generation, not parsing, so it is not a priority for us, but we'd fix it at some point probably

raphaelm avatar Jul 30 '24 10:07 raphaelm

How about

diff --git a/drafthorse/models/container.py b/drafthorse/models/container.py
index 8219378..eb1755e 100644
--- a/drafthorse/models/container.py
+++ b/drafthorse/models/container.py
@@ -61,10 +61,7 @@ class CurrencyContainer(SimpleContainer):
         el._currency = child[1]
 
     def add_from_etree(self, root):
-        if root.attrib.get("currencyID"):
-            self.add((root.text, root.attrib["currencyID"]))
-        else:
-            self.add(root.text)
+        self.add((root.text, root.attrib.get("currencyID")))
 
 
 class IDContainer(SimpleContainer):

Looks consistent with child_type=(tuple, list), which btw isn't enforced since isinstance(self.child_type, type) is false.

notEvil avatar Jul 30 '24 12:07 notEvil