accrual: Make accruals consistent by entity on the accrual side.

It is more common than I realized that we split an invoice by
entity on the accrual side, so this supports that better.

It still disregards inconsistency between accrual entity and payment entity
for reporting purposes, to help keep reporting clean around automatic
imports.

The changes to BaseReport._report shook out because at this point, the group
key is effectively arbitrary and shouldn't be used for any reporting
purposes.

conservancy_beancount/reports/accrual.py

@@ -246,20 +246,26 @@ class AccrualPostings(core.RelatedPostings):
 
     def make_consistent(self) -> Iterator[Tuple[MetaValue, 'AccrualPostings']]:
         account_ok = isinstance(self.account, str)
+        if len(self.accrued_entities) == 1:
+            entity = next(iter(self.accrued_entities))
+        else:
+            entity = None
         # `'/' 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:
+        if account_ok and entity is not None and invoice_ok:
             yield (self.invoice, self)
             return
         groups = collections.defaultdict(list)
         for post in self:
-            if invoice_ok:
+            post_invoice = self.invoice if invoice_ok else (
-                key = f'{self.invoice} {post.account}'
+                post.meta.get('invoice') or 'BlankInvoice'
-            else:
+            )
-                key = f'{post.account} {post.meta.get("entity")} {post.meta.get("invoice")}'
+            post_entity = entity if entity is not None else (
-            groups[key].append(post)
+                post.meta.get('entity') or 'BlankEntity'
+            )
+            groups[f'{post.account} {post_invoice} {post_entity}'].append(post)
         type_self = type(self)
         for group_key, posts in groups.items():
             yield group_key, type_self(posts, _can_own=True)
@@ -314,16 +320,12 @@ class BaseReport:
         self.out_file = out_file
         self.logger = logger.getChild(type(self).__name__)
 
-    def _report(self,
+    def _report(self, posts: AccrualPostings, index: int) -> Iterable[str]:
-                invoice: str,
-                posts: AccrualPostings,
-                index: int,
-    ) -> Iterable[str]:
         raise NotImplementedError("BaseReport._report")
 
     def run(self, groups: PostGroups) -> None:
         for index, invoice in enumerate(groups):
-            for line in self._report(str(invoice), groups[invoice], index):
+            for line in self._report(groups[invoice], index):
                 print(line, file=self.out_file)
 
 
@@ -490,16 +492,12 @@ class AgingReport(BaseReport):
 
 
 class BalanceReport(BaseReport):
-    def _report(self,
+    def _report(self, posts: AccrualPostings, index: int) -> Iterable[str]:
-                invoice: str,
-                posts: AccrualPostings,
-                index: int,
-    ) -> Iterable[str]:
         posts = posts.since_last_nonzero()
         date_s = posts[0].meta.date.strftime('%Y-%m-%d')
         if index:
             yield ""
-        yield f"{invoice}:"
+        yield f"{posts.invoice}:"
         yield f"  {posts.balance_at_cost()} outstanding since {date_s}"
 
 
@@ -520,11 +518,7 @@ class OutgoingReport(BaseReport):
         else:
             return parsed
 
-    def _report(self,
+    def _report(self, posts: AccrualPostings, index: int) -> Iterable[str]:
-                invoice: str,
-                posts: AccrualPostings,
-                index: int,
-    ) -> Iterable[str]:
         posts = posts.since_last_nonzero()
         try:
             ticket_id, _ = self._primary_rt_id(posts)
@@ -537,7 +531,7 @@ class OutgoingReport(BaseReport):
         if ticket is None:
             self.logger.error(
                 "can't generate outgoings report for %s because no RT ticket available: %s",
-                invoice, errmsg,
+                posts.invoice, errmsg,
             )
             return
 

setup.py

@@ -5,7 +5,7 @@ from setuptools import setup
 setup(
     name='conservancy_beancount',
     description="Plugin, library, and reports for reading Conservancy's books",
-    version='1.1.2',
+    version='1.1.3',
     author='Software Freedom Conservancy',
     author_email='info@sfconservancy.org',
     license='GNU AGPLv3+',

tests/test_reports_accrual.py

@@ -20,6 +20,7 @@ import datetime
 import io
 import itertools
 import logging
+import operator
 import re
 
 import babel.numbers
@@ -478,18 +479,21 @@ def test_consistency_check_cost():
         assert err.source.get('lineno') == post.meta['lineno']
 
 def test_make_consistent_not_needed():
-    invoice = 'Invoices/ConsistentDoc.pdf'
+    main_meta = {
+        'entity': 'ConsistentTest',
+        '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], 20, {**main_meta, **other_meta}),
-        (ACCOUNTS[0], 25, {'invoice': invoice}),
+        (ACCOUNTS[0], 25, {**main_meta}),
     ])
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     consistent = related.make_consistent()
     actual_key, actual_postings = next(consistent)
