refactor: harden error parser and drop dead capture mixin

Author: vildanbina

buckaroo/builders/payments/capabilities/authorize_capture_capable.py

@@ -16,37 +16,34 @@
 
 
 class AuthorizeCaptureCapable:
-    """Mixin for payment methods that support authorization (Credit Card)."""
+    """Mixin contributing the Authorize / CancelAuthorize action surface.
 
-    def authorize(self: "PaymentBuilder", validate: bool = True) -> PaymentResponse:
-        """
-        Authorize a payment without capturing it.
+    ``capture`` lives on :class:`BaseBuilder` with the full
+    ``original_transaction_key`` / ``amount`` signature and is shared by every
+    builder; it is intentionally not duplicated here.
+    """
 
-        Available for: Credit Card
-        Not available for: iDEAL, Sofort, PayConiq (immediate transfer)
+    def authorize(self: "PaymentBuilder", validate: bool = True) -> PaymentResponse:
+        """Authorize a payment without capturing it.
 
         Args:
-            validate (bool): Whether to validate service parameters before building
+            validate: Whether to validate service parameters before building.
 
         Returns:
-            PaymentResponse: The authorization response
+            PaymentResponse: The authorization response.
         """
         payment_request = self.build("Authorize", validate=validate)
         request_data = payment_request.to_dict()
         return self._post_transaction(request_data)
 
     def authorizeEncrypted(self: "PaymentBuilder", validate: bool = True) -> PaymentResponse:
-        """
-        Authorize a payment without capturing it.
-
-        Available for: Credit Card
-        Not available for: iDEAL, Sofort, PayConiq (immediate transfer)
+        """Authorize an encrypted-card payment without capturing it.
 
         Args:
-            validate (bool): Whether to validate service parameters before building
+            validate: Whether to validate service parameters before building.
 
         Returns:
-            PaymentResponse: The authorization response
+            PaymentResponse: The authorization response.
         """
         payment_request = self.build("AuthorizeEncrypted", validate=validate)
         request_data = payment_request.to_dict()
@@ -57,8 +54,7 @@ def cancelAuthorize(
         original_transaction_key: Optional[str] = None,
         validate: bool = True,
     ) -> PaymentResponse:
-        """
-        Cancel a previously authorized payment.
+        """Cancel a previously authorized payment.
 
         Uses AmountCredit (not AmountDebit) per Buckaroo API requirements.
         """
@@ -83,18 +79,3 @@ def cancelAuthorize(
         request_data["AmountCredit"] = request_data.pop("AmountDebit")
 
         return self._post_transaction(request_data)
-
-    def capture(self: "PaymentBuilder", validate: bool = True) -> PaymentResponse:
-        """
-        Capture a previously authorized payment.
-
-        Args:
-            validate (bool): Whether to validate service parameters before building
-
-        Returns:
-            PaymentResponse: The capture response
-        """
-
-        payment_request = self.build("Capture", validate=validate)
-        request_data = payment_request.to_dict()
-        return self._post_transaction(request_data)

buckaroo/builders/payments/riverty_builder.py

@@ -1,18 +1,28 @@
 from typing import Dict, Any
 from .payment_builder import PaymentBuilder
+from .capabilities.authorize_capture_capable import AuthorizeCaptureCapable
 
 
-class RivertyBuilder(PaymentBuilder):
-    """Builder for Riverty payments with bank transfer capabilities."""
+class RivertyBuilder(PaymentBuilder, AuthorizeCaptureCapable):
+    """Builder for Riverty (Afterpay New) buy-now-pay-later payments.
+
+    Riverty is the rebrand of AfterPay; the wire service name remains
+    ``afterpay``. The method supports Pay, Authorize/Capture/CancelAuthorize,
+    and Refund.
+    """
 
     def get_service_name(self) -> str:
         """Get the service name for Riverty payments."""
         return "afterpay"
 
     def get_allowed_service_parameters(self, action: str = "Pay") -> Dict[str, Any]:
-        """Get the allowed service parameters for Riverty payments based on action."""
+        """Get the allowed service parameters for Riverty payments based on action.
 
-        if action.lower() in ["pay"]:
+        Pay and Authorize share the same cart-line / customer trio.
+        Capture and CancelAuthorize reference the original via top-level
+        ``OriginalTransactionKey`` (handled by the SDK), no service params.
+        """
+        if action.lower() in ("pay", "authorize"):
             return {
                 "billingCustomer": {
                     "type": list,

buckaroo/models/payment_response.py

@@ -4,7 +4,7 @@
 This module provides response objects for payment transactions.
 """
 
