From 53fa08bdac3d1fabf8838ada5d064368f60709ac Mon Sep 17 00:00:00 2001 From: Hugo Osvaldo Barrera Date: Fri, 18 Mar 2022 18:33:56 +0100 Subject: [PATCH] Use a JSONField for BasePayment.extra_data --- CHANGELOG.rst | 3 ++ docs/modules.rst | 2 +- payments/core.py | 3 +- payments/cybersource/__init__.py | 14 +++---- payments/cybersource/forms.py | 4 +- payments/cybersource/test_cybersource.py | 6 +-- payments/mercadopago/__init__.py | 6 +-- payments/models.py | 53 ++++++++---------------- payments/paypal/__init__.py | 2 +- payments/stripe/__init__.py | 6 +-- payments/stripe/forms.py | 2 +- payments/test_core.py | 8 ++-- payments/todopago/__init__.py | 16 +++---- payments/todopago/test_todopago.py | 10 ++--- 14 files changed, 60 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 05a9fa60f..c04d02028 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,6 +8,9 @@ v2.0.0 ------ - Dropped support for Django 2.2. +- ``BasePayment.extra_data`` is now a JSONField and django will handle the + serialisation. Due to this, usage of the ``BasePayment.attrs`` proxy has been + deprecated. A migration needs to be generated to update this column in place. v1.0.0 ------ diff --git a/docs/modules.rst b/docs/modules.rst index 7f7b0dc8b..81d25cdda 100644 --- a/docs/modules.rst +++ b/docs/modules.rst @@ -106,7 +106,7 @@ about the payment or the order, such as an order number, additional customer information, or a special comment or request from the customer. This can be accomplished by passing your data to the :class:`Payment` instance:: - >>> payment.attrs.merchant_defined_data = {'01': 'foo', '02': 'bar'} + >>> payment.extra_data.merchant_defined_data = {'01': 'foo', '02': 'bar'} Dotpay diff --git a/payments/core.py b/payments/core.py index 4609d8e9f..294bcb326 100644 --- a/payments/core.py +++ b/payments/core.py @@ -1,4 +1,5 @@ import re +from importlib import import_module from typing import TYPE_CHECKING from typing import Dict from typing import Optional @@ -155,7 +156,7 @@ def _default_provider_factory(variant: str, payment: Optional["BasePayment"] = N raise ValueError("Payment variant does not exist: {}".format(variant)) if variant not in PROVIDER_CACHE: module_path, class_name = handler.rsplit(".", 1) - module = __import__(str(module_path), globals(), locals(), [str(class_name)]) + module = import_module(module_path) class_ = getattr(module, class_name) PROVIDER_CACHE[variant] = class_(**config) return PROVIDER_CACHE[variant] diff --git a/payments/cybersource/__init__.py b/payments/cybersource/__init__.py index c8dae1573..fb624ebfe 100644 --- a/payments/cybersource/__init__.py +++ b/payments/cybersource/__init__.py @@ -166,11 +166,11 @@ def charge(self, payment, data): else: params = self._prepare_preauth(payment, data) response = self._make_request(payment, params) - payment.attrs.capture = self._capture + payment.extra_data.capture = self._capture payment.transaction_id = response.requestID if response.reasonCode == AUTHENTICATE_REQUIRED: xid = response.payerAuthEnrollReply.xid - payment.attrs.xid = xid + payment.extra_data.xid = xid payment.change_status( PaymentStatus.WAITING, message=_("3-D Secure verification in progress") ) @@ -279,7 +279,7 @@ def _get_params_for_new_payment(self, payment): "merchantReferenceCode": payment.id, } try: - fingerprint_id = payment.attrs.fingerprint_session_id + fingerprint_id = payment.extra_data.fingerprint_session_id except KeyError: pass else: @@ -291,7 +291,7 @@ def _get_params_for_new_payment(self, payment): def _make_request(self, payment, params): response = self.client.service.runTransaction(**params) - payment.attrs.last_response = self._serialize_response(response) + payment.extra_data.last_response = self._serialize_response(response) return response def _prepare_payer_auth_validation_check(self, payment, card_data, pa_response): @@ -300,7 +300,7 @@ def _prepare_payer_auth_validation_check(self, payment, card_data, pa_response): check_service.signedPARes = pa_response params = self._get_params_for_new_payment(payment) params["payerAuthValidateService"] = check_service - if payment.attrs.capture: + if payment.extra_data.capture: service = self.client.factory.create("data:CCCreditService") service._run = "true" params["ccCreditService"] = service @@ -445,7 +445,7 @@ def _prepare_items(self, payment): def _prepare_merchant_defined_data(self, payment): try: - merchant_defined_data = payment.attrs.merchant_defined_data + merchant_defined_data = payment.extra_data.merchant_defined_data except KeyError: return else: @@ -476,7 +476,7 @@ def _serialize_response(self, response): def process_data(self, payment, request): xid = request.POST.get("MD") - if xid != payment.attrs.xid: + if xid != payment.extra_data.xid: return redirect(payment.get_failure_url()) if payment.status in [PaymentStatus.CONFIRMED, PaymentStatus.PREAUTH]: return redirect(payment.get_success_url()) diff --git a/payments/cybersource/forms.py b/payments/cybersource/forms.py index f4af83d21..832d9f3ed 100644 --- a/payments/cybersource/forms.py +++ b/payments/cybersource/forms.py @@ -40,7 +40,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) if self.provider.org_id: try: - fingerprint_id = self.payment.attrs.fingerprint_session_id + fingerprint_id = self.payment.extra_data.fingerprint_session_id except KeyError: fingerprint_id = str(uuid4()) self.fields["fingerprint"] = FingerprintInput( @@ -56,7 +56,7 @@ def clean(self): if not self.errors: if self.provider.org_id: fingerprint = cleaned_data["fingerprint"] - self.payment.attrs.fingerprint_session_id = fingerprint + self.payment.extra_data.fingerprint_session_id = fingerprint if not self.payment.transaction_id: try: self.provider.charge(self.payment, cleaned_data) diff --git a/payments/cybersource/test_cybersource.py b/payments/cybersource/test_cybersource.py index 6155e819f..6c42f2e7f 100644 --- a/payments/cybersource/test_cybersource.py +++ b/payments/cybersource/test_cybersource.py @@ -152,7 +152,7 @@ def test_provider_redirects_on_success_captured_payment( ): transaction_id = 1234 xid = "abc" - self.payment.attrs.xid = xid + self.payment.extra_data.xid = xid response = MagicMock() response.requestID = transaction_id @@ -187,7 +187,7 @@ def test_provider_redirects_on_success_preauth_payment( ) transaction_id = 1234 xid = "abc" - self.payment.attrs.xid = xid + self.payment.extra_data.xid = xid response = MagicMock() response.requestID = transaction_id @@ -217,7 +217,7 @@ def test_provider_redirects_on_success_preauth_payment( def test_provider_redirects_on_failure(self, mocked_request, mocked_redirect): transaction_id = 1234 xid = "abc" - self.payment.attrs.xid = xid + self.payment.extra_data.xid = xid response = MagicMock() response.requestID = transaction_id diff --git a/payments/mercadopago/__init__.py b/payments/mercadopago/__init__.py index a0b3d5038..f3080df49 100644 --- a/payments/mercadopago/__init__.py +++ b/payments/mercadopago/__init__.py @@ -71,7 +71,7 @@ def create_preference(self, payment: BasePayment): if payment.transaction_id: raise ValueError("This payment already has a preference.") - payment.attrs.external_reference = uuid4().hex + payment.extra_data.external_reference = uuid4().hex payload = { "auto_return": "all", @@ -86,7 +86,7 @@ def create_preference(self, payment: BasePayment): } for item in payment.get_purchased_items() ], - "external_reference": payment.attrs.external_reference, + "external_reference": payment.extra_data.external_reference, "back_urls": { "success": self.get_return_url(payment), "pending": self.get_return_url(payment), @@ -214,7 +214,7 @@ def poll_for_updates(self, payment: BasePayment): """ data = self.client.payment().search( { - "external_reference": payment.attrs.external_reference, + "external_reference": payment.extra_data.external_reference, } ) diff --git a/payments/models.py b/payments/models.py index b97d12c81..922158100 100644 --- a/payments/models.py +++ b/payments/models.py @@ -1,4 +1,4 @@ -import json +import warnings from typing import Iterable from typing import Union from uuid import uuid4 @@ -14,29 +14,6 @@ from .core import provider_factory -class PaymentAttributeProxy: - def __init__(self, payment): - self._payment = payment - super().__init__() - - def __getattr__(self, item): - data = json.loads(self._payment.extra_data or "{}") - try: - return data[item] - except KeyError as e: - raise AttributeError(*e.args) - - def __setattr__(self, key, value): - if key == "_payment": - return super().__setattr__(key, value) - try: - data = json.loads(self._payment.extra_data) - except ValueError: - data = {} - data[key] = value - self._payment.extra_data = json.dumps(data) - - class BasePayment(models.Model): """ Represents a single transaction. Each instance has one or more PaymentItem. @@ -78,7 +55,7 @@ class BasePayment(models.Model): billing_email = models.EmailField(blank=True) billing_phone = PhoneNumberField(blank=True) customer_ip_address = models.GenericIPAddressField(blank=True, null=True) - extra_data = models.TextField(blank=True, default="") + extra_data = models.JsonField(blank=True, default=dict) message = models.TextField(blank=True, default="") token = models.CharField(max_length=36, blank=True, default="") captured_amount = models.DecimalField(max_digits=9, decimal_places=2, default="0.0") @@ -195,14 +172,18 @@ def refund(self, amount=None): @property def attrs(self): - """A JSON-serialised wrapper around `extra_data`. - - This property exposes a a dict or list which is serialised into the `extra_data` - text field. Usage of this wrapper is preferred over accessing the underlying - field directly. - - You may think of this as a `JSONField` which is saved to the `extra_data` - column. - """ - # TODO: Deprecate in favour of JSONField when we drop support for django 2.2. - return PaymentAttributeProxy(self) + warnings.warn( + "Using BasePayment.attrs is deprecated. Use BasePayment.extra_data instead", + DeprecationWarning, + stacklevel=2, + ) + return self.extra_data + + @attrs.setter + def attrs(self, value): + warnings.warn( + "Using BasePayment.attrs is deprecated. Use BasePayment.extra_data instead", + DeprecationWarning, + stacklevel=2, + ) + self.extra_data = value diff --git a/payments/paypal/__init__.py b/payments/paypal/__init__.py index 6fd336cb0..f3c6eca51 100644 --- a/payments/paypal/__init__.py +++ b/payments/paypal/__init__.py @@ -249,7 +249,7 @@ def process_data(self, payment, request): except PaymentError: return redirect(failure_url) self.set_response_links(payment, executed_payment) - payment.attrs.payer_info = executed_payment["payer"]["payer_info"] + payment.extra_data.payer_info = executed_payment["payer"]["payer_info"] if self._capture: payment.captured_amount = payment.total payment.change_status(PaymentStatus.CONFIRMED) diff --git a/payments/stripe/__init__.py b/payments/stripe/__init__.py index d1571daa1..860c6bbcc 100644 --- a/payments/stripe/__init__.py +++ b/payments/stripe/__init__.py @@ -50,19 +50,19 @@ def capture(self, payment, amount=None): except stripe.InvalidRequestError: payment.change_status(PaymentStatus.REFUNDED) raise PaymentError("Payment already refunded") - payment.attrs.capture = json.dumps(charge) + payment.extra_data.capture = json.dumps(charge) return Decimal(amount) / 100 def release(self, payment): charge = stripe.Charge.retrieve(payment.transaction_id) charge.refund() - payment.attrs.release = json.dumps(charge) + payment.extra_data.release = json.dumps(charge) def refund(self, payment, amount=None): amount = int((amount or payment.total) * 100) charge = stripe.Charge.retrieve(payment.transaction_id) charge.refund(amount=amount) - payment.attrs.refund = json.dumps(charge) + payment.extra_data.refund = json.dumps(charge) return Decimal(amount) / 100 diff --git a/payments/stripe/forms.py b/payments/stripe/forms.py index c29264b9d..428f911e0 100644 --- a/payments/stripe/forms.py +++ b/payments/stripe/forms.py @@ -76,7 +76,7 @@ def clean(self): def save(self): self.payment.transaction_id = self.charge.id - self.payment.attrs.charge = json.dumps(self.charge) + self.payment.extra_data.charge = json.dumps(self.charge) self.payment.change_status(PaymentStatus.PREAUTH) if self.provider._capture: self.payment.capture() diff --git a/payments/test_core.py b/payments/test_core.py index a8f640b63..8e4e73b14 100644 --- a/payments/test_core.py +++ b/payments/test_core.py @@ -41,10 +41,10 @@ class Payment(BasePayment): class TestBasePayment(TestCase): def test_payment_attributes(self): payment = Payment(extra_data='{"attr1": "test1", "attr2": "test2"}') - self.assertEqual(payment.attrs.attr1, "test1") - self.assertEqual(payment.attrs.attr2, "test2") - self.assertEqual(getattr(payment.attrs, "attr5", None), None) - self.assertEqual(hasattr(payment.attrs, "attr7"), False) + self.assertEqual(payment.extra_data.attr1, "test1") + self.assertEqual(payment.extra_data.attr2, "test2") + self.assertEqual(getattr(payment.extra_data, "attr5", None), None) + self.assertEqual(hasattr(payment.extra_data, "attr7"), False) def test_capture_with_wrong_status(self): payment = Payment(variant="default", status=PaymentStatus.WAITING) diff --git a/payments/todopago/__init__.py b/payments/todopago/__init__.py index 4c194ff70..a7827595c 100644 --- a/payments/todopago/__init__.py +++ b/payments/todopago/__init__.py @@ -74,9 +74,9 @@ def authorize_operation(self, payment: BasePayment) -> Authorization: gateway_message=authorization.status_message, ) - payment.attrs.request_key = authorization.request_key - payment.attrs.public_request_key = authorization.public_request_key - payment.attrs.form_url = authorization.form_url + payment.extra_data.request_key = authorization.request_key + payment.extra_data.public_request_key = authorization.public_request_key + payment.extra_data.form_url = authorization.form_url payment.transaction_id = transaction_id payment.save() @@ -84,8 +84,8 @@ def authorize_operation(self, payment: BasePayment) -> Authorization: return authorization def fetch_operation_status(self, payment: BasePayment) -> OperationStatus: - request_key = payment.attrs.request_key - answer_key = payment.attrs.answer_key + request_key = payment.extra_data.request_key + answer_key = payment.extra_data.answer_key status = self.client.get_operation_status(request_key, answer_key) @@ -116,7 +116,7 @@ def process_callback(self, payment: BasePayment, request: HttpRequest): payment.change_status(PaymentStatus.ERROR) return redirect(payment.get_failure_url()) - payment.attrs.answer_key = answer_key + payment.extra_data.answer_key = answer_key payment.save() self.fetch_operation_status(payment) @@ -137,10 +137,10 @@ def process_data(self, payment: BasePayment, request: HttpRequest): return self.process_callback(payment, request) def get_form(self, payment: BasePayment, data=None): - if "form_url" not in payment.attrs: + if "form_url" not in payment.extra_data: self.authorize_operation(payment) - url = payment.attrs.form_url + url = payment.extra_data.form_url raise RedirectNeeded(url) diff --git a/payments/todopago/test_todopago.py b/payments/todopago/test_todopago.py index 7913562cd..b46310798 100644 --- a/payments/todopago/test_todopago.py +++ b/payments/todopago/test_todopago.py @@ -85,13 +85,13 @@ def test_authorize_operation(tp_provider): tp_provider.authorize_operation(payment) assert payment.status == PaymentStatus.WAITING - assert payment.attrs.request_key == "f5ad41bc-92ba-40ff-889d-8a23fe562a28" + assert payment.extra_data.request_key == "f5ad41bc-92ba-40ff-889d-8a23fe562a28" def test_approved_payment_notification(rf, tp_provider): payment = Payment() - payment.attrs.request_key = "1fb7cc9a-14dd-42ec-bf1e-6d5820799642" - payment.attrs.form_url = ( + payment.extra_data.request_key = "1fb7cc9a-14dd-42ec-bf1e-6d5820799642" + payment.extra_data.form_url = ( "https://forms.todopago.com.ar/formulario/commands?command=formulario&m=a6104bad3-1be7-4e8e-932e-e927100b2e86&fr=1", ) payment.save() @@ -119,8 +119,8 @@ def test_approved_payment_notification(rf, tp_provider): def test_rejected_payment_notification(rf, tp_provider): payment = Payment() - payment.attrs.request_key = "1fb7cc9a-14dd-42ec-bf1e-6d5820799642" - payment.attrs.form_url = ( + payment.extra_data.request_key = "1fb7cc9a-14dd-42ec-bf1e-6d5820799642" + payment.extra_data.form_url = ( "https://forms.todopago.com.ar/formulario/commands?command=formulario&m=a6104bad3-1be7-4e8e-932e-e927100b2e86&fr=1", ) payment.save()