From eff98f4e8f0ca6935ea2117cf16459ce79728b18 Mon Sep 17 00:00:00 2001
From: Pelle Koster <pelle.koster@geant.org>
Date: Fri, 21 Mar 2025 09:13:55 +0100
Subject: [PATCH] Add reporting command for visitors with invoices or invoices
 without visitors

---
 config-example.json                           |   2 +-
 docs/source/development.rst                   |   2 +-
 stripe_checkout/config.py                     |   2 +-
 stripe_checkout/settings/base.py              |   2 +-
 .../stripe_checkout/compare_visit_stripe.py   | 119 ++++++++++++++++++
 .../stripe_checkout/generate_report.py        | 114 +++++++++++------
 .../management/commands/generatereport.py     |   8 +-
 .../management/commands/processevents.py      |   2 +-
 .../management/commands/sendpossibleerrors.py |  58 +++++++++
 stripe_checkout/stripe_checkout/visit.py      |  12 +-
 test/conftest.py                              |   2 +-
 11 files changed, 269 insertions(+), 54 deletions(-)
 create mode 100644 stripe_checkout/stripe_checkout/compare_visit_stripe.py
 create mode 100644 stripe_checkout/stripe_checkout/management/commands/sendpossibleerrors.py

diff --git a/config-example.json b/config-example.json
index cd4f15b..962a994 100644
--- a/config-example.json
+++ b/config-example.json
@@ -5,5 +5,5 @@
   "STRIPE_TAX_RATE_ID": "txr_1QeddlDSpyjzuj5pPwUcMwTd",
   "VISIT_API_KEY": "<visit api key>",
   "VISIT_EXPO_ID": "18lm2fafttito",
-  "UNPROCESSED_PAYMENT_EMAIL_TO": []
+  "SEND_ERROR_EMAILS_TO": []
 }
\ No newline at end of file
diff --git a/docs/source/development.rst b/docs/source/development.rst
index 0919d94..7c2673c 100644
--- a/docs/source/development.rst
+++ b/docs/source/development.rst
@@ -124,7 +124,7 @@ Email
 
 It so happens postfix is configured on the ``stripe-checkout`` vm's to forward to the geant smtp
 servers. So we don't need much additional configuration. We only configure the `DEFAULT_FROM_EMAIL`
-setting. The recipients for these emails can be set using the ``UNPROCESSED_PAYMENT_EMAIL_TO``
+setting. The recipients for these emails can be set using the ``SEND_ERROR_EMAILS_TO``
 setting in the :ref:`config_json`
 
 .. _stripe_webhook_endpoint:
