plugin.core: _RequireLinksPostingMetadataHook can check several metadata.

Extend the base class from checking 1 metadata value to checking N.

This is preparation for RT#10643, letting payables be documented with
invoice or contract.

This does unify error reporting, because now we always report all
type/invalid value errors *plus* a missing error if appropriate.
I think this consistency and thoroughness is appropriate, although
it did require some adjustments to the tests.

conservancy_beancount/plugin/core.py

@@ -31,6 +31,7 @@ from typing import (
     Iterator,
     Mapping,
     Optional,
+    Sequence,
     Type,
     TypeVar,
 )
@@ -243,26 +244,29 @@ class _NormalizePostingMetadataHook(_PostingHook):
 class _RequireLinksPostingMetadataHook(_PostingHook):
     """Base class to require that posting metadata include links"""
     # This base class confirms that a posting's metadata has one or more links
-    # under METADATA_KEY.
+    # under one of the metadata keys listed in CHECKED_METADATA.
-    # Most subclasses only need to define METADATA_KEY and _run_on_post.
+    # Most subclasses only need to define CHECKED_METADATA and _run_on_post.
-    METADATA_KEY: str
+    CHECKED_METADATA: Sequence[MetaKey]
 
     def __init_subclass__(cls) -> None:
         super().__init_subclass__()
-        cls.HOOK_GROUPS = cls.HOOK_GROUPS.union(['metadata', cls.METADATA_KEY])
+        cls.HOOK_GROUPS = cls.HOOK_GROUPS.union(cls.CHECKED_METADATA).union('metadata')
 
-    def _check_links(self, txn: Transaction, post: data.Posting, key: MetaKey) -> None:
+    def _check_metadata(self,
-        try:
+                        txn: Transaction,
-            problem = not post.meta.get_links(key)
+                        post: data.Posting,
-            value = None
+                        keys: Sequence[MetaKey],
-        except TypeError:
+    ) -> Iterable[errormod.InvalidMetadataError]:
-            problem = True
+        have_docs = False
-            value = post.meta[key]
+        for key in keys:
-        if problem:
+            try:
-            raise errormod.InvalidMetadataError(txn, key, value, post)
+                links = post.meta.get_links(key)
+            except TypeError as error:
+                yield errormod.InvalidMetadataError(txn, key, post.meta[key], post)
+            else:
+                have_docs = have_docs or any(links)
+        if not have_docs:
+            yield errormod.InvalidMetadataError(txn, '/'.join(keys), None, post)
 
     def post_run(self, txn: Transaction, post: data.Posting) -> errormod.Iter:
-        try:
+        return self._check_metadata(txn, post, self.CHECKED_METADATA)
-            self._check_links(txn, post, self.METADATA_KEY)
-        except errormod.Error as error:
-            yield error

conservancy_beancount/plugin/meta_approval.py

@@ -25,8 +25,7 @@ from ..beancount_types import (
 )
 
 class MetaApproval(core._RequireLinksPostingMetadataHook):
-    METADATA_KEY = 'approval'
+    CHECKED_METADATA = ['approval']
-    CREDIT_CARD_ACCT = 'Liabilities:CreditCard'
 
     def __init__(self, config: configmod.Config) -> None:
         self.payment_threshold = config.payment_threshold()

conservancy_beancount/plugin/meta_invoice.py

@@ -23,7 +23,7 @@ from ..beancount_types import (
 )
 
 class MetaInvoice(core._RequireLinksPostingMetadataHook):
-    METADATA_KEY = 'invoice'
+    CHECKED_METADATA = ['invoice']
 
     def _run_on_post(self, txn: Transaction, post: data.Posting) -> bool:
         return post.account.is_under(

conservancy_beancount/plugin/meta_receipt.py

@@ -29,10 +29,8 @@ from typing import (
     Callable,
 )
 
-_CheckMethod = Callable[[Transaction, data.Posting, MetaKey], None]
-
 class MetaReceipt(core._RequireLinksPostingMetadataHook):
-    METADATA_KEY = 'receipt'
+    CHECKED_METADATA = ['receipt']
 
     def __init__(self, config: configmod.Config) -> None:
         self.payment_threshold = abs(config.payment_threshold())
@@ -44,43 +42,35 @@ class MetaReceipt(core._RequireLinksPostingMetadataHook):
             and abs(post.units.number) >= self.payment_threshold
         )
 
