From 6658696d06873a65807b58f8056fa808aa6d7d93 2020-04-04 14:54:08 From: Brett Smith Date: 2020-04-04 14:54:08 Subject: [PATCH] meta_receipt: Use check-id as fallback metadata for outgoing checks. When we send checks, we don't have a check document anywhere (for security reasons), we just note the check number. Update the validation to match. RT#10507. --- diff --git a/conservancy_beancount/plugin/meta_receipt.py b/conservancy_beancount/plugin/meta_receipt.py index a524656e8c85b7cc10a30b6d8f493b9a308dcaab..54cbae31c1b1eb5b035296acf753c357e5197b92 100644 --- a/conservancy_beancount/plugin/meta_receipt.py +++ b/conservancy_beancount/plugin/meta_receipt.py @@ -14,14 +14,23 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from decimal import Decimal + from . import core from .. import config as configmod from .. import data from .. import errors as errormod from ..beancount_types import ( + MetaKey, Transaction, ) +from typing import ( + Callable, +) + +_CheckMethod = Callable[[Transaction, data.Posting, MetaKey], None] + class MetaReceipt(core._RequireLinksPostingMetadataHook): METADATA_KEY = 'receipt' @@ -35,6 +44,13 @@ 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: + value = post.meta.get(key) + if (not isinstance(value, Decimal) + or value < 1 + or value % 1): + raise errormod.InvalidMetadataError(txn, key, value, post, Decimal) + def post_run(self, txn: Transaction, post: data.Posting) -> errormod.Iter: try: self._check_links(txn, post, self.METADATA_KEY) @@ -50,8 +66,13 @@ class MetaReceipt(core._RequireLinksPostingMetadataHook): else: post_amount = -1 + check_method: _CheckMethod = self._check_links if post.account.is_checking(): - fallback_key = 'check' + if post_amount == -1: + check_method = self._check_check_id + fallback_key = 'check-id' + else: + fallback_key = 'check' elif post.account.is_credit_card() and post_amount == -1: fallback_key = 'invoice' elif post.account.is_under('Assets:PayPal') and post_amount == 1: @@ -61,7 +82,7 @@ class MetaReceipt(core._RequireLinksPostingMetadataHook): return try: - self._check_links(txn, post, fallback_key) + 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( diff --git a/tests/test_meta_receipt.py b/tests/test_meta_receipt.py index 82a1b021fc9caaf64de76d8135532d02610fca63..519a31345650ed4b5914c936d318facc0ce0be82 100644 --- a/tests/test_meta_receipt.py +++ b/tests/test_meta_receipt.py @@ -48,17 +48,21 @@ class AccountForTesting(typing.NamedTuple): return f"{self.name} missing {TEST_KEY}{rest}" def wrong_type_message(self, wrong_value, key=TEST_KEY): - return "{} has wrong type of {}: expected str but is a {}".format( + expect_type = 'Decimal' if key == 'check-id' else 'str' + return "{} has wrong type of {}: expected {} but is a {}".format( self.name, key, + expect_type, type(wrong_value).__name__, ) ACCOUNTS = [AccountForTesting._make(t) for t in [ - ('Assets:Bank:CheckCard', PostType.BOTH, 'check'), + ('Assets:Bank:CheckCard', PostType.CREDIT, 'check'), + ('Assets:Bank:CheckCard', PostType.DEBIT, 'check-id'), ('Assets:Cash', PostType.BOTH, None), - ('Assets:Checking', PostType.BOTH, 'check'), + ('Assets:Checking', PostType.CREDIT, 'check'), + ('Assets:Checking', PostType.DEBIT, 'check-id'), ('Assets:PayPal', PostType.CREDIT, 'paypal-id'), ('Assets:PayPal', PostType.DEBIT, None), ('Assets:Savings', PostType.BOTH, None), @@ -66,9 +70,12 @@ ACCOUNTS = [AccountForTesting._make(t) for t in [ ('Liabilities:CreditCard', PostType.DEBIT, 'invoice'), ]] -ACCOUNTS_WITH_FALLBACKS = [acct for acct in ACCOUNTS if acct.fallback_meta] +ACCOUNTS_WITH_LINK_FALLBACK = [acct for acct in ACCOUNTS + if acct.fallback_meta and acct.fallback_meta != 'check-id'] +ACCOUNTS_WITH_CHECK_ID_FALLBACK = [acct for acct in ACCOUNTS + if acct.fallback_meta == 'check-id'] ACCOUNTS_WITHOUT_FALLBACKS = [acct for acct in ACCOUNTS if not acct.fallback_meta] -KNOWN_FALLBACKS = {acct.fallback_meta for acct in ACCOUNTS_WITH_FALLBACKS} +KNOWN_FALLBACKS = {acct.fallback_meta for acct in ACCOUNTS if acct.fallback_meta} # These are mostly fill-in values. # We don't need to run every test on every value for these, just enough to @@ -86,6 +93,14 @@ NOT_REQUIRED_ACCOUNTS = itertools.cycle([ 'Liabilities:UnearnedIncome:Donations', ]) +CHECK_IDS = (decimal.Decimal(n) for n in itertools.count(1)) +def BAD_CHECK_IDS(): + # Valid check-id values are positive integers + yield decimal.Decimal(0) + yield -next(CHECK_IDS) + yield next(CHECK_IDS) * decimal.Decimal('1.1') +BAD_CHECK_IDS = BAD_CHECK_IDS() + def check(hook, test_acct, other_acct, expected, *, txn_meta={}, post_meta={}, check_type=PostType.BOTH, min_amt=0): check_type &= test_acct.required_types @@ -167,7 +182,7 @@ def test_bad_type_receipt_on_txn(hook, test_acct, other_acct, value): txn_meta={TEST_KEY: value}) @pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( - ACCOUNTS_WITH_FALLBACKS, + ACCOUNTS_WITH_LINK_FALLBACK, NOT_REQUIRED_ACCOUNTS, testutil.LINK_METADATA_STRINGS, )) @@ -176,7 +191,7 @@ def test_valid_fallback_on_post(hook, test_acct, other_acct, value): post_meta={test_acct.fallback_meta: value}) @pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( - ACCOUNTS_WITH_FALLBACKS, + ACCOUNTS_WITH_LINK_FALLBACK, NOT_REQUIRED_ACCOUNTS, testutil.NON_LINK_METADATA_STRINGS, )) @@ -185,7 +200,7 @@ def test_invalid_fallback_on_post(hook, test_acct, other_acct, value): post_meta={test_acct.fallback_meta: value}) @pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( - ACCOUNTS_WITH_FALLBACKS, + ACCOUNTS_WITH_LINK_FALLBACK, NOT_REQUIRED_ACCOUNTS, testutil.NON_STRING_METADATA_VALUES, )) @@ -198,7 +213,7 @@ def test_bad_type_fallback_on_post(hook, test_acct, other_acct, value): post_meta={test_acct.fallback_meta: value}) @pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( - ACCOUNTS_WITH_FALLBACKS, + ACCOUNTS_WITH_LINK_FALLBACK, NOT_REQUIRED_ACCOUNTS, testutil.LINK_METADATA_STRINGS, )) @@ -207,7 +222,7 @@ def test_valid_fallback_on_txn(hook, test_acct, other_acct, value): txn_meta={test_acct.fallback_meta: value}) @pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( - ACCOUNTS_WITH_FALLBACKS, + ACCOUNTS_WITH_LINK_FALLBACK, NOT_REQUIRED_ACCOUNTS, testutil.NON_LINK_METADATA_STRINGS, )) @@ -216,7 +231,7 @@ def test_invalid_fallback_on_txn(hook, test_acct, other_acct, value): txn_meta={test_acct.fallback_meta: value}) @pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( - ACCOUNTS_WITH_FALLBACKS, + ACCOUNTS_WITH_LINK_FALLBACK, NOT_REQUIRED_ACCOUNTS, testutil.NON_STRING_METADATA_VALUES, )) @@ -228,6 +243,80 @@ def test_bad_type_fallback_on_txn(hook, test_acct, other_acct, value): check(hook, test_acct, other_acct, expected, txn_meta={test_acct.fallback_meta: value}) +@pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( + ACCOUNTS_WITH_CHECK_ID_FALLBACK, + NOT_REQUIRED_ACCOUNTS, + CHECK_IDS, +)) +def test_valid_check_id_on_post(hook, test_acct, other_acct, value): + check(hook, test_acct, other_acct, None, + post_meta={test_acct.fallback_meta: value}) + +@pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( + ACCOUNTS_WITH_CHECK_ID_FALLBACK, + NOT_REQUIRED_ACCOUNTS, + BAD_CHECK_IDS, +)) +def test_invalid_check_id_on_post(hook, test_acct, other_acct, value): + expected = { + test_acct.missing_message(False), + f"{test_acct.name} has invalid {test_acct.fallback_meta}: {value}", + } + check(hook, test_acct, other_acct, expected, + post_meta={test_acct.fallback_meta: value}) + +@pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( + ACCOUNTS_WITH_CHECK_ID_FALLBACK, + NOT_REQUIRED_ACCOUNTS, + testutil.NON_STRING_METADATA_VALUES, +)) +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.wrong_type_message(value, test_acct.fallback_meta), + } + check(hook, test_acct, other_acct, expected, + post_meta={test_acct.fallback_meta: value}) + +@pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( + ACCOUNTS_WITH_CHECK_ID_FALLBACK, + NOT_REQUIRED_ACCOUNTS, + CHECK_IDS, +)) +def test_valid_check_id_on_txn(hook, test_acct, other_acct, value): + check(hook, test_acct, other_acct, None, + txn_meta={test_acct.fallback_meta: value}) + +@pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( + ACCOUNTS_WITH_CHECK_ID_FALLBACK, + NOT_REQUIRED_ACCOUNTS, + BAD_CHECK_IDS, +)) +def test_invalid_check_id_on_txn(hook, test_acct, other_acct, value): + expected = { + test_acct.missing_message(False), + f"{test_acct.name} has invalid {test_acct.fallback_meta}: {value}", + } + check(hook, test_acct, other_acct, expected, + txn_meta={test_acct.fallback_meta: value}) + +@pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values( + ACCOUNTS_WITH_CHECK_ID_FALLBACK, + NOT_REQUIRED_ACCOUNTS, + testutil.NON_STRING_METADATA_VALUES, +)) +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.wrong_type_message(value, test_acct.fallback_meta), + } + check(hook, test_acct, other_acct, expected, + txn_meta={test_acct.fallback_meta: value}) + @pytest.mark.parametrize('test_acct,other_acct,key,value', testutil.combine_values( ACCOUNTS_WITHOUT_FALLBACKS, NOT_REQUIRED_ACCOUNTS,