diff --git a/stripe_checkout/config.py b/stripe_checkout/config.py
index bda88c5..8d64fd9 100644
--- a/stripe_checkout/config.py
+++ b/stripe_checkout/config.py
@@ -13,7 +13,7 @@ CONFIG_SCHEMA = {
         "STRIPE_TAX_RATE_ID": {"type": "string"},
         "VISIT_API_KEY": {"type": "string"},
         "VISIT_EXPO_ID": {"type": "string"},
-        "UNPROCESSED_PAYMENT_EMAIL_TO": {
+        "SEND_ERROR_EMAILS_TO": {
             "type": "array",
             "items": {"type": "string"},
         },
diff --git a/stripe_checkout/settings/base.py b/stripe_checkout/settings/base.py
index 0338f49..3ac913f 100644
--- a/stripe_checkout/settings/base.py
+++ b/stripe_checkout/settings/base.py
@@ -132,4 +132,4 @@ LOGGING = {
 STRIPE_WEBHOOK_ALLOWED_IPS = ["*"]
 
 DEFAULT_FROM_EMAIL = "noreply@geant.org"
-UNPROCESSED_PAYMENT_EMAIL_TO = []
+SEND_ERROR_EMAILS_TO = []
diff --git a/stripe_checkout/stripe_checkout/compare_visit_stripe.py b/stripe_checkout/stripe_checkout/compare_visit_stripe.py
new file mode 100644
index 0000000..a6a0648
--- /dev/null
+++ b/stripe_checkout/stripe_checkout/compare_visit_stripe.py
@@ -0,0 +1,119 @@
+import stripe
+
+from stripe_checkout.stripe_checkout import visit
+from stripe_checkout.stripe_checkout.generate_report import (
+    CSVReporter,
+    ReportField,
+    Serializer,
+    StripeWrapper,
+)
+from stripe_checkout.stripe_checkout.utils import list_all_stripe_items
+
+
+class VisitorWrapper(StripeWrapper):
+    inner: visit.Visitor
+
+    def __init__(self, inner: visit.Visitor, all_objects):
+        self.all_visitors = all_objects["visit"]
+        super().__init__(inner, all_objects["stripe"])
+
+    @property
+    def invoice_count(self):
+        return len(
+            [
+                obj
+                for obj_id, obj in self.all_objects.items()
+                if obj_id.startswith("in_")
+                and obj.get("customer_email") == self.inner.email
+            ]
+        )
+
+    @classmethod
+    def iter_items(cls, all_objects):
+        for visitor in all_objects["visit"]:
+            yield cls(visitor, all_objects)
+
+
+class PossibleErrorReporter(CSVReporter):
+    def iter_report_items(self, data):
+        seen = set()
+        for obj in super().iter_report_items(data):
+            seen.add(obj["Email address"])
+            if self._is_suspect(obj):
+                yield obj
+        unseen_invoices: dict[str, list[dict]] = {}
+        for obj_id, obj in data["stripe"].items():
+            email = obj["customer_email"]
+            if obj_id.startswith("in_") and email not in seen:
+                unseen_invoices.setdefault(email, []).append(obj)
+        for email, invoices in unseen_invoices.items():
+            yield {
+                "Email address": email,
+                "Invoice Count": len(invoices),
+                "Paid": (
+                    "PAID"
+                    if any(invoice["status"] == "paid" for invoice in invoices)
+                    else ""
+                ),
+            }
+
+    @staticmethod
+    def _is_suspect(obj: dict):
+        if obj["Registration Type"] in {"VIP", "Staff"}:
+            return False
+        return obj["Invoice Count"] != 1
+
+
+def get_all_data(log_progress=None):
+    return {
+        "visit": get_all_visitors(log_progress=log_progress),
+        "stripe": get_all_invoices(log_progress=log_progress),
+    }
+
+
+def get_all_visitors(log_progress=None):
+    client = visit.VisitorAPI()
+    return sorted(
+        client.list_all_visitors(log_progress=log_progress),
+        key=lambda v: v.email.lower(),
+    )
+
+
+def get_all_invoices(log_progress=None):
+    return {
+        o["id"]: o
+        for o in list_all_stripe_items(stripe.Invoice, log_progress=log_progress)
+    }
+
+
+VISITOR_SERIALIZER = Serializer(
+    wrapper_cls=VisitorWrapper,
+    fields=[
+        ReportField("Email address", "email"),
+        ReportField("Visitor ID", "id"),
+        ReportField("Registration Type", "registrationType.name"),
+        ReportField("Registration State", "registrationState"),
+        ReportField("Invoice Count", "invoice_count"),
+        ReportField("Paid", "paid", transform=lambda v: "PAID" if v else ""),
+    ],
+)
+
+
+reporter = PossibleErrorReporter([VISITOR_SERIALIZER], get_data=get_all_data)
+
+
+def main():
+    import logging
+    import pathlib
+
+    logger = logging.getLogger()
+    logging.basicConfig(level=logging.INFO)
+
+    path = pathlib.Path.cwd() / "comparison.csv"
+    with open(path, "w") as file:
+        length = reporter.write_csv(file, log_progress=logger.info)
+    logger.info(f"Wrote {length} items to {path}")
+
+
+if __name__ == "__main__":
+    main()
diff --git a/stripe_checkout/stripe_checkout/generate_report.py b/stripe_checkout/stripe_checkout/generate_report.py
index dd6dbd5..c58403f 100644
--- a/stripe_checkout/stripe_checkout/generate_report.py
+++ b/stripe_checkout/stripe_checkout/generate_report.py
@@ -24,7 +24,7 @@ import datetime
 import io
 import itertools
 import re
-from typing import Any, Callable, Iterable, Optional, Type
+from typing import Any, Callable, Iterable, Optional, Sequence, Type
 
 import stripe
 
@@ -35,15 +35,13 @@ from stripe_checkout.stripe_checkout.utils import list_all_stripe_items
 class Serializer:
     wrapper_cls: Type[StripeWrapper]
     fields: Iterable[ReportField]
-    id_prefix: str
 
     def serialize(self, wrapper: StripeWrapper):
         return {field.display_name: field.get_value(wrapper) for field in self.fields}
 
     def iter_items(self, all_objects: dict[str, Any]):
-        for obj_id, obj in all_objects.items():
-            if obj_id.startswith(self.id_prefix):
-                yield self.serialize(self.wrapper_cls(obj, all_objects=all_objects))
+        for wrapped in self.wrapper_cls.iter_items(all_objects):
+            yield self.serialize(wrapped)
 
 
 class StripeWrapper:
@@ -54,10 +52,18 @@ class StripeWrapper:
     the foreign_key method
     """
 
+    id_prefix: str = ""
+
     def __init__(self, inner, all_objects):
         self.inner = inner
         self.all_objects = all_objects
 
+    @classmethod
+    def iter_items(cls, all_objects: dict[str, dict]):
+        for obj_id, obj in all_objects.items():
+            if obj_id.startswith(cls.id_prefix):
+                yield cls(obj, all_objects=all_objects)
+
     def __getattr__(self, attr):
         try:
             super().__getattr__(attr)
@@ -67,6 +73,9 @@ class StripeWrapper:
     def foreign_key(
         self, field: str, cls: Optional[Type[StripeWrapper]] = None, required=True
     ):
+        """Lookup an attribute value as a key in the all_objects dictionary and return
+        that object as a StripeWrapper if found
+        """
         if (fk := self.inner[field]) is None:
             return None
         obj = self.all_objects.get(fk)
@@ -80,6 +89,8 @@ class StripeWrapper:
 
 
 class InvoiceWrapper(StripeWrapper):
+    id_prefix: str = "in_"
+
     @property
     def kind(self):
         return "invoice"
@@ -98,6 +109,8 @@ class InvoiceWrapper(StripeWrapper):
 
 
 class CreditNoteWrapper(StripeWrapper):
+    id_prefix: str = "cn_"
+
     @property
     def kind(self):
         return "credit_note"
@@ -127,6 +140,18 @@ class PaymentIntentWrapper(StripeWrapper):
 
 @dataclasses.dataclass
 class ReportField:
+    """Use this to declaratively define a column that should be present in the csv
+    report.
+
+    :param display_name: the column name to be shown in the csv
+    :param path: a dotted path to a nested attribute in a ``StripeWrapper``. Both
+        attribute access and key/item lookup will be attempted to traverse the path
+        through ``StripeWrapper``\s. If a path cannot be resolved, ``None`` will be
+        returned.
+    :param transform: an optional callable to transform the retrieved attribute value.If
+        given,transform will only be called if the value is not ``None``
+    """
+
     display_name: str
     path: str
     transform: Optional[Callable] = None
@@ -178,9 +203,50 @@ def extract_gbp_vat(val):
     return ""
 
 
+class CSVReporter:
+    def __init__(self, serializers: Sequence[Serializer], get_data: Callable):
+        self.serializers = serializers
+        self.get_data = get_data
+
+    def write_csv(
+        self, buffer: io.StringIO, log_progress: Optional[Callable] = None, **kwargs
+    ):
+        data = self.get_data(log_progress)
+
+        writer = csv.DictWriter(
+            buffer, fieldnames=list(self.iter_field_names()), **kwargs
+        )
+        writer.writeheader()
+        length = 0
+        for item in self.iter_report_items(data):
+            length += 1
+            writer.writerow(item)
+        return length
+
+    def iter_fieldnames(self):
+        seen = set()
+        for field in itertools.chain.from_iterable(s.fields for s in self.serializers):
+            if field.display_name in seen:
+                continue
+            seen.add(field.display_name)
+            yield field.display_name
+
+    def iter_field_names(self):
+        seen = set()
+        for field in itertools.chain.from_iterable(s.fields for s in self.serializers):
+            if field.display_name in seen:
+                continue
+            seen.add(field.display_name)
+            yield field.display_name
+
+    def iter_report_items(self, data):
+        yield from itertools.chain.from_iterable(
+            s.iter_items(data) for s in self.serializers
+        )
+
+
 INVOICE_SERIALIZER = Serializer(
     wrapper_cls=InvoiceWrapper,
-    id_prefix="in_",
     fields=[
         ReportField("Kind", "kind"),
         ReportField("Stripe Reference Number", "id"),
@@ -217,7 +283,6 @@ INVOICE_SERIALIZER = Serializer(
 
 CREDIT_NOTE_SERIALIZER = Serializer(
     wrapper_cls=CreditNoteWrapper,
-    id_prefix="cn_",
     fields=[
         ReportField("Kind", "kind"),
         ReportField("Stripe Reference Number", "id"),
@@ -247,32 +312,6 @@ CREDIT_NOTE_SERIALIZER = Serializer(
 )
 
 
-def all_fieldnames():
-    seen = set()
-    for field in itertools.chain(
-        INVOICE_SERIALIZER.fields, CREDIT_NOTE_SERIALIZER.fields
-    ):
-        if field.display_name in seen:
-            continue
-        seen.add(field.display_name)
-        yield field.display_name
-
-
-def write_report(buffer: io.StringIO, all_objects: dict[str, stripe.StripeObject]):
-    writer = csv.DictWriter(buffer, fieldnames=list(all_fieldnames()))
-    writer.writeheader()
-    length = 0
-    for item in iter_report_items(all_objects):
-        length += 1
-        writer.writerow(item)
-    return length
-
-
-def iter_report_items(all_objects: Iterable[stripe.StripeObject]):
-    yield from INVOICE_SERIALIZER.iter_items(all_objects)
-    yield from CREDIT_NOTE_SERIALIZER.iter_items(all_objects)
-
-
 def get_all_objects(log_progress: Callable):
     def _iter_all_items():
         yield from list_all_stripe_items(stripe.Customer, log_progress)
@@ -283,6 +322,11 @@ def get_all_objects(log_progress: Callable):
     return {o["id"]: o for o in _iter_all_items()}
 
 
+reporter = CSVReporter(
+    [INVOICE_SERIALIZER, CREDIT_NOTE_SERIALIZER], get_data=get_all_objects
+)
+
+
 def main():
     import logging
     import os
@@ -294,9 +338,7 @@ def main():
 
     path = pathlib.Path.cwd() / "report.csv"
     with open(path, "w") as file:
-        length = write_report(
-            file, all_objects=get_all_objects(log_progress=logger.info)
-        )
+        length = reporter.write_csv(file, log_progress=logger.info)
     logger.info(f"Wrote {length} items to {path}")
 
 
diff --git a/stripe_checkout/stripe_checkout/management/commands/generatereport.py b/stripe_checkout/stripe_checkout/management/commands/generatereport.py
index 52b665b..1e4c1af 100644
--- a/stripe_checkout/stripe_checkout/management/commands/generatereport.py
+++ b/stripe_checkout/stripe_checkout/management/commands/generatereport.py
@@ -16,10 +16,7 @@ import stripe
 from django.conf import settings
 from django.core.management import BaseCommand
 
-from stripe_checkout.stripe_checkout.generate_report import (
-    get_all_objects,
-    write_report,
-)
+from stripe_checkout.stripe_checkout.generate_report import reporter
 from stripe_checkout.stripe_checkout.models import Report
 
 
@@ -28,8 +25,7 @@ class Command(BaseCommand):
         stripe.api_key = settings.STRIPE_API_KEY
 
         buffer = io.StringIO()
-        all_objects = get_all_objects(log_progress=self.stdout.write)
-        length = write_report(buffer, all_objects=all_objects)
+        length = reporter.write_csv(buffer, log_progress=self.stdout.write)
 
         report = self.get_report_object()
         report.data = buffer.getvalue()
diff --git a/stripe_checkout/stripe_checkout/management/commands/processevents.py b/stripe_checkout/stripe_checkout/management/commands/processevents.py
index fa1c0ea..25c0956 100644
--- a/stripe_checkout/stripe_checkout/management/commands/processevents.py
+++ b/stripe_checkout/stripe_checkout/management/commands/processevents.py
@@ -101,7 +101,7 @@ class Command(BaseCommand):
 
     def _notify_unprocessed_payment(self, payment_intent: dict):
         self.stdout.write(f"Unrecognized payment {payment_intent['id']}")
-        send_mail_to = settings.UNPROCESSED_PAYMENT_EMAIL_TO
+        send_mail_to = settings.SEND_ERROR_EMAILS_TO
         if not send_mail_to:
             return False
         if not isinstance(send_mail_to, list):
diff --git a/stripe_checkout/stripe_checkout/management/commands/sendpossibleerrors.py b/stripe_checkout/stripe_checkout/management/commands/sendpossibleerrors.py
new file mode 100644
index 0000000..02389d1
--- /dev/null
+++ b/stripe_checkout/stripe_checkout/management/commands/sendpossibleerrors.py
@@ -0,0 +1,58 @@
+"""
+Generate a CSV report with all Visitors that have zero or more than one invoices in
+Stripe, but should have (at least) one, and send this report to the
+``SEND_ERROR_EMAILS_TO`` setting recipients
+
+Usage:
+
+* Set the ``SEND_ERROR_EMAILS_TO`` setting
+* Invoke this command using `django-admin sendpossibleerrors` (or other manage.py
+    variant)
+* Check your email
+"""
+
+from __future__ import annotations
+
+import io
+
+import stripe
+from django.conf import settings
+from django.core.management import BaseCommand
+from django.core.mail import EmailMessage
+
+from stripe_checkout.stripe_checkout.compare_visit_stripe import reporter
+
+EMAIL_TEMPLATE = """\
+Hi!
+
+This is your daily report with possible errors in the connection between
+Visit and Stripe
+
+Please see the attached CSV file
+"""
+
+
+class Command(BaseCommand):
+    def handle(self, *args, **options):
+        stripe.api_key = settings.STRIPE_API_KEY
+
+        buffer = io.StringIO()
+        reporter.write_csv(buffer, log_progress=self.stdout.write, delimiter=";")
+        data = buffer.getvalue()
+        self.send_report(data)
+
+    def send_report(self, content):
+        send_mail_to = settings.SEND_ERROR_EMAILS_TO
+        if not send_mail_to:
+            self.stdout.write(
+                self.style.WARNING("SEND_ERROR_EMAILS_TO not set, not sending report")
+            )
+            return
+        email = EmailMessage(
+            subject="Daily possible Stripe/Visit error report",
+            body=EMAIL_TEMPLATE,
+            from_email=None,
+            to=send_mail_to,
+        )
+        email.attach("possible_errors.csv", content=content, mimetype="text/csv")
+        email.send()
diff --git a/stripe_checkout/stripe_checkout/visit.py b/stripe_checkout/stripe_checkout/visit.py
index 5ba5f06..fb0df50 100644
--- a/stripe_checkout/stripe_checkout/visit.py
+++ b/stripe_checkout/stripe_checkout/visit.py
@@ -47,13 +47,13 @@ class VisitorAPI:
         visitors_by_id = {}
         while True:
             response = self._request_visitors(from_revision=next_revision)
-            if log_progress is not None:
-                log_progress(f"Retrieved {len(response)} visitors")
-
             next_revision = response[-1]["revision"]
             for visitor in response:
                 visitors_by_id[visitor["id"]] = visitor
 
+            if log_progress is not None:
+                log_progress(f"Retrieved {len(visitors_by_id)} visitors")
+
             if next_revision == last_revision:
                 break
             last_revision = next_revision
@@ -72,7 +72,7 @@ class VisitorAPI:
         # properly raise
         if not isinstance(result, dict):
             raise Http404()
-        if result['deleted'] and not allow_deleted:
+        if result["deleted"] and not allow_deleted:
             raise Http404()
         return Visitor.from_api(result)
 
@@ -110,8 +110,8 @@ class Visitor(dict):
         return f"{self.first_name} {self.last_name}".strip()
 
     @property
-    def email(self):
-        return self["contact"].get("email")
+    def email(self) -> str:
+        return self["contact"].get("email", "")
 
     @property
     def billing_address(self):
diff --git a/test/conftest.py b/test/conftest.py
index cc44913..ad96e81 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -246,7 +246,7 @@ def config_file(stripe_api_key, visit_api_key, visit_expo_id, tmp_path):
                 "STRIPE_SIGNING_SECRET": "stripe-signing-secret",
                 "STRIPE_INVOICE_TEMPLATE_ID": "stripe-invoice-template-id",
                 "STRIPE_TAX_RATE_ID": "stripe-tax-rate-id",
-                "UNPROCESSED_PAYMENT_EMAIL_TO": ["test@geant.org"],
+                "SEND_ERROR_EMAILS_TO": ["test@geant.org"],
             }
         )
     )
-- 
GitLab