-    def _check_check_id(self, txn: Transaction, post: data.Posting, key: MetaKey) -> None:
+    def _run_checking_debit(self, txn: Transaction, post: data.Posting) -> errormod.Iter:
-        value = post.meta.get(key)
+        receipt_errors = list(self._check_metadata(txn, post, self.CHECKED_METADATA))
-        if (not isinstance(value, Decimal)
+        if not receipt_errors:
-            or value < 1
+            return
-            or value % 1):
+        for error in receipt_errors:
-            raise errormod.InvalidMetadataError(txn, key, value, post, Decimal)
+            if error.value is not None:
+                yield error
+        try:
+            check_id = post.meta['check-id']
+        except KeyError:
+            check_id_ok = False
+        else:
+            check_id_ok = (isinstance(check_id, Decimal)
+                           and check_id >= 1
+                           and not check_id % 1)
+            if not check_id_ok:
+                yield errormod.InvalidMetadataError(txn, 'check-id', check_id, post, Decimal)
+        if not check_id_ok:
+            yield errormod.InvalidMetadataError(txn, 'receipt/check-id', post=post)
 
     def post_run(self, txn: Transaction, post: data.Posting) -> errormod.Iter:
-        try:
+        keys = list(self.CHECKED_METADATA)
-            self._check_links(txn, post, self.METADATA_KEY)
+        is_checking = post.account.is_checking()
-        except errormod.InvalidMetadataError as error:
+        if is_checking and post.is_debit():
-            receipt_error = error
+            return self._run_checking_debit(txn, post)
-        else:
+        elif is_checking:
-            return
+            keys.append('check')
-
-        check_method: _CheckMethod = self._check_links
-        if post.account.is_checking():
-            if post.is_debit():
-                check_method = self._check_check_id
-                fallback_key = 'check-id'
-            else:
-                fallback_key = 'check'
         elif post.account.is_credit_card() and not post.is_credit():
-            fallback_key = 'invoice'
+            keys.append('invoice')
         elif post.account.is_under('Assets:PayPal') and not post.is_debit():
-            fallback_key = 'paypal-id'
+            keys.append('paypal-id')
-        else:
+        return self._check_metadata(txn, post, keys)
-            yield receipt_error
-            return
-
-        try:
-            check_method(txn, post, fallback_key)
-        except errormod.InvalidMetadataError as fallback_error:
-            if receipt_error.value is None and fallback_error.value is None:
-                yield errormod.InvalidMetadataError(
-                    txn, f"{self.METADATA_KEY} or {fallback_key}", None, post,
-                )
-            else:
-                yield receipt_error
-                yield fallback_error

conservancy_beancount/plugin/meta_receivable_documentation.py

@@ -32,12 +32,7 @@ from typing import (
 
 class MetaReceivableDocumentation(core._RequireLinksPostingMetadataHook):
     HOOK_GROUPS = frozenset(['network', 'rt'])
-    SUPPORTING_METADATA = frozenset([
+    CHECKED_METADATA = ['approval', 'contract', 'purchase-order']
-        'approval',
-        'contract',
-        'purchase-order',
-    ])
-    METADATA_KEY = '/'.join(sorted(SUPPORTING_METADATA))
     # Conservancy invoice filenames have followed two patterns.
     # The pre-RT pattern: `YYYY-MM-DD_Entity_invoice-YYYYMMDDNN??_as-sent.pdf`
     # The RT pattern: `ProjectInvoice-30NNNN??.pdf`
@@ -74,21 +69,3 @@ class MetaReceivableDocumentation(core._RequireLinksPostingMetadataHook):
             ticket_id, attachment_id = rt_args
             invoice_link = self.rt.url(ticket_id, attachment_id) or invoice_link
         return self.ISSUED_INVOICE_RE.search(invoice_link) is not None
-
-    def post_run(self, txn: Transaction, post: data.Posting) -> errormod.Iter:
-        errors: Dict[MetaKey, Optional[errormod.InvalidMetadataError]] = {
-            key: None for key in self.SUPPORTING_METADATA
-        }
-        have_support = False
-        for key in errors:
-            try:
-                self._check_links(txn, post, key)
-            except errormod.InvalidMetadataError as key_error:
-                errors[key] = key_error
-            else:
-                have_support = True
-        for key, error in errors.items():
-            if error is not None and error.value is not None:
-                yield error
-        if not have_support:
-            yield errormod.InvalidMetadataError(txn, self.METADATA_KEY, None, post)

tests/test_meta_invoice.py

@@ -37,6 +37,9 @@ NON_REQUIRED_ACCOUNTS = {
 
 TEST_KEY = 'invoice'
 
+MISSING_MSG = f'{{}} missing {TEST_KEY}'.format
+WRONG_TYPE_MSG = f'{{}} has wrong type of {TEST_KEY}: expected str but is a {{}}'.format
+
 @pytest.fixture(scope='module')
 def hook():
     config = testutil.TestConfig()
@@ -77,13 +80,12 @@ def test_bad_type_values_on_postings(hook, acct1, acct2, value):
         (acct2, -25),
         (acct1, 25, {TEST_KEY: value}),
     ])
