From 91b52220f32a0e9cb516687fd88c1f8ed83ba5b2 Mon Sep 17 00:00:00 2001 From: Maik Hoepfel Date: Tue, 14 Jul 2015 12:06:22 +0200 Subject: [PATCH 1/3] Remove get_payment_outcome This was removed from the Scaffold's interface in oshop, so we can remove it here. --- adyen/scaffold.py | 4 ---- tests/test_adyen.py | 20 -------------------- 2 files changed, 24 deletions(-) diff --git a/adyen/scaffold.py b/adyen/scaffold.py index 874367c..445472f 100644 --- a/adyen/scaffold.py +++ b/adyen/scaffold.py @@ -83,10 +83,6 @@ def handle_payment_feedback(self, request): return self._normalize_feedback( Facade().handle_payment_feedback(request, record_audit_trail=True)) - def check_payment_outcome(self, request): - return self._normalize_feedback( - Facade().handle_payment_feedback(request, record_audit_trail=False)) - def assess_notification_relevance(self, request): return Facade().assess_notification_relevance(request) diff --git a/tests/test_adyen.py b/tests/test_adyen.py index e1dd3bf..9842e3a 100644 --- a/tests/test_adyen.py +++ b/tests/test_adyen.py @@ -261,26 +261,6 @@ def test_get_origin_ip_address(self): def test_handle_authorised_payment(self): - # Before the test, there are no recorded transactions in the database. - num_recorded_transactions = AdyenTransaction.objects.all().count() - self.assertEqual(num_recorded_transactions, 0) - - self.request.GET = deepcopy(AUTHORISED_PAYMENT_PARAMS_GET) - success, status, details = self.scaffold.check_payment_outcome(self.request) - - self.assertTrue(success) - self.assertEqual(status, Scaffold.PAYMENT_STATUS_ACCEPTED) - self.assertEqual(details.get('amount'), 13894) - self.assertEqual(details.get('ip_address'), '127.0.0.1') - self.assertEqual(details.get('method'), 'adyen') - self.assertEqual(details.get('psp_reference'), '8814136447235922') - self.assertEqual(details.get('status'), 'AUTHORISED') - - # After calling `check_payment_outcome`, there are still no recorded - # transactions in the database. - num_recorded_transactions = AdyenTransaction.objects.all().count() - self.assertEqual(num_recorded_transactions, 0) - self.request.GET = deepcopy(AUTHORISED_PAYMENT_PARAMS_GET) success, status, details = self.scaffold.handle_payment_feedback(self.request) From 59681e1176ff77ef512264a614e024e9c0f5100c Mon Sep 17 00:00:00 2001 From: Maik Hoepfel Date: Tue, 14 Jul 2015 12:07:22 +0200 Subject: [PATCH 2/3] Always record audit trail As check_payment_outcome was removed, the record_audit_trail kwarg was always true. So we can remove it for simplicity. --- adyen/facade.py | 7 +++---- adyen/scaffold.py | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/adyen/facade.py b/adyen/facade.py index 99f805c..2c3709b 100644 --- a/adyen/facade.py +++ b/adyen/facade.py @@ -138,7 +138,7 @@ def _record_audit_trail(self, request, status, txn_details): pass - def handle_payment_feedback(self, request, record_audit_trail): + def handle_payment_feedback(self, request): """ Validate, process, optionally record audit trail and provide feedback about the current payment response. @@ -167,9 +167,8 @@ def handle_payment_feedback(self, request, record_audit_trail): success, status, details = response.process() txn_details = self._unpack_details(details) - # ... and record the audit trail if instructed to... - if record_audit_trail: - self._record_audit_trail(request, status, txn_details) + # ... and record the audit trail. + self._record_audit_trail(request, status, txn_details) # ... prepare the feedback data... output_data = { diff --git a/adyen/scaffold.py b/adyen/scaffold.py index 445472f..196f2bc 100644 --- a/adyen/scaffold.py +++ b/adyen/scaffold.py @@ -80,8 +80,7 @@ def _normalize_feedback(self, feedback): return success, common_status, details def handle_payment_feedback(self, request): - return self._normalize_feedback( - Facade().handle_payment_feedback(request, record_audit_trail=True)) + return self._normalize_feedback(Facade().handle_payment_feedback(request)) def assess_notification_relevance(self, request): return Facade().assess_notification_relevance(request) From 956eaaf17b6160db91eccfe8a302e3227da316f6 Mon Sep 17 00:00:00 2001 From: Maik Hoepfel Date: Tue, 14 Jul 2015 12:13:10 +0200 Subject: [PATCH 3/3] Ignore duplicate notifications Adyen duplicates many notifications. Luckily, our audit trail gives us an easy way to check for duplicates. We ignore duplicated notifications, but acknowledge them nonetheless. --- adyen/facade.py | 11 +++++++++++ tests/test_adyen.py | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/adyen/facade.py b/adyen/facade.py index 2c3709b..c9a74be 100644 --- a/adyen/facade.py +++ b/adyen/facade.py @@ -219,6 +219,17 @@ def assess_notification_relevance(self, request): if request.POST.get(Constants.EVENT_CODE) != Constants.EVENT_CODE_AUTHORISATION: return False, True + # Adyen duplicates many notifications. This bit makes sure we ignore them. + # "Duplicate notifications have the same corresponding values for their eventCode and + # pspReference fields." https://docs.adyen.com/display/TD/Accept+notifications + reference = request.POST[Constants.PSP_REFERENCE] + # The event code gets checked above, so we only need to check for the reference now. + if AdyenTransaction.objects.filter(reference=reference).exists(): + # We already stored a transaction with this reference, so we can ignore the + # notification. As above, we still acknowledge it to Adyen, in case it missed + # our previous acknowledgment. + return False, True + # Seems legit, just do it :) return True, True diff --git a/tests/test_adyen.py b/tests/test_adyen.py index 9842e3a..71db146 100644 --- a/tests/test_adyen.py +++ b/tests/test_adyen.py @@ -421,3 +421,22 @@ def test_assess_notification_relevance(self): self.request.POST['eventCode'] = 'REPORT_AVAILABLE' must_process, must_ack = self.scaffold.assess_notification_relevance(self.request) self.assertTupleEqual((must_process, must_ack), (False, True)) + + def test_assert_duplicate_notifications(self): + """ + This test tests that duplicate notifications are ignored. + """ + self.request.method = 'POST' + self.request.POST = deepcopy(AUTHORISED_PAYMENT_PARAMS_POST) + + # We have a valid request. So let's confirm that we think we should process + # and acknowledge it. + assert (True, True) == self.scaffold.assess_notification_relevance(self.request) + + # Let's process it then. + __, __, __ = self.scaffold.handle_payment_feedback(self.request) + + # As we have already processed that request, we now shouldn't process the request + # any more. But we still acknowledge it. + assert (False, True) == self.scaffold.assess_notification_relevance(self.request) +