Bank transfer: Allow using external IDs for deduplication (pretix#3803)

* Bank transfer: Allow using external IDs for deduplication * Do not use empty string in nullable field
thefelixno · Jan 9, 2024 · 2c67b82 · 2c67b82
1 parent 7a28786
commit 2c67b82
Show file tree

Hide file tree

Showing 7 changed files with 95 additions and 4 deletions.
diff --git a/doc/plugins/banktransfer.rst b/doc/plugins/banktransfer.rst
@@ -32,6 +32,7 @@ transactions                          list of objects            Transactions in
 ├ checksum                            string                     Checksum computed from payer, reference, amount and
                                                                  date
 ├ payer                               string                     Payment source
+├ external_id                         string                     Unique ID of the payment from an external source
 ├ reference                           string                     Payment reference
 ├ amount                              string                     Payment amount
 ├ iban                                string                     Payment IBAN
@@ -85,6 +86,7 @@ Endpoints
                 "date": "26.06.2017",
                 "payer": "John Doe",
                 "order": null,
+                "external_id": null,
                 "iban": "",
                 "bic": "",
                 "checksum": "5de03a601644dfa63420dacfd285565f8375a8f2",
@@ -139,6 +141,7 @@ Endpoints
             "iban": "",
             "bic": "",
             "order": null,
+            "external_id": null,
             "checksum": "5de03a601644dfa63420dacfd285565f8375a8f2",
             "reference": "GUTSCHRIFT\r\nSAMPLECONF-NAB12 EREF: SAMPLECONF-NAB12\r\nIBAN: DE1234556…",
             "state": "nomatch",

diff --git a/src/pretix/plugins/banktransfer/api.py b/src/pretix/plugins/banktransfer/api.py
@@ -46,7 +46,7 @@ class BankTransactionSerializer(serializers.ModelSerializer):
     class Meta:
         model = BankTransaction
         fields = ('state', 'message', 'checksum', 'payer', 'reference', 'amount', 'date', 'order',
-                  'comment', 'iban', 'bic', 'currency')
+                  'comment', 'iban', 'bic', 'currency', 'external_id')
 
 
 class BankImportJobSerializer(serializers.ModelSerializer):

diff --git a/src/pretix/plugins/banktransfer/migrations/0011_banktransaction_external_id.py b/src/pretix/plugins/banktransfer/migrations/0011_banktransaction_external_id.py
@@ -0,0 +1,17 @@
+# Generated by Django 4.2.9 on 2024-01-09 09:30
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("banktransfer", "0010_bigint"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="banktransaction",
+            name="external_id",
+            field=models.CharField(db_index=True, max_length=190, null=True),
+        ),
+    ]
diff --git a/src/pretix/plugins/banktransfer/models.py b/src/pretix/plugins/banktransfer/models.py
@@ -82,6 +82,7 @@ class BankTransaction(models.Model):
     currency = models.CharField(max_length=10, null=True)
     state = models.CharField(max_length=32, choices=STATES, default=STATE_UNCHECKED)
     message = models.TextField()
+    external_id = models.CharField(max_length=190, db_index=True, null=True, blank=True)
     checksum = models.CharField(max_length=190, db_index=True)
     payer = models.TextField(blank=True)
     reference = models.TextField(blank=True)

