NPO-Accounting/conservancy_beancount Changeset - fdb62dd1c641

Changeset - fdb62dd1c641

Parent rev.

Child rev.

[Not reviewed]

0 7 0

Brett Smith - 4 years ago 2020-04-07 02:02:14
brettcsmith@brettcsmith.org

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.

7 files changed with 73 insertions and 103 deletions:

conservancy_beancount/plugin/core.py

conservancy_beancount/plugin/meta_approval.py

conservancy_beancount/plugin/meta_invoice.py

conservancy_beancount/plugin/meta_receipt.py

conservancy_beancount/plugin/meta_receivable_documentation.py

tests/test_meta_invoice.py

tests/test_meta_receipt.py

0 comments (0 inline, 0 general)

conservancy_beancount/plugin/core.py

➞

Show inline comments

@@ ... / @@ -22,24 +22,25 @@ from .. import config as configmod @@
 from .. import data
 from .. import errors as errormod
 from typing import (
     Any,
     Dict,
     FrozenSet,
     Generic,
     Iterable,
     Iterator,
     Mapping,
     Optional,
     Sequence,
     Type,
     TypeVar,
+)
 from ..beancount_types import (
     Account,
     Directive,
     MetaKey,
     MetaValue,
     MetaValueEnum,
     Transaction,
+)
@@ ... / @@ -234,35 +235,38 @@ class _NormalizePostingMetadataHook(_PostingHook): @@
                 error = errormod.InvalidMetadataError(
                     txn, self.METADATA_KEY, source_value, post,
+                )
         if error is None:
             post.meta[self.METADATA_KEY] = set_value
         else:
             yield error
 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.
     # Most subclasses only need to define METADATA_KEY and _run_on_post.
     METADATA_KEY: str
     # under one of the metadata keys listed in CHECKED_METADATA.
     # Most subclasses only need to define CHECKED_METADATA and _run_on_post.
     CHECKED_METADATA: Sequence[MetaKey]
     def __init_subclass__(cls) -> None:
         super().__init_subclass__()
         cls.HOOK_GROUPS = cls.HOOK_GROUPS.union(['metadata', cls.METADATA_KEY])
     def _check_links(self, txn: Transaction, post: data.Posting, key: MetaKey) -> None:
         try:
             problem = not post.meta.get_links(key)
             value = None
         except TypeError:
             problem = True
             value = post.meta[key]
         if problem:
             raise errormod.InvalidMetadataError(txn, key, value, post)
         cls.HOOK_GROUPS = cls.HOOK_GROUPS.union(cls.CHECKED_METADATA).union('metadata')
     def _check_metadata(self,
                         txn: Transaction,
                         post: data.Posting,
                         keys: Sequence[MetaKey],
     ) -> Iterable[errormod.InvalidMetadataError]:
         have_docs = False
         for key in keys:
             try:
                 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:
             self._check_links(txn, post, self.METADATA_KEY)
         except errormod.Error as error:
             yield error
         return self._check_metadata(txn, post, self.CHECKED_METADATA)

conservancy_beancount/plugin/meta_approval.py

➞

Show inline comments

