plugin: Be more selective about when ! skips validation.

It makes sense to let the bookkeeper skip validations in situations
where the metadata requires information that might not be available
when entered. It does not make sense to skip validations that *must*
be available and affect the structure of the books, like project and
entity.

This commit ensures every plugin hook has a test for flagged
transactions, even for hooks that currently have the desired
behavior where no code changes were required for the test to
pass.

conservancy_beancount/plugin/core.py

@@ -24,6 +24,7 @@ from .. import errors as errormod
 
 from typing import (
     Any,
+    Container,
     Dict,
     FrozenSet,
     Generic,
@@ -176,9 +177,10 @@ class MetadataEnum:
 
 class TransactionHook(Hook[Transaction]):
     DIRECTIVE = Transaction
+    SKIP_FLAGS: Container[str] = frozenset()
     TXN_DATE_RANGE: _GenericRange = _GenericRange(DEFAULT_START_DATE, DEFAULT_STOP_DATE)
 
-    def _run_on_txn(self, txn: Transaction, skip_flags: str='!') -> bool:
+    def _run_on_txn(self, txn: Transaction) -> bool:
         """Check whether we should run on a given transaction
 
         This method implements our usual checks for whether or not a hook
@@ -186,7 +188,7 @@ class TransactionHook(Hook[Transaction]):
         their own implementations. See _PostingHook below for an example.
         """
         return (
-            txn.flag not in skip_flags
+            txn.flag not in self.SKIP_FLAGS
             and txn.date in self.TXN_DATE_RANGE
             and not data.is_opening_balance_txn(txn)
         )

conservancy_beancount/plugin/meta_approval.py

@@ -26,13 +26,14 @@ from ..beancount_types import (
 
 class MetaApproval(core._RequireLinksPostingMetadataHook):
     CHECKED_METADATA = ['approval']
+    SKIP_FLAGS = '!'
 
     def __init__(self, config: configmod.Config) -> None:
         self.payment_threshold = -config.payment_threshold()
 
-    def _run_on_txn(self, txn: Transaction, skip_flags: str='!') -> bool:
+    def _run_on_txn(self, txn: Transaction) -> bool:
         return (
-            super()._run_on_txn(txn, skip_flags)
+            super()._run_on_txn(txn)
             # approval is required when funds leave a cash equivalent asset,
             # UNLESS that transaction is a transfer to another asset,
             # or paying off a credit card.

conservancy_beancount/plugin/meta_entity.py

@@ -67,7 +67,7 @@ class MetaEntity(core.TransactionHook):
             return entity, self.ENTITY_RE.match(entity) is not None
 
     def run(self, txn: Transaction) -> errormod.Iter:
-        if not self._run_on_txn(txn, ''):
+        if not self._run_on_txn(txn):
             return
         txn_entity, txn_entity_ok = self._check_entity(txn.meta, txn.payee)
         if txn_entity_ok is False:

conservancy_beancount/plugin/meta_invoice.py

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

conservancy_beancount/plugin/meta_payable_documentation.py

@@ -24,6 +24,7 @@ from ..beancount_types import (
 
 class MetaPayableDocumentation(core._RequireLinksPostingMetadataHook):
     CHECKED_METADATA = ['approval', 'contract']
+    SKIP_FLAGS = '!'
 
     def _run_on_post(self, txn: Transaction, post: data.Posting) -> bool:
         if post.account.is_under('Liabilities:Payable:Accounts'):

conservancy_beancount/plugin/meta_project.py

@@ -102,7 +102,7 @@ class MetaProject(core._NormalizePostingMetadataHook):
         else:
             raise errormod.InvalidMetadataError(txn, self.METADATA_KEY, None, post)
 
-    def _run_on_txn(self, txn: Transaction, skip_flags: str='') -> bool:
+    def _run_on_txn(self, txn: Transaction) -> bool:
         return txn.date in self.TXN_DATE_RANGE
 
     def run(self, txn: Transaction) -> errormod.Iter:

conservancy_beancount/plugin/meta_receipt.py

@@ -31,6 +31,7 @@ from typing import (
 
 class MetaReceipt(core._RequireLinksPostingMetadataHook):
     CHECKED_METADATA = ['receipt']
+    SKIP_FLAGS = '!'
 
     def __init__(self, config: configmod.Config) -> None:
         self.payment_threshold = abs(config.payment_threshold())

conservancy_beancount/plugin/meta_receivable_documentation.py

@@ -41,6 +41,7 @@ class MetaReceivableDocumentation(core._RequireLinksPostingMetadataHook):
     ISSUED_INVOICE_RE = re.compile(
         r'[Ii]nvoice[-_ ]*(?:2[0-9]{9,}|30[0-9]+)[A-Za-z]*[-_ .]',
     )
+    SKIP_FLAGS = '!'
 
     def __init__(self, config: configmod.Config) -> None:
         rt_wrapper = config.rt_wrapper()

conservancy_beancount/plugin/meta_repo_links.py

@@ -36,6 +36,7 @@ class MetaRepoLinks(core.TransactionHook):
     HOOK_GROUPS = frozenset(['linkcheck'])
     LINK_METADATA = data.LINK_METADATA.difference('rt-id')
     PATH_PUNCT_RE = re.compile(r'[:/]')
+    SKIP_FLAGS = '!'
 
     def __init__(self, config: configmod.Config) -> None:
         repo_path = config.repository_path()

conservancy_beancount/plugin/meta_rt_links.py

@@ -32,6 +32,7 @@ from typing import (
 
 class MetaRTLinks(core.TransactionHook):
     HOOK_GROUPS = frozenset(['linkcheck', 'network', 'rt'])
+    SKIP_FLAGS = '!'
 
     def __init__(self, config: configmod.Config) -> None:
         rt_wrapper = config.rt_wrapper()

conservancy_beancount/plugin/meta_tax_implication.py

@@ -39,6 +39,9 @@ class MetaTaxImplication(core._NormalizePostingMetadataHook):
         'USA-Corporation',
         'W2',
     ])
+    # Sometimes we accrue a payment before we have determined the recipient's
+    # tax status.
+    SKIP_FLAGS = '!'
 
     def __init__(self, config: configmod.Config) -> None:
         self.payment_threshold = -config.payment_threshold()

tests/test_meta_entity.py

@@ -226,3 +226,10 @@ def test_required_by_date(hook, date, need_value):
         ('Assets:Checking', 10),
     ])
     assert any(hook.run(txn)) == need_value
+
+def test_still_required_on_flagged(hook):
+    txn = testutil.Transaction(flag='!', postings=[
+        ('Income:Donations', -10),
+        ('Assets:Checking', 10),
+    ])
+    assert list(hook.run(txn))

tests/test_meta_expense_allocation.py

@@ -132,3 +132,13 @@ def test_default_value_set_in_date_range(hook, date, set_value):
     assert not errors
     expect_meta = None if set_value is None else {TEST_KEY: set_value}
     testutil.check_post_meta(txn, None, expect_meta)
+
+@pytest.mark.parametrize('src_value', INVALID_VALUES)
+def test_flagged_txn_checked(hook, src_value):
+    txn = testutil.Transaction(flag='!', postings=[
+        ('Assets:Cash', -25),
+        ('Expenses:General', 25, {TEST_KEY: src_value}),
+    ])
+    errors = list(hook.run(txn))
+    assert errors
+    testutil.check_post_meta(txn, None, {TEST_KEY: src_value})

tests/test_meta_income_type.py

@@ -149,3 +149,13 @@ def test_default_value_set_in_date_range(hook, date, set_value):
     assert not errors
     expect_meta = None if set_value is None else {TEST_KEY: set_value}
     testutil.check_post_meta(txn, None, expect_meta)
+
+@pytest.mark.parametrize('src_value', INVALID_VALUES)
+def test_flagged_txn_checked(hook, src_value):
+    txn = testutil.Transaction(flag='!', postings=[
+        ('Assets:Cash', 25),
+        ('Income:Other', -25, {TEST_KEY: src_value}),
+    ])
+    errors = list(hook.run(txn))
+    assert errors
+    testutil.check_post_meta(txn, None, {TEST_KEY: src_value})

tests/test_meta_paypal_id.py

@@ -194,3 +194,10 @@ def test_not_required_on_opening(hook):
         (next(testutil.OPENING_EQUITY_ACCOUNTS), -1000),
     ])
     assert not list(hook.run(txn))