diff --git a/src/pretix/plugins/banktransfer/tasks.py b/src/pretix/plugins/banktransfer/tasks.py
@@ -307,6 +307,9 @@ def _get_unknown_transactions(job: BankImportJob, data: list, event: Event = Non
     known_checksums = set(t['checksum'] for t in BankTransaction.objects.filter(
         Q(event=event) if event else Q(organizer=organizer)
     ).values('checksum'))
+    known_by_external_id = set((t['external_id'], t['date'], t['amount']) for t in BankTransaction.objects.filter(
+        Q(event=event) if event else Q(organizer=organizer), external_id__isnull=False
+    ).values('external_id', 'date', 'amount'))
 
     transactions = []
     for row in data:
@@ -328,14 +331,17 @@ def _get_unknown_transactions(job: BankImportJob, data: list, event: Event = Non
         trans = BankTransaction(event=event, organizer=organizer, import_job=job,
                                 payer=row.get('payer', ''),
                                 reference=row.get('reference', ''),
-                                amount=amount, date=row.get('date', ''),
-                                iban=row.get('iban', ''), bic=row.get('bic', ''),
+                                amount=amount,
+                                date=row.get('date', ''),
+                                iban=row.get('iban', ''),
+                                bic=row.get('bic', ''),
+                                external_id=row.get('external_id'),
                                 currency=event.currency if event else job.currency)
 
         trans.date_parsed = parse_date(trans.date)
 
         trans.checksum = trans.calculate_checksum()
-        if trans.checksum not in known_checksums:
+        if trans.checksum not in known_checksums and (not trans.external_id or (trans.external_id, trans.date, trans.amount) not in known_by_external_id):
             trans.state = BankTransaction.STATE_UNCHECKED
             trans.save()
             transactions.append(trans)

diff --git a/src/tests/plugins/banktransfer/test_api.py b/src/tests/plugins/banktransfer/test_api.py
@@ -75,6 +75,7 @@ def env():
          'checksum': '',
          'iban': '',
          'bic': '',
+         'external_id': None,
          'amount': '0.00',
          'date': 'unknown',
          'state': 'error',

diff --git a/src/tests/plugins/banktransfer/test_import.py b/src/tests/plugins/banktransfer/test_import.py
@@ -693,3 +693,66 @@ def test_refund_handling_pending_refund(env, orga_job, orga_job2):
         r.refresh_from_db()
         assert env[2].payments.count() == 1
         assert r.state == OrderRefund.REFUND_STATE_DONE
+
+
+@pytest.mark.django_db
+def test_ignore_by_checksum(env, job):
+    process_banktransfers(job, [{
+        'payer': 'Karla Kundin',
+        'reference': 'Bestellung DUMMY6789Z',
+        'date': '2016-01-26',
+        'amount': '23.00'
+    }])
+    with scopes_disabled():
+        assert BankTransaction.objects.count() == 1
+
+    process_banktransfers(job, [{
+        'payer': 'Karla Kundin',
+        'reference': 'Bestellung    DUMMY6789Z',
+        'date': '2016-01-26',
+        'amount': '23.00'
+    }])
+    with scopes_disabled():
+        assert BankTransaction.objects.count() == 1
+
+    process_banktransfers(job, [{
+        'payer': 'Karla Kundin',
+        'reference': 'Bestellung    DUMMY6789Z',
+        'date': '2016-01-27',
+        'amount': '23.00'
+    }])
+    with scopes_disabled():
+        assert BankTransaction.objects.count() == 2
+
+
+@pytest.mark.django_db
+def test_ignore_by_external_id(env, job):
+    process_banktransfers(job, [{
+        'payer': 'Karla Kundin',
+        'reference': 'Bestellung DUMMY6789Z',
+        'external_id': 'abcd12345',
+        'date': '2016-01-26',
+        'amount': '23.00'
+    }])
+    with scopes_disabled():
+        assert BankTransaction.objects.count() == 1
+
+    process_banktransfers(job, [{
+        'payer': 'Karla Kundin',
+        'reference': 'Completely different reference because banks are weird',
+        'external_id': 'abcd12345',
+        'date': '2016-01-26',
+        'amount': '23.00'
+    }])
+    with scopes_disabled():
+        assert BankTransaction.objects.count() == 1
+
+    process_banktransfers(job, [{
+        'payer': 'Karla Kundin',
+        'reference': 'Same ID with different amount because banks are weird',
+        'external_id': 'abcd12345',
+        'date': '2016-01-26',
+        'amount': '24.00'
+    }])
+    with scopes_disabled():
+        assert BankTransaction.objects.count() == 2