diff --git a/conservancy_beancount/reports/accrual.py b/conservancy_beancount/reports/accrual.py index 584bc42ac704dfb272c81f23be2fae297534bbdb..cc705fae12307283f690d7307ed886bdfc6abb82 100644 --- a/conservancy_beancount/reports/accrual.py +++ b/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: - key = f'{self.invoice} {post.account}' - else: - key = f'{post.account} {post.meta.get("entity")} {post.meta.get("invoice")}' - groups[key].append(post) + post_invoice = self.invoice if invoice_ok else ( + post.meta.get('invoice') or 'BlankInvoice' + ) + post_entity = entity if entity is not None else ( + 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, - invoice: str, - posts: AccrualPostings, - index: int, - ) -> Iterable[str]: + def _report(self, 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, - invoice: str, - posts: AccrualPostings, - index: int, - ) -> Iterable[str]: + def _report(self, 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, - invoice: str, - posts: AccrualPostings, - index: int, - ) -> Iterable[str]: + def _report(self, 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 diff --git a/setup.py b/setup.py index 0afb6bb41251d9a3999c52a85f70d95e68f739f6..d8e2c8e3c19f0b812dfe01e9f5ab01aefce976b8 100755 --- a/setup.py +++ b/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+', diff --git a/tests/test_reports_accrual.py b/tests/test_reports_accrual.py index 3a16dfc3105b9f5fb9a3bdca5dca02bcd4a71da2..9b14372a419c3c84d05cf26b00427e12965c73cd 100644 --- a/tests/test_reports_accrual.py +++ b/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], 25, {'invoice': invoice}), + (ACCOUNTS[0], 20, {**main_meta, **other_meta}), + (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: - actual = consistent[f'{invoice} {acct_name}'] - assert len(actual) == 1 - assert actual[0].account == acct_name + for key, posts in consistent.items(): + assert len(posts) == 1 + assert posts.account in key 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: - actual = consistent[f'{acct_name} None None'] - assert len(actual) == 1 - assert actual[0].account == acct_name + for key, posts in consistent.items(): + assert len(posts) == 1 + assert posts.account in key + +@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(