+
+def test_still_required_on_flagged_txn(hook):
+    txn = testutil.Transaction(flag='!', postings=[
+        ('Assets:PayPal', 1000),
+        ('Income:Donations', -1000),
+    ])
+    assert list(hook.run(txn))

tests/test_meta_project.py

@@ -166,3 +166,13 @@ def test_always_required_on_restricted_funds(hook):
     txn = testutil.OpeningBalance(acct)
     actual = {error.message for error in hook.run(txn)}
     assert actual == {f'{acct} missing project'}
+
+@pytest.mark.parametrize('src_value', INVALID_VALUES)
+def test_still_required_on_flagged_txn(hook, src_value):
+    txn = testutil.Transaction(flag='!', **{TEST_KEY: src_value}, postings=[
+        ('Assets:Cash', -25),
+        ('Expenses:General', 25),
+    ])
+    errors = list(hook.run(txn))
+    assert errors
+    testutil.check_post_meta(txn, None, None)

tests/test_meta_tax_implication.py

@@ -135,3 +135,11 @@ def test_validation_only_in_date_range(hook, date, need_value):
     errors = list(hook.run(txn))
     assert bool(errors) == bool(need_value)
     testutil.check_post_meta(txn, None, None)
+
+@pytest.mark.parametrize('src_value', INVALID_VALUES)
+def test_flagged_txn_skipped(hook, src_value):
+    txn = testutil.Transaction(flag='!', **{TEST_KEY: src_value}, postings=[
+        ('Liabilities:Payable:Accounts', 25),
+        ('Assets:Cash', -25),
+    ])
+    assert not list(hook.run(txn))