-from typing import Dict, Any, Optional, List
+from typing import Any, Dict, Iterator, List, Optional
 from dataclasses import dataclass
 from enum import IntEnum
 
@@ -230,6 +230,7 @@ def _parse_response(self):
         self.request_errors = data.get("RequestErrors")
         self.related_transactions = data.get("RelatedTransactions")
         self.consumer_message = data.get("ConsumerMessage")
+        self.message = data.get("Message")
         self.order = data.get("Order")
         self.issuing_country = data.get("IssuingCountry")
         self.start_recurrent = data.get("StartRecurrent", False)
@@ -300,6 +301,112 @@ def get_service_parameter(self, parameter_name: str) -> Optional[Any]:
                     return param.value
         return None
 
+    _ERROR_TYPES = (
+        "ChannelErrors",
+        "ServiceErrors",
+        "ActionErrors",
+        "ParameterErrors",
+        "CustomParameterErrors",
+    )
+
+    @staticmethod
+    def _normalize_error_bucket(bucket: Any) -> List[Dict[str, Any]]:
+        """Coerce a ``RequestErrors`` bucket into a list of dict entries.
+
+        Real-world Buckaroo responses occasionally collapse a single-error
+        bucket into a bare dict, or supply unexpected scalars. Coerce both
+        shapes here so callers can iterate without type checks.
+        """
+        if isinstance(bucket, list):
+            return [entry for entry in bucket if isinstance(entry, dict)]
+        if isinstance(bucket, dict):
+            return [bucket]
+        return []
+
+    def _iter_error_entries(self) -> Iterator[Dict[str, Any]]:
+        """Yield error-entry dicts across every bucket in priority order."""
+        errors = self.request_errors
+        if not isinstance(errors, dict):
+            return
+        for bucket_name in self._ERROR_TYPES:
+            for entry in self._normalize_error_bucket(errors.get(bucket_name)):
+                yield entry
+
+    def has_error(self) -> bool:
+        """Return True when ``RequestErrors`` carries any usable entry."""
+        return next(self._iter_error_entries(), None) is not None
+
+    def get_first_error(self) -> Dict[str, Any]:
+        """Return the first error entry from ``RequestErrors``, or ``{}``."""
+        return next(self._iter_error_entries(), {})
+
+    def has_consumer_message(self) -> bool:
+        """Return True when the response carries a non-empty ConsumerMessage."""
+        message = self.consumer_message or {}
+        if isinstance(message, dict):
+            return bool(message.get("HtmlText"))
+        return False
+
+    def get_consumer_message(self) -> str:
+        """Return the consumer-facing HTML message, or ``''``."""
+        message = self.consumer_message or {}
+        if isinstance(message, dict):
+            return message.get("HtmlText") or ""
+        return ""
+
+    def has_message(self) -> bool:
+        """Return True when the top-level ``Message`` field is set."""
+        return bool(self.message)
+
+    def get_message(self) -> str:
+        """Return the top-level ``Message`` field, or ``''``."""
+        return self.message or ""
+
+    def has_sub_code_message(self) -> bool:
+        """Return True when ``Status.SubCode.Description`` is set."""
+        return bool(
+            self.status
+            and self.status.sub_code
+            and self.status.sub_code.description
+        )
+
+    def get_sub_code_message(self) -> str:
+        """Return the ``Status.SubCode.Description``, or ``''``."""
+        if self.has_sub_code_message():
+            return self.status.sub_code.description
+        return ""
+
+    def has_some_error(self) -> bool:
+        """Return True when any error/message channel carries text."""
+        return bool(self.get_some_error())
+
+    def get_some_error(self) -> str:
+        """Return the most-specific error message from the response.
+
+        Walks the response in priority order, mirroring PHP SDK's
+        ``TransactionResponse::getSomeError``:
+
+        1. The first entry from ``RequestErrors[*]`` (ChannelErrors,
+           ServiceErrors, ActionErrors, ParameterErrors,
+           CustomParameterErrors) → ``ErrorMessage``.
+        2. ``ConsumerMessage.HtmlText`` (consumer-facing copy).
+        3. Top-level ``Message`` (gateway-level message).
+        4. ``Status.SubCode.Description`` (e.g. Riverty 491 reason).
+
+        Returns ``''`` when none of those carry text.
+        """
+        for entry in self._iter_error_entries():
+            message = entry.get("ErrorMessage")
+            if message:
+                return message
+        if self.has_consumer_message():
+            return self.get_consumer_message()
+        if self.has_message():
+            return self.get_message()
+        if self.has_sub_code_message():
+            return self.get_sub_code_message()
+        return ""
+
     def to_dict(self) -> Dict[str, Any]:
         """Convert the response back to a dictionary."""
         return self._raw_data