@@ ... / @@ -16,26 +16,25 @@ @@
 import decimal
 from . import core
 from .. import config as configmod
 from .. import data
 from .. import errors as errormod
 from ..beancount_types import (
     Transaction,
+)
 class MetaApproval(core._RequireLinksPostingMetadataHook):
     METADATA_KEY = 'approval'
     CREDIT_CARD_ACCT = 'Liabilities:CreditCard'
     CHECKED_METADATA = ['approval']
     def __init__(self, config: configmod.Config) -> None:
         self.payment_threshold = config.payment_threshold()
     def _run_on_txn(self, txn: Transaction) -> bool:
         if not super()._run_on_txn(txn):
             return False
         debits_sum = decimal.Decimal(0)
         for post in data.iter_postings(txn):
             # approval is required:
             # 1. When a payable is accrued
             if (post.account.is_under('Liabilities:Payable:Accounts')

conservancy_beancount/plugin/meta_invoice.py

➞

Show inline comments

@@ ... / @@ -14,19 +14,19 @@ @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 from . import core
 from .. import data
 from .. import errors as errormod
 from ..beancount_types import (
     MetaValueEnum,
     Transaction,
+)
 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(
             'Assets:Receivable',
             'Liabilities:Payable',
         ) is not None

conservancy_beancount/plugin/meta_receipt.py

➞

Show inline comments

@@ ... / @@ -20,67 +20,57 @@ 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'
+    CHECKED_METADATA = ['receipt']
     def __init__(self, config: configmod.Config) -> None:
         self.payment_threshold = abs(config.payment_threshold())
     def _run_on_post(self, txn: Transaction, post: data.Posting) -> bool:
         return (
             (post.account.is_cash_equivalent() or post.account.is_credit_card())
             and post.units.number is not None
             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:
     def _run_checking_debit(self, txn: Transaction, post: data.Posting) -> errormod.Iter:
         receipt_errors = list(self._check_metadata(txn, post, self.CHECKED_METADATA))
         if not receipt_errors:
             return
         for error in receipt_errors:
             if error.value is not None:
                 yield error
         try:
             self._check_links(txn, post, self.METADATA_KEY)
         except errormod.InvalidMetadataError as error:
             receipt_error = error
             check_id = post.meta['check-id']
         except KeyError:
             check_id_ok = False
         else:
             return
             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)
         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'
     def post_run(self, txn: Transaction, post: data.Posting) -> errormod.Iter:
         keys = list(self.CHECKED_METADATA)
         is_checking = post.account.is_checking()
         if is_checking and post.is_debit():
             return self._run_checking_debit(txn, post)
         elif is_checking:
             keys.append('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'
         else:
             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
             keys.append('paypal-id')
         return self._check_metadata(txn, post, keys)

conservancy_beancount/plugin/meta_receivable_documentation.py

➞

Show inline comments

@@ ... / @@ -23,30 +23,25 @@ from .. import errors as errormod @@
 from ..beancount_types import (
     MetaKey,
     Transaction,
+)
 from typing import (
     Dict,
     Optional,
+)
 class MetaReceivableDocumentation(core._RequireLinksPostingMetadataHook):
     HOOK_GROUPS = frozenset(['network', 'rt'])
     SUPPORTING_METADATA = frozenset([
         'approval',
         'contract',
         'purchase-order',
     ])
     METADATA_KEY = '/'.join(sorted(SUPPORTING_METADATA))
     CHECKED_METADATA = ['approval', 'contract', 'purchase-order']
     # 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`
     # This regexp matches both, with a little slack to try to reduce the false
     # negative rate due to minor renames, etc.
     ISSUED_INVOICE_RE = re.compile(
         r'[Ii]nvoice[-_ ]*(?:2[0-9]{9,}|30[0-9]+)[A-Za-z]*[-_ .]',
+    )
     def __init__(self, config: configmod.Config) -> None:
         rt_wrapper = config.rt_wrapper()
         # In principle, we could still check for non-RT invoices and enforce
@@ ... @@
         # Get the first invoice, or return False if it doesn't exist.
         try:
             invoice_link = post.meta.get_links('invoice')[0]
         except (IndexError, TypeError):
             return False
         # Get the filename, following an RT link if necessary.
         rt_args = self.rt.parse(invoice_link)
         if rt_args is not None:
             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

➞

Show inline comments

@@ ... / @@ -28,24 +28,27 @@ REQUIRED_ACCOUNTS = { @@
+}
 NON_REQUIRED_ACCOUNTS = {
     'Assets:Cash',
     'Equity:OpeningBalance',
     'Expenses:Other',
     'Income:Other',
     'Liabilities:CreditCard',
+}
 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()
     return meta_invoice.MetaInvoice(config)
 @pytest.mark.parametrize('acct1,acct2,value', testutil.combine_values(
     REQUIRED_ACCOUNTS,
     NON_REQUIRED_ACCOUNTS,
     testutil.LINK_METADATA_STRINGS,
 ))
 def test_valid_values_on_postings(hook, acct1, acct2, value):
     txn = testutil.Transaction(postings=[
@@ ... @@
     assert actual == {"{} missing {}".format(acct1, TEST_KEY)}
 @pytest.mark.parametrize('acct1,acct2,value', testutil.combine_values(
     REQUIRED_ACCOUNTS,
     NON_REQUIRED_ACCOUNTS,
     testutil.NON_STRING_METADATA_VALUES,
 ))
 def test_bad_type_values_on_postings(hook, acct1, acct2, value):
     txn = testutil.Transaction(postings=[
         (acct2, -25),
         (acct1, 25, {TEST_KEY: value}),
     ])
     expected_msg = "{} has wrong type of {}: expected str but is a {}".format(
         acct1,
         TEST_KEY,
         type(value).__name__,
+    )
     expected = {
         MISSING_MSG(acct1),
         WRONG_TYPE_MSG(acct1, 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,
     NON_REQUIRED_ACCOUNTS,
     testutil.LINK_METADATA_STRINGS,
 ))
 def test_valid_values_on_transaction(hook, acct1, acct2, value):
     txn = testutil.Transaction(**{TEST_KEY: value}, postings=[
         (acct2, -25),
         (acct1, 25),
     ])
     assert not list(hook.run(txn))
@@ ... @@
     assert actual == {"{} missing {}".format(acct1, TEST_KEY)}
 @pytest.mark.parametrize('acct1,acct2,value', testutil.combine_values(
     REQUIRED_ACCOUNTS,
     NON_REQUIRED_ACCOUNTS,
     testutil.NON_STRING_METADATA_VALUES,
 ))
 def test_bad_type_values_on_transaction(hook, acct1, acct2, value):
     txn = testutil.Transaction(**{TEST_KEY: value}, postings=[
         (acct2, -25),
         (acct1, 25),
     ])
     expected_msg = "{} has wrong type of {}: expected str but is a {}".format(
         acct1,
         TEST_KEY,
         type(value).__name__,
+    )
     expected = {
         MISSING_MSG(acct1),
         WRONG_TYPE_MSG(acct1, 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,
     NON_REQUIRED_ACCOUNTS,
 ))
 def test_missing_invoice(hook, acct1, acct2):
     txn = testutil.Transaction(postings=[
         (acct2, -25),
         (acct1, 25),
     ])
     actual = {error.message for error in hook.run(txn)}
     assert actual == {"{} missing {}".format(acct1, TEST_KEY)}

tests/test_meta_receipt.py

➞

Show inline comments

@@ ... / @@ -35,25 +35,25 @@ class PostType(enum.IntFlag): @@
     BOTH = CREDIT | DEBIT
 class AccountForTesting(typing.NamedTuple):
     name: str
     required_types: PostType
     fallback_meta: typing.Optional[str]
     def missing_message(self, include_fallback=True):
         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):
         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__,
+        )
@@ ... @@
 ))
 def test_invalid_fallback_on_post(hook, test_acct, other_acct, value):
     check(hook, test_acct, other_acct, {test_acct.missing_message()},
           post_meta={test_acct.fallback_meta: value})
 @pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values(
     ACCOUNTS_WITH_LINK_FALLBACK,
     NOT_REQUIRED_ACCOUNTS,
     testutil.NON_STRING_METADATA_VALUES,
 ))
 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,
           post_meta={test_acct.fallback_meta: value})
 @pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values(
     ACCOUNTS_WITH_LINK_FALLBACK,
     NOT_REQUIRED_ACCOUNTS,
     testutil.LINK_METADATA_STRINGS,
 ))
 def test_valid_fallback_on_txn(hook, test_acct, other_acct, value):
     check(hook, test_acct, other_acct, None,
@@ ... @@
 ))
 def test_invalid_fallback_on_txn(hook, test_acct, other_acct, value):
     check(hook, test_acct, other_acct, {test_acct.missing_message()},
           txn_meta={test_acct.fallback_meta: value})
 @pytest.mark.parametrize('test_acct,other_acct,value', testutil.combine_values(
     ACCOUNTS_WITH_LINK_FALLBACK,
     NOT_REQUIRED_ACCOUNTS,
     testutil.NON_STRING_METADATA_VALUES,
 ))
 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,
           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),
         test_acct.missing_message(),
         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.missing_message(),
         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),
         test_acct.missing_message(),
         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.missing_message(),
         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,
     KNOWN_FALLBACKS,
     testutil.LINK_METADATA_STRINGS,
 ))
 def test_fallback_not_accepted_on_other_accounts(hook, test_acct, other_acct, key, value):
     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,
     testutil.LINK_METADATA_STRINGS,
 ))
 def test_fallback_on_zero_amount_postings(hook, test_acct, other_acct, value):
     # Unfortunately it does happen that we get donations that go 100% to
     # banking fees, and our importer writes a zero-amount posting to the
     # Assets account.
     txn = testutil.Transaction(postings=[
         ('Income:Donations', '-.1'),
         ('Expenses:BankingFees', '.1'),

0 comments (0 inline, 0 general)