-    assert actual_key == invoice
+    assert actual_key == main_meta['invoice']
     assert actual_postings is related
     assert next(consistent, None) is None
 
@@ -500,13 +504,17 @@ def test_make_consistent_not_needed():
 ))
 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})
+        (acct_name, index * 10, {'invoice': invoice, 'entity': f'BadInvoice{day}'})
         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}')
+    key = next(iter(consistent))
+    assert acct_name in key
+    if invoice:
+        assert str(invoice) in key
+    actual = consistent[key]
     assert actual
     assert len(actual) == 3
     for act_post, exp_post in zip(actual, txn.postings):
@@ -516,16 +524,15 @@ def test_make_consistent_bad_invoice(acct_name, invoice, day):
 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})
+        (acct_name, 100, {'invoice': invoice, 'entity': 'CrossAccount'})
         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:
+    for key, posts in consistent.items():
-        actual = consistent[f'{invoice} {acct_name}']
+        assert len(posts) == 1
-        assert len(actual) == 1
+        assert posts.account in key
-        assert actual[0].account == acct_name
 
 def test_make_consistent_both_invoice_and_account():
     txn = testutil.Transaction(date=datetime.date(2019, 2, 2), postings=[
@@ -534,10 +541,39 @@ def test_make_consistent_both_invoice_and_account():
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     consistent = dict(related.make_consistent())
     assert len(consistent) == len(ACCOUNTS)
-    for acct_name in ACCOUNTS:
+    for key, posts in consistent.items():
-        actual = consistent[f'{acct_name} None None']
+        assert len(posts) == 1
-        assert len(actual) == 1
+        assert posts.account in key
-        assert actual[0].account == acct_name
+
+@pytest.mark.parametrize('acct_name', ACCOUNTS)
+def test_make_consistent_across_entity(acct_name):
+    amt_sign = operator.pos if acct_name.startswith('Assets') else operator.neg
+    txn = testutil.Transaction(postings=[
+        (acct_name, amt_sign(n), {'invoice': 'Inv/1.pdf', 'entity': f'Entity{n}'})
+        for n in range(1, 4)
+    ])
+    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    consistent = dict(related.make_consistent())
+    assert len(consistent) == 3
+    for key, posts in consistent.items():
+        assert len(posts) == 1
+        assert len(posts.accrued_entities) == 1
+        assert next(posts.entities()) in key
+
+@pytest.mark.parametrize('acct_name', ACCOUNTS)
+def test_make_consistent_entity_differs_accrual_payment(acct_name):
+    invoice = 'Invoices/DifferPay.pdf'
+    txn = testutil.Transaction(postings=[
+        # Depending on the account, the order of the accrual and payment might
+        # be swapped here, but that shouldn't matter.
+        (acct_name, 125, {'invoice': invoice, 'entity': 'Positive'}),
+        (acct_name, -125, {'invoice': invoice, 'entity': 'Negative'}),
+    ])
+    related = accrual.AccrualPostings(data.Posting.from_txn(txn))
+    consistent = related.make_consistent()
+    _, actual = next(consistent)
+    assert actual is related
+    assert next(consistent, None) is None
 
 def check_output(output, expect_patterns):
     output.seek(0)
@@ -659,6 +695,17 @@ def test_aging_report_date_cutoffs(accrual_postings, date, recv_end, pay_end):
     output = run_aging_report(accrual_postings, date)
     check_aging_ods(output, date, expect_recv, expect_pay)
 
+def test_aging_report_entity_consistency(accrual_postings):
+    output = run_aging_report((
+        post for post in accrual_postings
+        if post.meta.get('rt-id') == 'rt:480'
+        and post.units.number < 0
+    ))
+    check_aging_ods(output, None, [], [
+        AgingRow.make_simple('2010-04-15', 'MultiPartyA', 125, 'rt:480/4800'),
+        AgingRow.make_simple('2010-04-15', 'MultiPartyB', 125, 'rt:480/4800'),
+    ])
+
 def run_main(arglist, config=None):
     if config is None:
         config = testutil.TestConfig(