-    expected_msg = "{} has wrong type of {}: expected str but is a {}".format(
+    expected = {
-        acct1,
+        MISSING_MSG(acct1),
-        TEST_KEY,
+        WRONG_TYPE_MSG(acct1, type(value).__name__),
-        type(value).__name__,
+    }
-    )
     actual = {error.message for error in hook.run(txn)}
-    assert actual == {expected_msg}
+    assert actual == expected
 
 @pytest.mark.parametrize('acct1,acct2,value', testutil.combine_values(
     REQUIRED_ACCOUNTS,
@@ -120,13 +122,12 @@ def test_bad_type_values_on_transaction(hook, acct1, acct2, value):
         (acct2, -25),
         (acct1, 25),
     ])
-    expected_msg = "{} has wrong type of {}: expected str but is a {}".format(
+    expected = {
-        acct1,
+        MISSING_MSG(acct1),
-        TEST_KEY,
+        WRONG_TYPE_MSG(acct1, type(value).__name__),
-        type(value).__name__,
+    }
-    )
     actual = {error.message for error in hook.run(txn)}
-    assert actual == {expected_msg}
+    assert actual == expected
 
 @pytest.mark.parametrize('acct1,acct2', testutil.combine_values(
     REQUIRED_ACCOUNTS,

tests/test_meta_receipt.py

@@ -44,7 +44,7 @@ class AccountForTesting(typing.NamedTuple):
         if self.fallback_meta is None or not include_fallback:
             rest = ""
         else:
-            rest = f" or {self.fallback_meta}"
+            rest = f"/{self.fallback_meta}"
         return f"{self.name} missing {TEST_KEY}{rest}"
 
     def wrong_type_message(self, wrong_value, key=TEST_KEY):
@@ -206,7 +206,7 @@ def test_invalid_fallback_on_post(hook, test_acct, other_acct, value):
 ))
 def test_bad_type_fallback_on_post(hook, test_acct, other_acct, value):
     expected = {
-        test_acct.missing_message(False),
+        test_acct.missing_message(),
         test_acct.wrong_type_message(value, test_acct.fallback_meta),
     }
     check(hook, test_acct, other_acct, expected,
@@ -237,7 +237,7 @@ def test_invalid_fallback_on_txn(hook, test_acct, other_acct, value):
 ))
 def test_bad_type_fallback_on_txn(hook, test_acct, other_acct, value):
     expected = {
-        test_acct.missing_message(False),
+        test_acct.missing_message(),
         test_acct.wrong_type_message(value, test_acct.fallback_meta),
     }
     check(hook, test_acct, other_acct, expected,
@@ -259,7 +259,7 @@ def test_valid_check_id_on_post(hook, test_acct, other_acct, value):
 ))
 def test_invalid_check_id_on_post(hook, test_acct, other_acct, value):
     expected = {
-        test_acct.missing_message(False),
+        test_acct.missing_message(),
         f"{test_acct.name} has invalid {test_acct.fallback_meta}: {value}",
     }
     check(hook, test_acct, other_acct, expected,
@@ -274,7 +274,7 @@ def test_bad_type_check_id_on_post(hook, test_acct, other_acct, value):
     if isinstance(value, decimal.Decimal):
         value = ''
     expected = {
-        test_acct.missing_message(False),
+        test_acct.missing_message(),
         test_acct.wrong_type_message(value, test_acct.fallback_meta),
     }
     check(hook, test_acct, other_acct, expected,
@@ -296,7 +296,7 @@ def test_valid_check_id_on_txn(hook, test_acct, other_acct, value):
 ))
 def test_invalid_check_id_on_txn(hook, test_acct, other_acct, value):
     expected = {
-        test_acct.missing_message(False),
+        test_acct.missing_message(),
         f"{test_acct.name} has invalid {test_acct.fallback_meta}: {value}",
     }
     check(hook, test_acct, other_acct, expected,
@@ -311,7 +311,7 @@ def test_bad_type_check_id_on_txn(hook, test_acct, other_acct, value):
     if isinstance(value, decimal.Decimal):
         value = ''
     expected = {
-        test_acct.missing_message(False),
+        test_acct.missing_message(),
         test_acct.wrong_type_message(value, test_acct.fallback_meta),
     }
     check(hook, test_acct, other_acct, expected,
@@ -327,7 +327,6 @@ def test_fallback_not_accepted_on_other_accounts(hook, test_acct, other_acct, ke
     check(hook, test_acct, other_acct, {test_acct.missing_message()},
           post_meta={key: value})
 
-
 @pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values(
     ACCOUNTS_WITH_LINK_FALLBACK,
     NOT_REQUIRED_ACCOUNTS,