accrual: Better error handling and reporting around payment-method.

conservancy_beancount/reports/accrual.py

@@ -466,6 +466,44 @@ class OutgoingReport(BaseReport):
         else:
             return parsed
 
+    def _get_payment_method(self, posts: AccrualPostings, ticket_id: str) -> Optional[str]:
+        payment_methods = posts.meta_values('payment-method')
+        payment_methods.discard(None)
+        if all(isinstance(s, str) for s in payment_methods):
+            # type:ignore for <https://github.com/python/mypy/issues/7853>
+            payment_methods = {s.strip().lower() for s in payment_methods}  # type:ignore[union-attr]
+        log_prefix = f"cannot set payment-method for rt:{ticket_id}:"
+        payment_method_count = len(payment_methods)
+        if payment_method_count != 1:
+            self.logger.warning("%s %s metadata values found",
+                                log_prefix, payment_method_count)
+            return None
+        payment_method = payment_methods.pop()
+        if not isinstance(payment_method, str):
+            self.logger.warning("%s %r is not a string value",
+                                log_prefix, payment_method)
+            return None
+        try:
+            currency, method_key = payment_method.split(None, 1)
+        except ValueError:
+            self.logger.warning("%s no method specified in %r",
+                                log_prefix, payment_method)
+            return None
+        curr_match = re.fullmatch(r'[a-z]{3}', currency)
+        if curr_match is None:
+            self.logger.warning("%s invalid currency %r",
+                                log_prefix, currency)
+        try:
+            method_enum = self.PaymentMethods[method_key]
+        except KeyError:
+            self.logger.warning("%s invalid method %r",
+                                log_prefix, method_key)
+            curr_match = None
+        if curr_match is None:
+            return None
+        else:
+            return f'{currency.upper()} {method_enum.value}'
+
     def _report(self, posts: AccrualPostings, index: int) -> Iterable[str]:
         try:
             ticket_id, _ = self._primary_rt_id(posts)
@@ -543,41 +581,10 @@ class OutgoingReport(BaseReport):
 
         cf_targets = {
             'payment-amount': payment_amount,
+            'payment-method': (self._get_payment_method(posts, ticket_id)
+                               or ticket.get('CF.{payment-method}')),
             'payment-to': payment_to,
         }
-        payment_methods = filters.iter_unique(
-            post.meta['payment-method'].lower()
-            for post in posts
-            if isinstance(post.meta.get('payment-method'), str)
-        )
-        payment_method: Optional[str] = next(payment_methods, None)
-        if payment_method is None:
-            payment_method_count = "no"
-        elif next(payment_methods, None) is None:
-            pass
-        else:
-            payment_method_count = "multiple"
-            payment_method = None
-        if payment_method is None:
-            self.logger.warning(
-                "cannot set payment-method for rt:%s: %s metadata values found",
-                ticket_id, payment_method_count,
-            )
-        else:
-            currency, method_key = payment_method.lower().split(None, 1)
-            try:
-                method_enum = self.PaymentMethods[method_key]
-            except KeyError:
-                match: Optional[Match] = None
-            else:
-                match = re.fullmatch(r'[a-z]{3}', currency)
-            if match is None:
-                self.logger.warning(
-                    "cannot set payment-method for rt:%s: invalid value %r",
-                    ticket_id, payment_method,
-                )
-            else:
-                cf_targets['payment-method'] = f'{currency.upper()} {method_enum.value}'
 
         cf_updates = {
             f'CF_{key}': value

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.5.7',
+    version='1.5.8',
     author='Software Freedom Conservancy',
     author_email='info@sfconservancy.org',
     license='GNU AGPLv3+',

tests/test_reports_accrual.py

@@ -566,6 +566,46 @@ def test_outgoing_report_multi_invoice(accrual_postings, caplog):
     }}
     assert 'payment-method:' not in output.getvalue()
 
+@pytest.mark.parametrize('arg', [
+    'usd ach',
+    '  eur  wire',
+    'cad   vendorportal  ',
+    ' gbp check ',
+])
+def test_outgoing_report_good_payment_method(caplog, accrual_postings, arg):
+    rt_id = 'rt:40'
+    meta = {'rt-id': rt_id, 'invoice': 'rt:40/100', 'payment-method': arg}
+    txn = testutil.Transaction(postings=[
+        ('Liabilities:Payable:Accounts', -100, meta),
+    ])
+    rt_client = RTClient()
+    run_outgoing(rt_id, data.Posting.from_txn(txn), rt_client)
+    assert not caplog.records
+    cf_values = rt_client.edits[rt_id[3:]]['CF_payment-method'].split()
+    assert cf_values[0] == arg.split()[0].upper()
+    assert len(cf_values) > 1
+
+@pytest.mark.parametrize('arg', [
+    '',
+    'usd',
+    'usd nonexistent',
+    'check',
+    'us check',
+])
+def test_outgoing_report_bad_payment_method(caplog, accrual_postings, arg):
+    rt_id = 'rt:40'
+    meta = {'rt-id': rt_id, 'invoice': 'rt:40/100', 'payment-method': arg}
+    txn = testutil.Transaction(postings=[
+        ('Liabilities:Payable:Accounts', -100, meta),
+    ])
+    rt_client = RTClient()
+    run_outgoing(rt_id, data.Posting.from_txn(txn), rt_client)
+    assert caplog.records
+    for log in caplog.records:
+        assert log.levelname == 'WARNING'
+        assert log.message.startswith(f'cannot set payment-method for {rt_id}: ')
+    assert 'CF_payment-method' not in rt_client.edits[rt_id[3:]]
+
 def test_outgoing_report_without_rt_id(accrual_postings, caplog):
     invoice = 'rt://ticket/515/attachments/5150'
     related = accruals_by_meta(