From a5b3dc146351644e7dd2ee9d4c76ada513bc55f2 Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Tue, 30 Jun 2020 17:28:08 -0400 Subject: [PATCH] accrual: AccrualPostings.make_consistent() groups accruals by date. This accommodates cases of contracts without separate invoices, where a series of payments are scheduled over time. The dance we used to do of group-by-invoice, then make consistent was already suspect. It was originally motivated by the consistency checks that are now gone. Use this opportunity to clean up and just make make_consistent a classmethod. --- conservancy_beancount/reports/accrual.py | 97 ++++++++----- conservancy_beancount/reports/core.py | 4 +- tests/books/accruals.beancount | 2 +- tests/test_reports_accrual.py | 172 +++++++++++++++++++---- 4 files changed, 213 insertions(+), 62 deletions(-) diff --git a/conservancy_beancount/reports/accrual.py b/conservancy_beancount/reports/accrual.py index 259ecae..ec25bf0 100644 --- a/conservancy_beancount/reports/accrual.py +++ b/conservancy_beancount/reports/accrual.py @@ -81,6 +81,9 @@ from typing import ( Any, BinaryIO, Callable, + Deque, + Dict, + Hashable, Iterable, Iterator, List, @@ -119,7 +122,7 @@ from .. import rtutil PROGNAME = 'accrual-report' -PostGroups = Mapping[Optional[str], 'AccrualPostings'] +PostGroups = Mapping[Optional[Hashable], 'AccrualPostings'] T = TypeVar('T') logger = logging.getLogger('conservancy_beancount.reports.accrual') @@ -166,6 +169,7 @@ class AccrualAccount(enum.Enum): class AccrualPostings(core.RelatedPostings): __slots__ = ( 'accrual_type', + 'date', 'end_balance', 'account', 'entity', @@ -183,12 +187,15 @@ class AccrualPostings(core.RelatedPostings): if isinstance(self.account, Sentinel): self.accrual_type: Optional[AccrualAccount] = None norm_func: Callable[[T], T] = lambda x: x - entity_pred: Callable[[data.Posting], bool] = bool + accrual_pred: Callable[[data.Posting], bool] = bool else: self.accrual_type = AccrualAccount.by_account(self.account) norm_func = self.accrual_type.normalize_amount - entity_pred = lambda post: norm_func(post.units).number > 0 - self.entity = self._single_item(self.entities(entity_pred)) + accrual_pred = lambda post: norm_func(post.units).number > 0 + if not any(accrual_pred(post) for post in self): + accrual_pred = bool + self.date = self._single_item(self.dates(accrual_pred)) + self.entity = self._single_item(self.entities(accrual_pred)) self.invoice = self._single_item(self.first_meta_links('invoice', None)) self.end_balance = norm_func(self.balance_at_cost()) @@ -202,6 +209,9 @@ class AccrualPostings(core.RelatedPostings): all_same = all(item == item1 for item in items) return item1 if all_same else self.INCONSISTENT + def dates(self, pred: Callable[[data.Posting], bool]=bool) -> Iterator[datetime.date]: + return filters.iter_unique(post.meta.date for post in self if pred(post)) + def entities(self, pred: Callable[[data.Posting], bool]=bool) -> Iterator[MetaValue]: return filters.iter_unique( post.meta['entity'] @@ -209,29 +219,52 @@ class AccrualPostings(core.RelatedPostings): if pred(post) and 'entity' in post.meta ) - def make_consistent(self) -> Iterator[Tuple[str, 'AccrualPostings']]: - account_ok = isinstance(self.account, str) - entity_ok = isinstance(self.entity, 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 entity_ok and invoice_ok: - # type ignore for - yield (self.invoice, self) # type:ignore[misc] - return - groups = collections.defaultdict(list) - for post in self: - post_invoice = self.invoice if invoice_ok else ( - post.meta.get('invoice') or 'BlankInvoice' - ) - post_entity = self.entity if entity_ok 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) + @classmethod + def make_consistent(cls, + postings: Iterable[data.Posting], + ) -> Iterator[Tuple[Hashable, 'AccrualPostings']]: + accruals: Dict[Tuple[str, ...], List[data.Posting]] = collections.defaultdict(list) + payments: Dict[Tuple[str, ...], Deque[data.Posting]] = collections.defaultdict(Deque) + key: Tuple[str, ...] + for post in postings: + norm_func = core.normalize_amount_func(post.account) + invoice = str(post.meta.get('invoice', 'BlankInvoice')) + if norm_func(post.units.number) >= 0: + entity = str(post.meta.get('entity', 'BlankEntity')) + key = (post.meta.date.isoformat(), entity, invoice, post.account) + accruals[key].append(post) + else: + key = (invoice, post.account) + payments[key].append(post) + + for key, acc_posts in accruals.items(): + pay_posts = payments[key[2:]] + if not pay_posts: + continue + norm_func = core.normalize_amount_func(key[-1]) + balance = norm_func(core.MutableBalance(post.at_cost() for post in acc_posts)) + while pay_posts and not balance.le_zero(): + pay_post = pay_posts.popleft() + acc_posts.append(pay_post) + balance += norm_func(pay_post.at_cost()) + if balance.le_zero() and not balance.is_zero(): + # pay_post causes the accrual to be overpaid. Split it into two + # synthesized postings: one that causes the accrual to be + # exactly zero, and one with the remainder back in payments. + post_cost = pay_post.at_cost() + # Calling norm_func() reverses the call in the while loop to add + # the amount to the balance. + overpayment = norm_func(balance[post_cost.currency]) + amt_to_zero = post_cost._replace(number=post_cost.number - overpayment.number) + acc_posts[-1] = pay_post._replace(units=amt_to_zero, cost=None, price=None) + pay_posts.appendleft(pay_post._replace(units=overpayment, cost=None, price=None)) + acc_posts.sort(key=lambda post: post.meta.date) + + for key, acc_posts in accruals.items(): + yield key, cls(acc_posts, _can_own=True) + for key, pay_posts in payments.items(): + if pay_posts: + yield key, cls(pay_posts, _can_own=True) def is_paid(self, default: Optional[bool]=None) -> Optional[bool]: if self.accrual_type is None: @@ -739,9 +772,7 @@ def main(arglist: Optional[Sequence[str]]=None, # but they do help symbolize what's being grouped. # For the outgoing approval report, groups maps rt-id link strings to # associated accruals. - # For all other reports, groups starts by grouping postings together by - # invoice link string, then uses AccrualReport.make_consistent() to split - # out groups that need it. + # For all other reports, groups comes from AccrualReport.make_consistent(). groups: PostGroups if args.report_type is None or args.report_type is ReportType.OUTGOING: groups = dict(AccrualPostings.group_by_first_meta_link(postings, 'rt-id')) @@ -754,11 +785,7 @@ def main(arglist: Optional[Sequence[str]]=None, ): args.report_type = ReportType.OUTGOING if args.report_type is not ReportType.OUTGOING: - groups = { - key: group - for _, source in AccrualPostings.group_by_first_meta_link(postings, 'invoice') - for key, group in source.make_consistent() - } + groups = dict(AccrualPostings.make_consistent(postings)) if args.report_type is not ReportType.AGING: groups = { key: posts for key, posts in groups.items() if not posts.is_paid() diff --git a/conservancy_beancount/reports/core.py b/conservancy_beancount/reports/core.py index 0209fcf..2767645 100644 --- a/conservancy_beancount/reports/core.py +++ b/conservancy_beancount/reports/core.py @@ -272,8 +272,8 @@ class RelatedPostings(Sequence[data.Posting]): *, _can_own: bool=False, ) -> None: - self._postings: List[data.Posting] - if _can_own and isinstance(source, list): + self._postings: Sequence[data.Posting] + if _can_own and isinstance(source, Sequence): self._postings = source else: self._postings = list(source) diff --git a/tests/books/accruals.beancount b/tests/books/accruals.beancount index 2106d89..ef355a7 100644 --- a/tests/books/accruals.beancount +++ b/tests/books/accruals.beancount @@ -139,7 +139,7 @@ Liabilities:Payable:Accounts -220.00 USD payment-method: "USD ACH" -2010-06-12 * "Lawyer" "Additional legal fees for May" +2010-06-10 * "Lawyer" "Additional legal fees for May" rt-id: "rt:510" invoice: "rt:510/6100" contract: "rt:510/4000" diff --git a/tests/test_reports_accrual.py b/tests/test_reports_accrual.py index a17fd43..4854363 100644 --- a/tests/test_reports_accrual.py +++ b/tests/test_reports_accrual.py @@ -116,7 +116,9 @@ AGING_AR = [ AgingRow.make_simple('2010-03-05', 'EarlyBird', -500, 'rt:40/400'), AgingRow.make_simple('2010-05-15', 'MatchingProgram', 1500, 'rt://ticket/515/attachments/5150'), - AgingRow.make_simple('2010-06-15', 'GrantCo', 11500, 'rt:470/4700', + AgingRow.make_simple('2010-06-15', 'GrantCo', 5500, 'rt:470/4700', + project='Development Grant'), + AgingRow.make_simple('2010-09-15', 'GrantCo', 6000, 'rt:470/4700', project='Development Grant'), ] @@ -343,12 +345,15 @@ def test_accrual_postings_rt_id_none(): itertools.count(1), )) def test_make_consistent_bad_invoice(acct_name, invoice, day): + if acct_name.startswith('Assets:'): + mult = 10 + else: + mult = -10 txn = testutil.Transaction(date=datetime.date(2019, 1, day), postings=[ - (acct_name, index * 10, {'invoice': invoice, 'entity': f'BadInvoice{day}'}) + (acct_name, index * mult, {'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()) + consistent = dict(accrual.AccrualPostings.make_consistent(data.Posting.from_txn(txn))) assert len(consistent) == 1 key = next(iter(consistent)) assert acct_name in key @@ -367,8 +372,7 @@ def test_make_consistent_across_accounts(): (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()) + consistent = dict(accrual.AccrualPostings.make_consistent(data.Posting.from_txn(txn))) assert len(consistent) == len(ACCOUNTS) for key, posts in consistent.items(): assert len(posts) == 1 @@ -378,8 +382,7 @@ 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()) + consistent = dict(accrual.AccrualPostings.make_consistent(data.Posting.from_txn(txn))) assert len(consistent) == len(ACCOUNTS) for key, posts in consistent.items(): assert len(posts) == 1 @@ -392,8 +395,7 @@ def test_make_consistent_across_entity(acct_name): (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()) + consistent = dict(accrual.AccrualPostings.make_consistent(data.Posting.from_txn(txn))) assert len(consistent) == 3 for key, posts in consistent.items(): assert len(posts) == 1 @@ -411,12 +413,143 @@ def test_make_consistent_entity_differs_accrual_payment(acct_name): (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() + related = list(data.Posting.from_txn(txn)) + consistent = accrual.AccrualPostings.make_consistent(related) _, actual = next(consistent) - assert actual is related + assert len(actual) == len(related) + assert all(post in actual for post in related) assert next(consistent, None) is None +def test_make_consistent_by_date_accruals_differ(): + meta = {'rt-id': '1', 'invoice': 'rt:1/2', 'entity': 'MultiDate'} + entries = [testutil.Transaction(date=date, postings=[ + ('Assets:Receivable:Accounts', date.day * 100, meta), + ]) for date in itertools.islice(testutil.date_seq(), 3)] + actual = [group for _, group in + accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))] + assert len(actual) == 3 + assert {post.units.number for group in actual for post in group} == {100, 200, 300} + +def test_make_consistent_by_date_with_exact_payment(): + meta = {'rt-id': '1', 'invoice': 'rt:1/3', 'entity': 'OnePayment'} + entries = [testutil.Transaction(date=date, postings=[( + 'Assets:Receivable:Accounts', + 35 * (1 if date.day % 2 else -1), + meta, + )]) for date in itertools.islice(testutil.date_seq(), 3)] + actual = [group for _, group in + accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))] + assert len(actual) == 2 + assert actual[0].is_zero() + assert len(actual[1]) == 1 + assert actual[1][0].meta.date.day == 3 + +def test_make_consistent_by_date_with_underpayment(): + meta = {'rt-id': '1', 'invoice': 'rt:1/4', 'entity': 'UnderPayment'} + entries = [testutil.Transaction(date=date, postings=[( + 'Assets:Receivable:Accounts', + 40 * (1 if date.day % 2 else -.5), + meta, + )]) for date in itertools.islice(testutil.date_seq(), 3)] + actual = [group for _, group in + accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))] + assert len(actual) == 2 + assert len(actual[0]) == 2 + assert actual[0][0].units.number == 40 + assert actual[0][1].units.number == -20 + assert len(actual[1]) == 1 + assert actual[1][0].meta.date.day == 3 + +def test_make_consistent_by_date_with_overpayment(): + meta = {'rt-id': '1', 'invoice': 'rt:1/5', 'entity': 'OverPayment'} + entries = [testutil.Transaction(date=date, postings=[( + 'Assets:Receivable:Accounts', + 50 * (1 if date.day % 2 else -1.5), + meta, + )]) for date in itertools.islice(testutil.date_seq(), 3)] + actual = [group for _, group in + accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))] + assert len(actual) == 2 + assert actual[0].is_zero() + assert len(actual[1]) == 2 + assert actual[1][0].meta.date.day == 2 + assert actual[1][0].units.number == -25 + assert actual[1][1].meta.date.day == 3 + +def test_make_consistent_by_date_with_late_payment(): + meta = {'rt-id': '1', 'invoice': 'rt:1/6', 'entity': 'LatePayment'} + entries = [testutil.Transaction(date=date, postings=[( + 'Assets:Receivable:Accounts', + 60 * (-1 if date.day > 2 else 1), + meta, + )]) for date in itertools.islice(testutil.date_seq(), 3)] + actual = [group for _, group in + accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))] + assert len(actual) == 2 + assert len(actual[0]) == 2 + assert actual[0][0].meta.date.day == 1 + assert actual[0][1].meta.date.day == 3 + assert len(actual[1]) == 1 + assert actual[1][0].meta.date.day == 2 + +def test_make_consistent_by_date_with_split_payments(): + meta = {'rt-id': '1', 'invoice': 'rt:1/7', 'entity': 'SplitPayments'} + entries = [testutil.Transaction(date=date, postings=[( + 'Assets:Receivable:Accounts', amount, meta, + )]) for date, amount in zip(testutil.date_seq(), [70, 80, -50, -100])] + actual = [group for _, group in + accrual.AccrualPostings.make_consistent(data.Posting.from_entries(entries))] + assert len(actual) == 2 + assert [post.units.number for post in actual[0]] == [70, -50, -20] + assert [post.units.number for post in actual[1]] == [80, -80] + +@pytest.mark.parametrize('account,day', itertools.product( + ACCOUNTS, + [1, 10, 20, 30], +)) +def test_make_consistent_with_three_one_split(account, day): + meta = {'rt-id': '1', 'invoice': 'rt:1/8', 'entity': '3Split'} + entries = [testutil.Transaction(date=datetime.date(2019, 5, dd), postings=[( + account, dd, meta, + )]) for dd in [5, 15, 25]] + meta['entity'] = '1Split' + entries.insert(day // 10, testutil.Transaction( + date=datetime.date(2019, 5, day), + postings=[(account, -45, meta)], + )) + postings = data.Posting.from_entries(entries) + actual = dict(accrual.AccrualPostings.make_consistent(iter(postings))) + if account.startswith('Assets:'): + group_count = 3 + post_count = 2 + else: + group_count = 1 + post_count = 4 + assert len(actual) == group_count + for related in actual.values(): + assert len(related) == post_count + assert sum(post.units.number for post in related) == 0 + assert all(post.meta['invoice'] == meta['invoice'] for post in related) + assert {post.meta['entity'] for post in related} == {'1Split', '3Split'} + +@pytest.mark.parametrize('account', ACCOUNTS) +def test_make_consistent_with_three_two_split(account): + meta = {'rt-id': '1', 'invoice': 'rt:1/9'} + entries = [testutil.Transaction(date=datetime.date(2019, 5, day), postings=[( + account, day * (1 if day % 10 else -1.5), meta, + )]) for day in range(5, 30, 5)] + postings = data.Posting.from_entries(entries) + actual = dict(accrual.AccrualPostings.make_consistent(iter(postings))) + if account.startswith('Assets:'): + group_count = 3 + else: + group_count = 2 + assert len(actual) == group_count + for related in actual.values(): + assert len(related) >= 2 + assert sum(post.units.number for post in related) == 0 + assert all(post.meta['invoice'] == meta['invoice'] for post in related) + def check_output(output, expect_patterns): output.seek(0) testutil.check_lines_match(iter(output), expect_patterns) @@ -465,7 +598,7 @@ def test_outgoing_report(accrual_postings, caplog): r'^\s*2010-06-10\s', fr'^\s+rt-id: "{rt_id_url}"$', r'^\s+Expenses:Services:Legal\s+220\.00 USD$', - r'^\s*2010-06-12\s', + r'^\s*2010-06-10\s', fr'^\s+contract: "{contract_url}"$', r'^\s+Expenses:FilingFees\s+60\.00 USD$', ]) @@ -536,11 +669,7 @@ def run_aging_report(postings, today=None): post for post in postings if post.account.is_under('Assets:Receivable', 'Liabilities:Payable') ) - groups = { - key: group - for _, related in accrual.AccrualPostings.group_by_meta(postings, 'invoice') - for key, group in related.make_consistent() - } + groups = dict(accrual.AccrualPostings.make_consistent(postings)) output = io.BytesIO() rt_wrapper = rtutil.RT(RTClient()) report = accrual.AgingReport(rt_wrapper, output, today) @@ -561,11 +690,6 @@ def test_aging_report(accrual_postings): def test_aging_report_date_cutoffs(accrual_postings, date, recv_end, pay_end): expect_recv = AGING_AR[:recv_end] expect_pay = AGING_AP[:pay_end] - if 10 <= date.day < 12: - # Take the 60 USD posting out of the invoice 510/6100 payable. - expect_pay[-1] = expect_pay[-1]._replace( - at_cost=testutil.Amount(expect_pay[-1].at_cost.number - 60), - ) output = run_aging_report(accrual_postings, date) check_aging_ods(output, date, expect_recv, expect_pay)