tests/unit/builders/payments/capabilities/test_authorize_capture_capable.py

@@ -87,9 +87,10 @@ def test_authorize_encrypted_posts_action_AuthorizeEncrypted(self):
 # ---------------------------------------------------------------------------
 # capture()
 #
-# AuthorizeCaptureCapable.capture is dead code: BaseBuilder.capture shadows
-# it through the MRO on every real builder (see TestMroShadowing below). Any
-# test calling the mixin method directly would only exercise unreachable code.
+# ``AuthorizeCaptureCapable`` does not contribute ``capture``;
+# :meth:`BaseBuilder.capture` carries the full
+# ``original_transaction_key`` / ``amount`` signature for every builder.
+# Behavioural coverage lives in the BaseBuilder tests.
 #
 # ---------------------------------------------------------------------------
 # cancelAuthorize()
@@ -208,30 +209,13 @@ def _build(action="Pay", validate=True, strict_validation=False):
 class TestMroShadowing:
     """Pin how capability methods compose into a builder's MRO."""
 
-    def test_base_builder_capture_shadows_mixin_capture(self):
+    def test_capture_resolves_to_base_builder(self):
         _, client = wire_recording_http()
         builder = _ready_builder(client)
 
-        # The capture the instance resolves is BaseBuilder's (needs auth key).
+        # ``capture`` lives only on BaseBuilder; the mixin no longer ships one.
         assert type(builder).capture.__qualname__ == "BaseBuilder.capture"
-        # The mixin's simpler capture is still reachable via the class itself.
-        assert AuthorizeCaptureCapable.capture.__qualname__ == ("AuthorizeCaptureCapable.capture")
-
-    def test_mixin_capture_posts_capture_action_when_invoked_directly(self):
-        """Direct-invocation pin on the mixin's ``capture`` body.
-
-        No composed builder routes to this method because ``BaseBuilder.capture``
-        shadows it in MRO. The method is only callable as an unbound reference.
-        Pinned here so the shadowed logic still has a behavioral contract.
-        """
-        mock, client = wire_recording_http()
-        mock.queue(BuckarooMockRequest.json("POST", "*/json/transaction*", {"Key": "cap"}))
-        builder = _ready_builder(client)
-
-        response = AuthorizeCaptureCapable.capture(builder, validate=False)
-
-        assert recorded_action(mock) == "Capture"
-        assert response.key == "cap"
+        assert not hasattr(AuthorizeCaptureCapable, "capture")
 
 
 class TestMultiCapabilityBuilder:
@@ -240,10 +224,9 @@ class TestMultiCapabilityBuilder:
     def test_all_six_action_methods_invoke_and_post_expected_actions(self):
         """MRO-resolved action methods must each post the right Buckaroo Action.
 
-        ``capture`` resolves to :meth:`BaseBuilder.capture` (needs an auth
-        key) - the mixin's ``capture`` is dead code. This test pins both the
-        resolution AND the on-wire Action for every method on a composed
-        builder.
+        ``capture`` resolves to :meth:`BaseBuilder.capture` (the mixin no
+        longer ships one). This test pins both the resolution AND the on-wire
+        Action for every method on a composed builder.
         """
         mock, client = wire_recording_http()
         # One queued response per invoked action (six total).

tests/unit/builders/payments/test_concrete_builders_contract.py

@@ -52,7 +52,6 @@
         "authorize",
         "authorizeEncrypted",
         "cancelAuthorize",
-        "capture",
     ],
     BankTransferCapabilities: ["instantRefund", "payFastCheckout"],
     EncryptedPayCapable: ["payEncrypted"],
@@ -144,6 +143,7 @@ def test_capability_method_present_and_callable(
 # of — direct mixin plus transitive bases.
 EXPECTED_CAPABILITIES: Dict[str, set] = {
     "creditcard": {EncryptedPayCapable, AuthorizeCaptureCapable},
+    "riverty": {AuthorizeCaptureCapable},
     "ideal": {BankTransferCapabilities, InstantRefundCapable, FastCheckoutCapable},
     "paybybank": {BankTransferCapabilities, InstantRefundCapable, FastCheckoutCapable},
     "payconiq": {BankTransferCapabilities, InstantRefundCapable, FastCheckoutCapable},