NPO-Accounting/conservancy_beancount Changeset - 5859421a15ce

Changeset - 5859421a15ce

Parent rev.

Child rev.

[Not reviewed]

0 3 0

Brett Smith - 4 years ago 2020-06-10 20:03:08
brettcsmith@brettcsmith.org

accrual: Remove the consistency checker.

Everything it said was a problem has been done legitimately in our books at
one point or another.

* Variation in contract can happen in different line items of an invoice or
"group of contractor" situations.

* Variation in cost can happen because one invoice spans a period of time,
like donation matching programs. There is probably still value in a tool
that checks to make sure we use consistent rates each day, but that
affects all kinds of transactions, not just accruals, so it would be
done better in a separate tool.

* Variation in account happens because invoices legitimately span accrual
accounts, like donation matching programs with fees payable.

So: it's gone, good riddance.

3 files changed with 54 insertions and 224 deletions:

conservancy_beancount/reports/accrual.py

setup.py

tests/test_reports_accrual.py

130

0 comments (0 inline, 0 general)

conservancy_beancount/reports/accrual.py

➞

Show inline comments

@@ ... / @@ -169,32 +169,14 @@ class AccrualAccount(enum.Enum): @@
 class AccrualPostings(core.RelatedPostings):
     def _meta_getter(key: MetaKey) -> Callable[[data.Posting], MetaValue]:  # type:ignore[misc]
         def meta_getter(post: data.Posting) -> MetaValue:
             return post.meta.get(key)
         return meta_getter
     _FIELDS: Dict[str, Callable[[data.Posting], MetaValue]] = {
         'account': operator.attrgetter('account'),
         'contract': _meta_getter('contract'),
         'invoice': _meta_getter('invoice'),
         'purchase_order': _meta_getter('purchase-order'),
+    }
     INCONSISTENT = Sentinel()
     __slots__ = (
         'accrual_type',
         'accrued_entities',
         'end_balance',
         'paid_entities',
         'account',
         'accounts',
         'contract',
         'contracts',
         'entity',
         'invoice',
         'invoices',
         'purchase_order',
         'purchase_orders',
+    )
     INCONSISTENT = Sentinel()
     def __init__(self,
                  source: Iterable[data.Posting]=(),
@@ ... / @@ -204,56 +186,57 @@ class AccrualPostings(core.RelatedPostings): @@
         super().__init__(source, _can_own=_can_own)
         # The following type declarations tell mypy about values set in the for
         # loop that are important enough to be referenced directly elsewhere.
         self.account: Union[data.Account, Sentinel]
         self.invoice: Union[MetaValue, Sentinel]
         for name, get_func in self._FIELDS.items():
             values = frozenset(get_func(post) for post in self)
             setattr(self, f'{name}s', values)
             if len(values) == 1:
                 one_value = next(iter(values))
             else:
                 one_value = self.INCONSISTENT
             setattr(self, name, one_value)
         if self.account is self.INCONSISTENT:
         self.account = self._single_item(post.account for post in self)
         if isinstance(self.account, Sentinel):
             self.accrual_type: Optional[AccrualAccount] = None
             self.end_balance = self.balance_at_cost()
             self.accrued_entities = self._collect_entities()
             self.paid_entities = self.accrued_entities
             norm_func: Callable[[T], T] = lambda x: x
             entity_pred: Callable[[data.Posting], bool] = bool
         else:
-            self.accrual_type = AccrualAccount.classify(self)
+            self.accrual_type = AccrualAccount.by_account(self.account)
             norm_func = self.accrual_type.normalize_amount
             self.end_balance = norm_func(self.balance_at_cost())
             self.accrued_entities = self._collect_entities(
                 lambda post: norm_func(post.units).number > 0,
+            )
             self.paid_entities = self._collect_entities(
                 lambda post: norm_func(post.units).number < 0,
+            )
             entity_pred = lambda post: norm_func(post.units).number > 0
         self.entity = self._single_item(self.entities(entity_pred))
         self.invoice = self._single_item(self.first_links('invoice'))
         self.end_balance = norm_func(self.balance_at_cost())
     def _collect_entities(self,
                           pred: Callable[[data.Posting], bool]=bool,
                           default: str='<empty>',
     ) -> FrozenSet[MetaValue]:
         return frozenset(
             post.meta.get('entity') or default
             for post in self if pred(post)
+        )
     def _single_item(self, seq: Iterable[T]) -> Union[T, Sentinel]:
         items = iter(seq)
         try:
             item1 = next(items)
         except StopIteration:
             all_same = False
         else:
             all_same = all(item == item1 for item in items)
         return item1 if all_same else self.INCONSISTENT
     def entities(self) -> Iterator[MetaValue]:
         yield from self.accrued_entities
         yield from self.paid_entities.difference(self.accrued_entities)
     def entities(self, pred: Callable[[data.Posting], bool]=bool) -> Iterator[MetaValue]:
         seen: Set[MetaValue] = set()
         for post in self:
             if pred(post):
                 try:
                     entity = post.meta['entity']
                 except KeyError:
                     pass
                 else:
                     if entity not in seen:
                         yield entity
                         seen.add(entity)
     def first_links(self, key: MetaKey, default: Optional[str]=None) -> Iterator[Optional[str]]:
         for post in self:
             try:
                 yield post.meta.get_links(key)[0]
             except (IndexError, TypeError):
                 yield default
     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
         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 is not None and invoice_ok:
+        if account_ok and entity_ok and invoice_ok:
             yield (self.invoice, self)
             return
         groups = collections.defaultdict(list)
@@ ... / @@ -261,7 +244,7 @@ class AccrualPostings(core.RelatedPostings): @@
             post_invoice = self.invoice if invoice_ok else (
                 post.meta.get('invoice') or 'BlankInvoice'
+            )
-            post_entity = entity if entity is not None else (
+            post_entity = self.entity if entity_ok else (
                 post.meta.get('entity') or 'BlankEntity'
+            )
             groups[f'{post.account} {post_invoice} {post_entity}'].append(post)
@@ ... / @@ -269,28 +252,6 @@ class AccrualPostings(core.RelatedPostings): @@
         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:
                 for post in self:
                     errmsg = 'inconsistent {} for invoice {}: {}'.format(
                         field_name.replace('_', '-'),
                         self.invoice or "<none>",
                         get_func(post),
+                    )
                     yield Error(post.meta, errmsg, post.meta.txn)
         costs = collections.defaultdict(set)
         for post in self:
             costs[post.units.currency].add(post.cost)
         for code, currency_costs in costs.items():
             if len(currency_costs) > 1:
                 for post in self:
                     if post.units.currency == code:
                         errmsg = 'inconsistent cost for invoice {}: {}'.format(
                             self.invoice or "<none>", post.cost,
+                        )
                         yield Error(post.meta, errmsg, post.meta.txn)
     def is_paid(self, default: Optional[bool]=None) -> Optional[bool]:
         if self.accrual_type is None:
             return default
@@ ... / @@ -533,7 +494,9 @@ class AgingReport(BaseReport): @@
         rows.sort(key=lambda related: (
             related.account,
             related[0].meta.date,
             min(related.entities()) if related.accrued_entities else '',
             ('\0'.join(related.entities())
              if related.entity is related.INCONSISTENT
              else related.entity),
         ))
         self.ods.write(rows)
         self.ods.save_file(self.out_bin)
@@ ... / @@ -556,12 +519,7 @@ class OutgoingReport(BaseReport): @@
         self.rt_wrapper = rtutil.RT(rt_client)
     def _primary_rt_id(self, posts: AccrualPostings) -> rtutil.TicketAttachmentIds:
         rt_ids: Set[str] = set()
         for post in posts:
             try:
                 rt_ids.add(post.meta.get_links('rt-id')[0])
             except (IndexError, TypeError):
                 pass
         rt_ids = {url for url in posts.first_links('rt-id') if url is not None}
         rt_ids_count = len(rt_ids)
         if rt_ids_count != 1:
             raise ValueError(f"{rt_ids_count} rt-id links found")
@@ ... / @@ -666,10 +624,10 @@ class ReportType(enum.Enum): @@
 class ReturnFlag(enum.IntFlag):
     LOAD_ERRORS = 1
     CONSISTENCY_ERRORS = 2
     REPORT_ERRORS = 4
     NOTHING_TO_REPORT = 8
 def filter_search(postings: Iterable[data.Posting],
                   search_terms: Iterable[cliutil.SearchTerm],
 ) -> Iterable[data.Posting]:
@@ ... / @@ -756,10 +714,6 @@ def main(arglist: Optional[Sequence[str]]=None, @@
     for error in load_errors:
         bc_printer.print_error(error, file=stderr)
         returncode |= ReturnFlag.LOAD_ERRORS
     for related in groups.values():
         for error in related.report_inconsistencies():
             bc_printer.print_error(error, file=stderr)
             returncode |= ReturnFlag.CONSISTENCY_ERRORS
     if not groups:
         logger.warning("no matching entries found to report")
         returncode |= ReturnFlag.NOTHING_TO_REPORT

setup.py

➞

Show inline comments

@@ ... / @@ -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.7',
+    version='1.1.8',
     author='Software Freedom Conservancy',
     author_email='info@sfconservancy.org',
     license='GNU AGPLv3+',

tests/test_reports_accrual.py

➞

Show inline comments

@@ ... / @@ -63,11 +63,6 @@ ACCOUNTS = [ @@
     'Liabilities:Payable:Vacation',
+]
 CONSISTENT_METADATA = [
     'contract',
     'purchase-order',
+]
 class AgingRow(NamedTuple):
     date: datetime.date
     entity: Sequence[str]
@@ ... / @@ -271,26 +266,6 @@ def test_accrual_postings_consistent_account(acct_name): @@
     ])
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     assert related.account == acct_name
     assert related.accounts == {acct_name}
 @pytest.mark.parametrize('meta_key,acct_name', testutil.combine_values(
     CONSISTENT_METADATA,
     ACCOUNTS,
 ))
 def test_accrual_postings_consistent_metadata(meta_key, acct_name):
     meta_value = f'{meta_key}.pdf'
     meta = {
         meta_key: meta_value,
         'invoice': f'invoice with {meta_key}.pdf',
+    }
     txn = testutil.Transaction(postings=[
         (acct_name, 70, meta),
         (acct_name, 35, meta),
     ])
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     attr_name = meta_key.replace('-', '_')
     assert getattr(related, attr_name) == meta_value
     assert getattr(related, f'{attr_name}s') == {meta_value}
 def test_accrual_postings_entity():
     txn = testutil.Transaction(postings=[
@@ ... / @@ -299,8 +274,8 @@ def test_accrual_postings_entity(): @@
         (ACCOUNTS[0], -10, {'entity': 'Payee10'}),
     ])
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     assert related.accrued_entities == {'Accruee'}
     assert related.paid_entities == {'Payee10', 'Payee15'}
     assert related.entity == 'Accruee'
     assert set(related.entities()) == {'Accruee', 'Payee10', 'Payee15'}
 def test_accrual_postings_entities():
     txn = testutil.Transaction(postings=[
@@ ... / @@ -333,107 +308,6 @@ def test_accrual_postings_inconsistent_account(): @@
     ])
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     assert related.account is related.INCONSISTENT
     assert related.accounts == set(ACCOUNTS)
 @pytest.mark.parametrize('meta_key,acct_name', testutil.combine_values(
     CONSISTENT_METADATA,
     ACCOUNTS,
 ))
 def test_accrual_postings_inconsistent_metadata(meta_key, acct_name):
     invoice = 'invoice with {meta_key}.pdf'
     meta_value = f'{meta_key}.pdf'
     txn = testutil.Transaction(postings=[
         (acct_name, 20, {'invoice': invoice, meta_key: meta_value}),
         (acct_name, 35, {'invoice': invoice}),
     ])
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     attr_name = meta_key.replace('-', '_')
     assert getattr(related, attr_name) is related.INCONSISTENT
     assert getattr(related, f'{attr_name}s') == {meta_value, None}
 @pytest.mark.parametrize('meta_key,account', testutil.combine_values(
     CONSISTENT_METADATA,
     ACCOUNTS,
 ))
 def test_consistency_check_when_consistent(meta_key, account):
     invoice = f'test-{meta_key}-invoice'
     meta_value = f'test-{meta_key}-value'
     meta = {
         'invoice': invoice,
         meta_key: meta_value,
+    }
     txn = testutil.Transaction(postings=[
         (account, 100, meta),
         (account, -100, meta),
     ])
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     assert not list(related.report_inconsistencies())
 @pytest.mark.parametrize('meta_key,account', testutil.combine_values(
     ['approval', 'entity', 'fx-rate', 'statement'],
     ACCOUNTS,
 ))
 def test_consistency_check_ignored_metadata(meta_key, account):
     invoice = f'test-{meta_key}-invoice'
     txn = testutil.Transaction(postings=[
         (account, 100, {'invoice': invoice, meta_key: 'credit'}),
         (account, -100, {'invoice': invoice, meta_key: 'debit'}),
     ])
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     assert not list(related.report_inconsistencies())
 @pytest.mark.parametrize('meta_key,account', testutil.combine_values(
     CONSISTENT_METADATA,
     ACCOUNTS,
 ))
 def test_consistency_check_when_inconsistent(meta_key, account):
     invoice = f'test-{meta_key}-invoice'
     txn = testutil.Transaction(postings=[
         (account, 100, {'invoice': invoice, meta_key: 'credit', 'lineno': 1}),
         (account, -100, {'invoice': invoice, meta_key: 'debit', 'lineno': 2}),
     ])
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     errors = list(related.report_inconsistencies())
     for exp_lineno, (actual, exp_msg) in enumerate(itertools.zip_longest(errors, [
             f'inconsistent {meta_key} for invoice {invoice}: credit',
             f'inconsistent {meta_key} for invoice {invoice}: debit',
     ]), 1):
         assert actual.message == exp_msg
         assert actual.entry is txn
         assert actual.source.get('lineno') == exp_lineno
 def test_consistency_check_cost():
     account = ACCOUNTS[0]
     invoice = 'test-cost-invoice'
     txn = testutil.Transaction(postings=[
         (account, 100, 'EUR', ('1.1251', 'USD'), {'invoice': invoice, 'lineno': 1}),
         (account, -100, 'EUR', ('1.125', 'USD'), {'invoice': invoice, 'lineno': 2}),
     ])
     related = accrual.AccrualPostings(data.Posting.from_txn(txn))
     errors = list(related.report_inconsistencies())
     for post, err in itertools.zip_longest(txn.postings, errors):
         assert err.message == f'inconsistent cost for invoice {invoice}: {post.cost}'
         assert err.entry is txn
         assert err.source.get('lineno') == post.meta['lineno']
 def test_make_consistent_not_needed():
     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, {**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 == main_meta['invoice']
     assert actual_postings is related
     assert next(consistent, None) is None
 @pytest.mark.parametrize('acct_name,invoice,day', testutil.combine_values(
     ACCOUNTS,
@@ ... / @@ -495,8 +369,10 @@ def test_make_consistent_across_entity(acct_name): @@
     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
         entities = posts.entities()
         assert next(entities, None) == posts.entity
         assert next(entities, None) is None
         assert posts.entity in key
 @pytest.mark.parametrize('acct_name', ACCOUNTS)
 def test_make_consistent_entity_differs_accrual_payment(acct_name):

0 comments (0 inline, 0 general)