From fdd9f2847b787b951984e3d7340cd341ceb88750 2021-01-29 14:38:37 From: Brett Smith Date: 2021-01-29 14:38:37 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. --- diff --git a/conservancy_beancount/plugin/core.py b/conservancy_beancount/plugin/core.py index c7bb566827ab1e3a590a76dd4fc1a633e1bec805..b4074551d4d6b9c5782b5767d5f26f37a4ccff26 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 209515f441e1ed6df30fd6e872911ec1f96cd4c9..dd11f39062b617aa287b078e542600d12fd4e79c 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 a0e7b286e3231f68f22bcb9630f9a44f0d1abb7f..8a4a82974f781e6e3f6607803a83741d6b94f1e5 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 c51ddc43d03d1a2f8996da2d833b151cd42d559e..5756e9ccaf65dbc0357e5b31d8c4f8c180d606d4 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 8e552ec925544b6cc684cece3c43d7c3d9957906..c1ca5273f349947a77f77c956f519fbaffc42afc 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 bf922f4548d393d4dba66ad5d7ef4a7b72f97e99..d8171a9c635c706cdd823e05468b56756dd19e62 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 913ea56a25a617c933763165d0efaf223f3ff84b..05407cb3ba5eaf80d34e90ed99a2b93fa2ca4ddf 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 3423fab01fbae75bfd835a6ad4166a58167eb000..9c58b469c0580ac8092bcffd6ae7207af83e1416 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 9c2aa31309bee6ce305d79a9cdd831ff78e919bb..520092db0fc82f4ec25202fafae3992a1e62cb54 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 effca5c50e1a6d7bf0d58ef8c159a48826b84496..a460ad6dea258dc5114ae88a3ce1544ccccc3a81 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',