From 7301bfc099a07dc8f9405ab3950eab4c28331f9e 2020-06-03 22:51:44
From: Brett Smith <brettcsmith@brettcsmith.org>
Date: 2020-06-03 22:51:44
Subject: [PATCH] accrual: Add AccrualPostings.make_consistent() method.

This will help the aging report better render dirty data.

---

diff --git a/conservancy_beancount/reports/accrual.py b/conservancy_beancount/reports/accrual.py
index f80de7c48cc4631fa4b7b87d53822da7b36a26d7..9b8255fbf8d48abe782117bbb85175cfc818c39d 100644
--- a/conservancy_beancount/reports/accrual.py
+++ b/conservancy_beancount/reports/accrual.py
@@ -157,7 +157,6 @@ class AccrualPostings(core.RelatedPostings):
         'invoice': _meta_getter('invoice'),
         'purchase_order': _meta_getter('purchase-order'),
     }
-    _INVOICE_COUNTER: Dict[str, int] = collections.defaultdict(int)
     INCONSISTENT = Sentinel()
     __slots__ = (
         'accrual_type',
@@ -202,6 +201,26 @@ class AccrualPostings(core.RelatedPostings):
         else:
             self.accrual_type = AccrualAccount.classify(self)
 
+    def make_consistent(self) -> Iterator[Tuple[MetaValue, 'AccrualPostings']]:
+        account_ok = isinstance(self.account, str)
+        # `'/' in self.invoice` is just our heuristic to ensure that the
+        # invoice metadata is "unique enough," and not just a placeholder
+        # value like "FIXME". It can be refined if needed.
+        invoice_ok = isinstance(self.invoice, str) and '/' in self.invoice
+        if account_ok and invoice_ok:
+            yield (self.invoice, self)
+            return
+        groups = collections.defaultdict(list)
+        for post in self:
+            if invoice_ok:
+                key = f'{self.invoice} {post.account}'
+            else:
+                key = f'{post.account} {post.meta.get("entity")} {post.meta.get("invoice")}'
+            groups[key].append(post)
+        type_self = type(self)
+        for group_key, posts in groups.items():
+            yield group_key, type_self(posts, _can_own=True)
+
     def report_inconsistencies(self) -> Iterable[Error]:
         for field_name, get_func in self._FIELDS.items():
             if getattr(self, field_name) is self.INCONSISTENT:
diff --git a/tests/test_reports_accrual.py b/tests/test_reports_accrual.py
index 8dd22f45fa960c95278136f98740f1c62de71a24..067fa393bcd60e80e7e7cef10eaa81cf961ec18a 100644
--- a/tests/test_reports_accrual.py
+++ b/tests/test_reports_accrual.py
@@ -16,6 +16,7 @@
 
 import collections
 import copy
+import datetime
 import io
 import itertools
 import logging
@@ -349,6 +350,68 @@ def test_consistency_check_cost():
         assert err.entry is txn
         assert err.source.get('lineno') == post.meta['lineno']
 
+def test_make_consistent_not_needed():
+    invoice = 'Invoices/ConsistentDoc.pdf'
+    other_meta = {key: f'{key}.pdf' for key in CONSISTENT_METADATA}
+    # We intentionally make inconsistencies in "minor" metadata that shouldn't
+    # split out the group.
+    txn = testutil.Transaction(postings=[
+        (ACCOUNTS[0], 20, {**other_meta, 'invoice': invoice}),
+        (ACCOUNTS[0], 25, {'invoice': invoice}),
+    ])
+    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    consistent = related.make_consistent()
+    actual_key, actual_postings = next(consistent)
+    assert actual_key == invoice
+    assert actual_postings is related
+    assert next(consistent, None) is None
+
+@pytest.mark.parametrize('acct_name,invoice,day', testutil.combine_values(
+    ACCOUNTS,
+    ['FIXME', '', None, *testutil.NON_STRING_METADATA_VALUES],
+    itertools.count(1),
+))
+def test_make_consistent_bad_invoice(acct_name, invoice, day):
+    txn = testutil.Transaction(date=datetime.date(2019, 1, day), postings=[
+        (acct_name, index * 10, {'invoice': invoice})
+        for index in range(1, 4)
+    ])
+    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    consistent = dict(related.make_consistent())
+    assert len(consistent) == 1
+    actual = consistent.get(f'{acct_name} None {invoice}')
+    assert actual
+    assert len(actual) == 3
+    for act_post, exp_post in zip(actual, txn.postings):
+        assert act_post.units == exp_post.units
+        assert act_post.meta.get('invoice') == invoice
+
+def test_make_consistent_across_accounts():
+    invoice = 'Invoices/CrossAccount.pdf'
+    txn = testutil.Transaction(date=datetime.date(2019, 2, 1), postings=[
+        (acct_name, 100, {'invoice': invoice})
+        for acct_name in ACCOUNTS
+    ])
+    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    consistent = dict(related.make_consistent())
+    assert len(consistent) == len(ACCOUNTS)
+    for acct_name in ACCOUNTS:
+        actual = consistent[f'{invoice} {acct_name}']
+        assert len(actual) == 1
+        assert actual[0].account == acct_name
+
+def test_make_consistent_both_invoice_and_account():
+    txn = testutil.Transaction(date=datetime.date(2019, 2, 2), postings=[
+        (acct_name, 150) for acct_name in ACCOUNTS
+    ])
+    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    consistent = dict(related.make_consistent())
+    assert len(consistent) == len(ACCOUNTS)
+    for acct_name in ACCOUNTS:
+        actual = consistent[f'{acct_name} None None']
+        assert len(actual) == 1
+        assert actual[0].account == acct_name
+
 def check_output(output, expect_patterns):
     output.seek(0)
     testutil.check_lines_match(iter(output), expect_patterns)