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.

conservancy_beancount/reports/accrual.py

@@ -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',
+        'entity',
-        'contract',
-        'contracts',
         '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.account = self._single_item(post.account for post in self)
-        self.invoice: Union[MetaValue, Sentinel]
+        if isinstance(self.account, 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.accrual_type: Optional[AccrualAccount] = None
-            self.end_balance = self.balance_at_cost()
+            norm_func: Callable[[T], T] = lambda x: x
-            self.accrued_entities = self._collect_entities()
+            entity_pred: Callable[[data.Posting], bool] = bool
-            self.paid_entities = self.accrued_entities
         else:
-            self.accrual_type = AccrualAccount.classify(self)
+            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))
+        self.invoice = self._single_item(self.first_links('invoice'))
         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,
-            )
 
-    def _collect_entities(self,
+    def _single_item(self, seq: Iterable[T]) -> Union[T, Sentinel]:
-                          pred: Callable[[data.Posting], bool]=bool,
+        items = iter(seq)
-                          default: str='<empty>',
+        try:
-    ) -> FrozenSet[MetaValue]:
+            item1 = next(items)
-        return frozenset(
+        except StopIteration:
-            post.meta.get('entity') or default
+            all_same = False
-            for post in self if pred(post)
+        else:
-        )
+            all_same = all(item == item1 for item in items)
+        return item1 if all_same else self.INCONSISTENT
 
-    def entities(self) -> Iterator[MetaValue]:
+    def entities(self, pred: Callable[[data.Posting], bool]=bool) -> Iterator[MetaValue]:
-        yield from self.accrued_entities
+        seen: Set[MetaValue] = set()
-        yield from self.paid_entities.difference(self.accrued_entities)
+        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_ok = isinstance(self.entity, str)
-            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 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()
+        rt_ids = {url for url in posts.first_links('rt-id') if url is not None}
-        for post in posts:
-            try:
-                rt_ids.add(post.meta.get_links('rt-id')[0])
-            except (IndexError, TypeError):
-                pass
         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

@@ -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

@@ -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.entity == 'Accruee'
-    assert related.paid_entities == {'Payee10', 'Payee15'}
+    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
+        entities = posts.entities()
-        assert next(posts.entities()) in key
+        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):