From fdd9f2847b787b951984e3d7340cd341ceb88750 Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Fri, 29 Jan 2021 09:38:37 -0500 Subject: [PATCH] plugin: Skip enum value checks with a flag+FIXME. We've long supported skipping documentation checks by flagging the transaction. We haven't done the same for enumerated metadata because we need it less often, and bad values tend to do more damage to reports. However, occasionally when something very off-process happens, we do need it as a matter of expediency. So support it. In order to skip validation of these fields, the plugin requires that the value start with the string "FIXME". This helps ensure that reports have a consistent way to detect and warn about unfilled values in flagged transactions. --- conservancy_beancount/plugin/core.py | 14 +++++++++++--- conservancy_beancount/plugin/meta_paypal_id.py | 2 +- .../plugin/meta_tax_implication.py | 4 ---- setup.py | 2 +- tests/test_meta_expense_type.py | 10 ++++++++++ tests/test_meta_income_type.py | 10 ++++++++++ tests/test_meta_paypal_id.py | 12 ++++++++++++ tests/test_meta_project.py | 10 ++++++++++ tests/test_meta_tax_implication.py | 11 ++++++++++- tests/testutil.py | 6 ++++++ 10 files changed, 71 insertions(+), 10 deletions(-) diff --git a/conservancy_beancount/plugin/core.py b/conservancy_beancount/plugin/core.py index c7bb566..b407455 100644 --- a/conservancy_beancount/plugin/core.py +++ b/conservancy_beancount/plugin/core.py @@ -160,6 +160,13 @@ class _PostingHook(TransactionHook, metaclass=abc.ABCMeta): def __init_subclass__(cls) -> None: cls.HOOK_GROUPS = cls.HOOK_GROUPS.union(['posting']) + def _is_flagged_fixme(self, post: data.Posting, value: MetaValue) -> bool: + return ( + post.meta.txn.flag == '!' + and isinstance(value, str) + and value.startswith('FIXME') + ) + def _run_on_post(self, txn: Transaction, post: data.Posting) -> bool: return True @@ -205,9 +212,10 @@ class _NormalizePostingMetadataHook(_PostingHook): try: set_value = self.VALUES_ENUM[source_value] except KeyError: - error = errormod.InvalidMetadataError( - txn, self.METADATA_KEY, source_value, post, - ) + if not self._is_flagged_fixme(post, source_value): + error = errormod.InvalidMetadataError( + txn, self.METADATA_KEY, source_value, post, + ) if error is None: post.meta[self.METADATA_KEY] = set_value else: diff --git a/conservancy_beancount/plugin/meta_paypal_id.py b/conservancy_beancount/plugin/meta_paypal_id.py index 209515f..dd11f39 100644 --- a/conservancy_beancount/plugin/meta_paypal_id.py +++ b/conservancy_beancount/plugin/meta_paypal_id.py @@ -39,5 +39,5 @@ class MetaPayPalID(core._PostingHook): match = regexp.match(value) # type:ignore[arg-type] except TypeError: match = None - if match is None: + if match is None and not self._is_flagged_fixme(post, value): yield errormod.InvalidMetadataError(txn, self.METADATA_KEY, value, post) diff --git a/conservancy_beancount/plugin/meta_tax_implication.py b/conservancy_beancount/plugin/meta_tax_implication.py index a0e7b28..8a4a829 100644 --- a/conservancy_beancount/plugin/meta_tax_implication.py +++ b/conservancy_beancount/plugin/meta_tax_implication.py @@ -59,10 +59,6 @@ class MetaTaxImplication(core._NormalizePostingMetadataHook): VALUES_ENUM = core.MetadataEnum('tax-implication', _STDNAMES, _ALIASES) del _STDNAMES, _ALIASES - # 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() diff --git a/setup.py b/setup.py index c51ddc4..5756e9c 100755 --- a/setup.py +++ b/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.16.1', + version='1.16.2', author='Software Freedom Conservancy', author_email='info@sfconservancy.org', license='GNU AGPLv3+', diff --git a/tests/test_meta_expense_type.py b/tests/test_meta_expense_type.py index 8e552ec..c1ca527 100644 --- a/tests/test_meta_expense_type.py +++ b/tests/test_meta_expense_type.py @@ -157,3 +157,13 @@ def test_flagged_txn_checked(hook, src_value): errors = list(hook.run(txn)) assert errors testutil.check_post_meta(txn, None, {TEST_KEY: src_value}) + +@pytest.mark.parametrize('src_value', testutil.FIXME_VALUES) +def test_flagged_fixme_ok(hook, src_value): + txn = testutil.Transaction(flag='!', postings=[ + ('Assets:Cash', -25), + ('Expenses:General', 25, {TEST_KEY: src_value}), + ]) + errors = list(hook.run(txn)) + assert not errors + testutil.check_post_meta(txn, None, {TEST_KEY: src_value}) diff --git a/tests/test_meta_income_type.py b/tests/test_meta_income_type.py index bf922f4..d8171a9 100644 --- a/tests/test_meta_income_type.py +++ b/tests/test_meta_income_type.py @@ -151,3 +151,13 @@ def test_flagged_txn_checked(hook, src_value): errors = list(hook.run(txn)) assert errors testutil.check_post_meta(txn, None, {TEST_KEY: src_value}) + +@pytest.mark.parametrize('src_value', testutil.FIXME_VALUES) +def test_flagged_fixme_ok(hook, src_value): + txn = testutil.Transaction(flag='!', postings=[ + ('Assets:Cash', 25), + ('Income:Other', -25, {TEST_KEY: src_value}), + ]) + errors = list(hook.run(txn)) + assert not errors + testutil.check_post_meta(txn, None, {TEST_KEY: src_value}) diff --git a/tests/test_meta_paypal_id.py b/tests/test_meta_paypal_id.py index 913ea56..05407cb 100644 --- a/tests/test_meta_paypal_id.py +++ b/tests/test_meta_paypal_id.py @@ -192,3 +192,15 @@ def test_still_required_on_flagged_txn(hook): ('Income:Donations', -1000), ]) assert list(hook.run(txn)) + +@pytest.mark.parametrize('account,src_value', testutil.combine_values( + ACCOUNTS, + testutil.FIXME_VALUES, +)) +def test_flagged_fixme_ok(hook, account, src_value): + txn = testutil.Transaction(flag='!', postings=[ + (account, 200, {TEST_KEY: src_value}), + ('Income:Donations', -200), + ]) + assert not list(hook.run(txn)) + testutil.check_post_meta(txn, {TEST_KEY: src_value}, None) diff --git a/tests/test_meta_project.py b/tests/test_meta_project.py index 3423fab..9c58b46 100644 --- a/tests/test_meta_project.py +++ b/tests/test_meta_project.py @@ -201,3 +201,13 @@ def test_still_required_on_flagged_txn(hook, src_value): errors = list(hook.run(txn)) assert errors testutil.check_post_meta(txn, None, None) + +@pytest.mark.parametrize('src_value', testutil.FIXME_VALUES) +def test_flagged_fixme_ok(hook, src_value): + txn = testutil.Transaction(flag='!', postings=[ + ('Assets:Cash', -25), + ('Expenses:General', 25, {TEST_KEY: src_value}), + ]) + errors = list(hook.run(txn)) + assert not errors + testutil.check_post_meta(txn, None, {TEST_KEY: src_value}) diff --git a/tests/test_meta_tax_implication.py b/tests/test_meta_tax_implication.py index 9c2aa31..520092d 100644 --- a/tests/test_meta_tax_implication.py +++ b/tests/test_meta_tax_implication.py @@ -137,9 +137,18 @@ def test_validation_only_in_date_range(hook, date, need_value): testutil.check_post_meta(txn, None, None) @pytest.mark.parametrize('src_value', INVALID_VALUES) -def test_flagged_txn_skipped(hook, src_value): +def test_flagged_txn_checked(hook, src_value): txn = testutil.Transaction(flag='!', **{TEST_KEY: src_value}, postings=[ ('Liabilities:Payable:Accounts', 25), ('Assets:Cash', -25), ]) + assert list(hook.run(txn)) + +@pytest.mark.parametrize('src_value', testutil.FIXME_VALUES) +def test_flagged_fixme_ok(hook, src_value): + txn = testutil.Transaction(flag='!', postings=[ + ('Liabilities:Payable:Accounts', 25), + ('Assets:Cash', -25, {TEST_KEY: src_value}), + ]) assert not list(hook.run(txn)) + testutil.check_post_meta(txn, None, {TEST_KEY: src_value}) diff --git a/tests/testutil.py b/tests/testutil.py index effca5c..a460ad6 100644 --- a/tests/testutil.py +++ b/tests/testutil.py @@ -194,6 +194,12 @@ NON_STRING_METADATA_VALUES = [ Amount(500, None), ] +FIXME_VALUES = [ + 'FIXME', + 'FIXME loose comment', + 'FIXME: comment with punctuation', +] + OPENING_EQUITY_ACCOUNTS = itertools.cycle([ 'Equity:Funds:Unrestricted', 'Equity:Funds:Restricted',