From b7d8c2f7582d763282bf1f017bfa4e437fcc712d Mon Sep 17 00:00:00 2001 From: Vildan Bina Date: Tue, 7 Jul 2026 14:44:17 +0200 Subject: [PATCH] feat(BTI-584): update Klarna MOR to DataRequestKey flow Klarna no longer returns a reservation number; follow-up actions reference the prior Reserve through the Buckaroo DataRequestKey service parameter. - Fix cancelReservation to post to /json/DataRequest with DataRequestKey (was posting to /json/transaction with OriginalTransactionKey) - Add updateReservation and extendReservation DataRequest actions - Align allowed service parameters and required_fields per action with the BPS Klarna docs (operatingCountry mandatory on Reserve; Pay requires an invoice) - Remove AddShippingInfo: the gateway rejects it for the klarna service (ActionUnknown); shipping details ride on the Pay request instead - Refund keeps the base OriginalTransactionKey path Verified end-to-end against the Buckaroo test gateway: Reserve -> hosted-page approval -> ExtendReservation -> Pay (captured) -> Refund all succeed. --- buckaroo/builders/payments/klarna_builder.py | 149 +++++++--- tests/feature/payments/test_klarna.py | 79 +++++ .../builders/payments/test_klarna_builder.py | 270 ++++++++++++++---- .../test_service_parameter_validator.py | 10 +- 4 files changed, 410 insertions(+), 98 deletions(-) diff --git a/buckaroo/builders/payments/klarna_builder.py b/buckaroo/builders/payments/klarna_builder.py index a74e2cb..e19a816 100644 --- a/buckaroo/builders/payments/klarna_builder.py +++ b/buckaroo/builders/payments/klarna_builder.py @@ -1,12 +1,52 @@ from __future__ import annotations -from typing import Dict, Any, Optional +from typing import Dict, Any from buckaroo.models.payment_response import PaymentResponse from .payment_builder import PaymentBuilder class KlarnaBuilder(PaymentBuilder): - """Builder for Klarna MOR (Merchant of Record) payments.""" + """Builder for Klarna MOR (Merchant of Record) payments. + + Klarna no longer returns a reservation number. Every follow-up action + (CancelReservation, UpdateReservation, ExtendReservation, and Pay) + references the prior Reserve through the Buckaroo ``DataRequestKey`` carried + as a service parameter. The reservation actions post to ``/json/DataRequest``; + Pay and Refund are transaction requests. Shipping details are attached to the + Pay request (``shippingMethod`` / ``company`` / ``trackingNumber``); the + gateway has no standalone AddShippingInfo action for this service. + """ + + def required_fields(self, action: str = "Pay") -> Dict[str, Any]: + """Narrow the base required-field set per action. + + Reserve only needs currency + invoice; Pay needs currency + amount; the + DataRequest follow-up actions need none of the base transaction fields. + Refund and any other action fall through to the base requirements. + """ + action = action.lower() + + if action == "reserve": + return { + "currency": self._currency, + "invoice": self._invoice, + } + + if action == "pay": + return { + "currency": self._currency, + "amount_debit": self._amount_debit, + "invoice": self._invoice, + } + + if action in ( + "cancelreservation", + "updatereservation", + "extendreservation", + ): + return {} + + return super().required_fields(action) def get_service_name(self) -> str: """Get the service name for Klarna payments.""" @@ -23,20 +63,30 @@ def get_allowed_service_parameters(self, action: str = "Pay") -> Dict[str, Any]: "required": True, "description": "Key of the prior Klarna Reserve", }, + "article": { + "type": list, + "required": False, + "description": "Articles to pay for on a partial delivery", + }, + "shippingMethod": { + "type": str, + "required": False, + "description": "Shipping method", + }, + "company": { + "type": str, + "required": False, + "description": "Shipping company name", + }, + "trackingNumber": { + "type": str, + "required": False, + "description": "Shipping tracking number", + }, } if action == "reserve": return { - "billingCustomer": { - "type": list, - "required": True, - "description": "Billing customer information", - }, - "shippingCustomer": { - "type": list, - "required": True, - "description": "Shipping customer information", - }, "article": { "type": list, "required": True, @@ -44,9 +94,19 @@ def get_allowed_service_parameters(self, action: str = "Pay") -> Dict[str, Any]: }, "operatingCountry": { "type": str, - "required": False, + "required": True, "description": "Operating country code", }, + "billingCustomer": { + "type": list, + "required": False, + "description": "Billing customer information", + }, + "shippingCustomer": { + "type": list, + "required": False, + "description": "Shipping customer information", + }, "pno": { "type": str, "required": False, @@ -64,37 +124,48 @@ def get_allowed_service_parameters(self, action: str = "Pay") -> Dict[str, Any]: }, } - if action == "cancelreservation": - return {} + if action in ("cancelreservation", "extendreservation"): + return { + "dataRequestKey": { + "type": str, + "required": True, + "description": "Buckaroo data request key of the prior Reserve", + }, + } + + if action == "updatereservation": + return { + "dataRequestKey": { + "type": str, + "required": True, + "description": "Buckaroo data request key of the prior Reserve", + }, + "article": { + "type": list, + "required": False, + "description": "Updated Klarna articles", + }, + "shippingCustomer": { + "type": list, + "required": False, + "description": "Updated shipping customer information", + }, + } return {} def reserve(self: "PaymentBuilder", validate: bool = True) -> PaymentResponse: - payment_request = self.build("Reserve", validate=validate) - request_data = payment_request.to_dict() - - return self._post_data_request(request_data) - - def cancelReservation( - self: "PaymentBuilder", - original_transaction_key: Optional[str] = None, - validate: bool = True, - ) -> PaymentResponse: - """Cancel a previously-reserved Klarna transaction. - - Mirrors :meth:`AuthorizeCaptureCapable.cancelAuthorize` but with the - ``CancelReservation`` action. - """ - txn_key = original_transaction_key or self._payload.get("original_transaction_key") - if not txn_key: - raise ValueError( - "Original transaction key is required for cancelReservation " - "(provide 'original_transaction_key' in payload)" - ) + return self._post_data_request(payment_request.to_dict()) + def cancelReservation(self: "PaymentBuilder", validate: bool = True) -> PaymentResponse: payment_request = self.build("CancelReservation", validate=validate) - request_data = payment_request.to_dict() - request_data["OriginalTransactionKey"] = txn_key + return self._post_data_request(payment_request.to_dict()) + + def updateReservation(self: "PaymentBuilder", validate: bool = True) -> PaymentResponse: + payment_request = self.build("UpdateReservation", validate=validate) + return self._post_data_request(payment_request.to_dict()) - return self._post_transaction(request_data) + def extendReservation(self: "PaymentBuilder", validate: bool = True) -> PaymentResponse: + payment_request = self.build("ExtendReservation", validate=validate) + return self._post_data_request(payment_request.to_dict()) diff --git a/tests/feature/payments/test_klarna.py b/tests/feature/payments/test_klarna.py index 7d8eb90..faac666 100644 --- a/tests/feature/payments/test_klarna.py +++ b/tests/feature/payments/test_klarna.py @@ -59,3 +59,82 @@ def test_klarna_pay_as_capture_attaches_data_request_key(self, buckaroo, mock_st services = body["Services"]["ServiceList"] names = [p["Name"] for p in services[0]["Parameters"]] assert "Datarequestkey" in names + + def test_klarna_cancel_reservation_posts_data_request_key(self, buckaroo, mock_strategy): + """Follow-up actions carry the Buckaroo DataRequestKey (no reservation + number) and post to /json/DataRequest.""" + response_body = Helpers.pending_redirect_response("klarna", action="CancelReservation") + mock_strategy.queue(BuckarooMockRequest.json("POST", "*/json/DataRequest", response_body)) + response = buckaroo.payments.create_payment( + "klarna", + Helpers.standard_payload( + invoice="INV-KLARNA-CAN", + service_parameters={"dataRequestKey": "RES-DRK-2"}, + ), + ).cancelReservation() + + assert response.key == response_body["Key"] + body = json.loads(mock_strategy.calls[-1]["data"]) + service = body["Services"]["ServiceList"][0] + assert service["Action"] == "CancelReservation" + assert "OriginalTransactionKey" not in body + names = [p["Name"] for p in service["Parameters"]] + assert "Datarequestkey" in names + + def test_klarna_update_reservation_dispatches_to_data_request(self, buckaroo, mock_strategy): + response_body = Helpers.pending_redirect_response("klarna", action="UpdateReservation") + mock_strategy.queue(BuckarooMockRequest.json("POST", "*/json/DataRequest", response_body)) + response = buckaroo.payments.create_payment( + "klarna", + Helpers.standard_payload( + invoice="INV-KLARNA-UPD", + service_parameters={ + "dataRequestKey": "RES-DRK-3", + "article": [{"description": "Widget", "quantity": "1", "price": "10.00"}], + }, + ), + ).updateReservation() + + assert response.key == response_body["Key"] + assert ( + json.loads(mock_strategy.calls[-1]["data"])["Services"]["ServiceList"][0]["Action"] + == "UpdateReservation" + ) + + def test_klarna_extend_reservation_dispatches_to_data_request(self, buckaroo, mock_strategy): + response_body = Helpers.pending_redirect_response("klarna", action="ExtendReservation") + mock_strategy.queue(BuckarooMockRequest.json("POST", "*/json/DataRequest", response_body)) + response = buckaroo.payments.create_payment( + "klarna", + Helpers.standard_payload( + invoice="INV-KLARNA-EXT", + service_parameters={"dataRequestKey": "RES-DRK-4"}, + ), + ).extendReservation() + + assert response.key == response_body["Key"] + assert ( + json.loads(mock_strategy.calls[-1]["data"])["Services"]["ServiceList"][0]["Action"] + == "ExtendReservation" + ) + + def test_klarna_pay_carries_shipping_details_on_transaction(self, buckaroo, mock_strategy): + """Shipping details ride on the Pay transaction (no AddShippingInfo action).""" + capture_body = Helpers.success_response( + overrides={"Key": "PAY-K-SHP", "AmountDebit": 22.45, "ServiceCode": "klarna"} + ) + mock_strategy.queue(BuckarooMockRequest.json("POST", "*/json/transaction", capture_body)) + builder = buckaroo.payments.create_payment( + "klarna", + Helpers.standard_payload(invoice="INV-KLARNA-SHP", amount=22.45), + ) + builder.add_parameter("dataRequestKey", "RES-DRK-5") + builder.add_parameter("trackingNumber", "AAAA1234567890") + response = builder.pay() + + assert response.key == "PAY-K-SHP" + service = json.loads(mock_strategy.calls[-1]["data"])["Services"]["ServiceList"][0] + assert service["Action"] == "Pay" + names = [p["Name"] for p in service["Parameters"]] + assert "Datarequestkey" in names + assert "Trackingnumber" in names diff --git a/tests/unit/builders/payments/test_klarna_builder.py b/tests/unit/builders/payments/test_klarna_builder.py index cf92b4e..739e46c 100644 --- a/tests/unit/builders/payments/test_klarna_builder.py +++ b/tests/unit/builders/payments/test_klarna_builder.py @@ -1,10 +1,11 @@ """Per-builder unit tests for :class:`KlarnaBuilder`. -Covers construction, service-name shape, allowed-parameter snapshots for every -supported action including the grouped article / line-item structure, the -mixin-free baseline (KlarnaBuilder composes no capability mixins despite the -docstring mention of "bank transfer capabilities"), and an end-to-end -``pay()`` dispatch through ``MockBuckaroo``. Phase 7.20. +Klarna MOR no longer returns a reservation number: every follow-up action +(``CancelReservation``, ``UpdateReservation``, ``ExtendReservation``, +``AddShippingInfo``) is a DataRequest keyed on the Buckaroo ``DataRequestKey`` +service parameter and posts to ``/json/DataRequest``. ``Pay``/``Refund`` remain +transaction requests. The source also narrows :meth:`required_fields` per +action so the DataRequest methods pass ``_validate_required_fields``. """ from __future__ import annotations @@ -15,6 +16,7 @@ from tests.support.builders import populate_required_fields from tests.support.mock_buckaroo import MockBuckaroo from tests.support.mock_request import BuckarooMockRequest +from tests.support.recording_mock import recorded_action, recorded_request, wire_recording_http def test_construct_with_buckaroo_client_returns_payment_builder(client): @@ -26,15 +28,39 @@ def test_get_service_name_returns_klarna(client): assert KlarnaBuilder(client).get_service_name() == "klarna" +# --------------------------------------------------------------------------- +# get_allowed_service_parameters + + def test_get_allowed_service_parameters_pay_snapshot(client): - """Pay-as-capture references the prior Reserve via ``dataRequestKey`` as a - service parameter. Cart contents are reused server-side from the Reserve.""" + """Pay-as-capture references the prior Reserve via ``dataRequestKey`` and + optionally carries partial-delivery articles and shipping details.""" assert KlarnaBuilder(client).get_allowed_service_parameters("Pay") == { "dataRequestKey": { "type": str, "required": True, "description": "Key of the prior Klarna Reserve", }, + "article": { + "type": list, + "required": False, + "description": "Articles to pay for on a partial delivery", + }, + "shippingMethod": { + "type": str, + "required": False, + "description": "Shipping method", + }, + "company": { + "type": str, + "required": False, + "description": "Shipping company name", + }, + "trackingNumber": { + "type": str, + "required": False, + "description": "Shipping tracking number", + }, } @@ -47,20 +73,9 @@ def test_get_allowed_service_parameters_is_case_insensitive_for_pay(client): def test_get_allowed_service_parameters_reserve_snapshot(client): - """Reserve action drives the Klarna MOR hosted-page flow. Same required - cart trio as Pay (billingCustomer, shippingCustomer, article) plus - optional Klarna-specific keys (operatingCountry, pno, gender, locale).""" + """Reserve drives the Klarna MOR hosted-page flow. Per BPS docs only the + article list and operatingCountry are mandatory; customer info is optional.""" assert KlarnaBuilder(client).get_allowed_service_parameters("Reserve") == { - "billingCustomer": { - "type": list, - "required": True, - "description": "Billing customer information", - }, - "shippingCustomer": { - "type": list, - "required": True, - "description": "Shipping customer information", - }, "article": { "type": list, "required": True, @@ -68,9 +83,19 @@ def test_get_allowed_service_parameters_reserve_snapshot(client): }, "operatingCountry": { "type": str, - "required": False, + "required": True, "description": "Operating country code", }, + "billingCustomer": { + "type": list, + "required": False, + "description": "Billing customer information", + }, + "shippingCustomer": { + "type": list, + "required": False, + "description": "Shipping customer information", + }, "pno": { "type": str, "required": False, @@ -96,8 +121,94 @@ def test_get_allowed_service_parameters_is_case_insensitive_for_reserve(client): ) == builder.get_allowed_service_parameters("Reserve") +def test_get_allowed_service_parameters_cancelreservation_requires_data_request_key(client): + """CancelReservation and ExtendReservation only need the DataRequestKey.""" + expected = { + "dataRequestKey": { + "type": str, + "required": True, + "description": "Buckaroo data request key of the prior Reserve", + }, + } + builder = KlarnaBuilder(client) + assert builder.get_allowed_service_parameters("CancelReservation") == expected + assert builder.get_allowed_service_parameters("cancelreservation") == expected + + +def test_get_allowed_service_parameters_extendreservation_matches_cancel(client): + builder = KlarnaBuilder(client) + assert builder.get_allowed_service_parameters( + "ExtendReservation" + ) == builder.get_allowed_service_parameters("CancelReservation") + + +def test_get_allowed_service_parameters_updatereservation_snapshot(client): + assert KlarnaBuilder(client).get_allowed_service_parameters("UpdateReservation") == { + "dataRequestKey": { + "type": str, + "required": True, + "description": "Buckaroo data request key of the prior Reserve", + }, + "article": { + "type": list, + "required": False, + "description": "Updated Klarna articles", + }, + "shippingCustomer": { + "type": list, + "required": False, + "description": "Updated shipping customer information", + }, + } + + def test_get_allowed_service_parameters_unsupported_action_returns_empty(client): + # Refund carries no service params; AddShippingInfo is not a klarna action + # (the gateway rejects it — shipping details ride on Pay). assert KlarnaBuilder(client).get_allowed_service_parameters("Refund") == {} + assert KlarnaBuilder(client).get_allowed_service_parameters("AddShippingInfo") == {} + + +# --------------------------------------------------------------------------- +# required_fields override + + +class TestRequiredFields: + def test_reserve_requires_currency_and_invoice(self, client): + builder = KlarnaBuilder(client).currency("EUR").invoice("INV-1") + assert builder.required_fields("Reserve") == {"currency": "EUR", "invoice": "INV-1"} + + def test_pay_requires_currency_amount_and_invoice(self, client): + """The gateway rejects Pay without an invoice number.""" + builder = KlarnaBuilder(client).currency("EUR").amount(12.34).invoice("INV-9") + assert builder.required_fields("Pay") == { + "currency": "EUR", + "amount_debit": 12.34, + "invoice": "INV-9", + } + + def test_data_request_actions_require_nothing(self, client): + builder = KlarnaBuilder(client) + assert builder.required_fields("CancelReservation") == {} + assert builder.required_fields("UpdateReservation") == {} + assert builder.required_fields("ExtendReservation") == {} + + def test_refund_falls_through_to_base(self, client): + """Refund keeps the full base required-field set (like every builder).""" + assert set(KlarnaBuilder(client).required_fields("Refund")) == { + "currency", + "amount_debit", + "description", + "invoice", + "return_url", + "return_url_cancel", + "return_url_error", + "return_url_reject", + } + + +# --------------------------------------------------------------------------- +# Pay / Refund dispatch through the transaction endpoint def test_pay_dispatches_klarna_service_through_mock_buckaroo(): @@ -120,45 +231,98 @@ def test_pay_dispatches_klarna_service_through_mock_buckaroo(): mock.assert_all_consumed() -def test_get_allowed_service_parameters_cancelreservation_is_empty(client): - """CancelReservation only needs OriginalTransactionKey at request level.""" - assert KlarnaBuilder(client).get_allowed_service_parameters("CancelReservation") == {} - assert KlarnaBuilder(client).get_allowed_service_parameters("cancelreservation") == {} - +# --------------------------------------------------------------------------- +# DataRequest follow-up actions — post to /json/DataRequest with the action name -def test_cancel_reservation_requires_original_transaction_key(client): - """Missing key raises ValueError, mirroring cancelAuthorize.""" - import pytest - builder = populate_required_fields(KlarnaBuilder(client), amount=10.0) - with pytest.raises(ValueError, match="Original transaction key is required"): - builder.cancelReservation(original_transaction_key="") +class TestReserve: + def test_posts_reserve_to_data_request_endpoint(self): + mock, client = wire_recording_http() + mock.queue( + BuckarooMockRequest.json( + "POST", + "*/json/DataRequest*", + {"Key": "KL-RES-1", "Status": {"Code": {"Code": 190}}}, + ) + ) + builder = KlarnaBuilder(client).currency("EUR").invoice("INV-1") + + response = builder.reserve(validate=False) + + assert "/json/DataRequest" in mock.calls[0]["url"] + assert recorded_action(mock) == "Reserve" + assert recorded_request(mock)["Services"]["ServiceList"][0]["Name"] == "klarna" + assert response.key == "KL-RES-1" + mock.assert_all_consumed() + + +class TestCancelReservation: + def test_posts_cancel_reservation_to_data_request_without_original_key(self): + mock, client = wire_recording_http() + mock.queue( + BuckarooMockRequest.json( + "POST", + "*/json/DataRequest*", + {"Key": "KL-CAN-1", "Status": {"Code": {"Code": 190}}}, + ) + ) + builder = KlarnaBuilder(client) + builder.add_parameter("dataRequestKey", "RES-DRK-1") + + response = builder.cancelReservation(validate=False) + + assert "/json/DataRequest" in mock.calls[0]["url"] + assert recorded_action(mock) == "CancelReservation" + sent = recorded_request(mock) + assert "OriginalTransactionKey" not in sent + names = [p["Name"] for p in sent["Services"]["ServiceList"][0]["Parameters"]] + assert "Datarequestkey" in names + assert response.key == "KL-CAN-1" + mock.assert_all_consumed() + + +class TestUpdateReservation: + def test_posts_update_reservation_to_data_request(self): + mock, client = wire_recording_http() + mock.queue( + BuckarooMockRequest.json( + "POST", + "*/json/DataRequest*", + {"Key": "KL-UPD-1", "Status": {"Code": {"Code": 190}}}, + ) + ) + builder = KlarnaBuilder(client) + response = builder.updateReservation(validate=False) -def test_cancel_reservation_dispatches_action_with_original_transaction_key(): - """cancelReservation builds action="CancelReservation" with the key in payload.""" - from unittest.mock import MagicMock + assert "/json/DataRequest" in mock.calls[0]["url"] + assert recorded_action(mock) == "UpdateReservation" + assert response.key == "KL-UPD-1" + mock.assert_all_consumed() - class _StubResponse: - def to_dict(self): - return {"Status": {"Code": {"Code": 190}}} - captured = {} +class TestExtendReservation: + def test_posts_extend_reservation_to_data_request(self): + mock, client = wire_recording_http() + mock.queue( + BuckarooMockRequest.json( + "POST", + "*/json/DataRequest*", + {"Key": "KL-EXT-1", "Status": {"Code": {"Code": 190}}}, + ) + ) + builder = KlarnaBuilder(client) - class _StubHttp: - def post(self, path, data): - captured["path"] = path - captured["data"] = data - return _StubResponse() + response = builder.extendReservation(validate=False) - stub_client = MagicMock() - stub_client.http_client = _StubHttp() + assert "/json/DataRequest" in mock.calls[0]["url"] + assert recorded_action(mock) == "ExtendReservation" + assert response.key == "KL-EXT-1" + mock.assert_all_consumed() - builder = populate_required_fields(KlarnaBuilder(stub_client), amount=49.95) - response = builder.cancelReservation(original_transaction_key="RES-KEY-9") - assert response.to_dict()["Status"]["Code"]["Code"] == 190 - assert captured["path"] == "/json/transaction" - sent = captured["data"] - assert sent["OriginalTransactionKey"] == "RES-KEY-9" - assert sent["Services"]["ServiceList"][0]["Action"] == "CancelReservation" +def test_klarna_has_no_add_shipping_info_action(): + """AddShippingInfo is not a klarna action — the gateway rejects it, so the + builder exposes no such method (shipping details ride on Pay).""" + _, client = wire_recording_http() + assert not hasattr(KlarnaBuilder(client), "addShippingInfo") diff --git a/tests/unit/services/test_service_parameter_validator.py b/tests/unit/services/test_service_parameter_validator.py index 44ec8a3..7a394b6 100644 --- a/tests/unit/services/test_service_parameter_validator.py +++ b/tests/unit/services/test_service_parameter_validator.py @@ -218,26 +218,24 @@ def test_validate_required_parameters_raises_required_missing_for_single_gap(): def test_validate_required_parameters_raises_validation_error_for_multiple_gaps(): validator = _validator_for(KlarnaBuilder) - # Klarna Reserve requires billingCustomer, shippingCustomer, article — all missing. + # Klarna Reserve requires article and operatingCountry — both missing. with pytest.raises(ParameterValidationError) as exc: validator.validate_required_parameters([], action="Reserve") # Multiple missing -> plain ParameterValidationError (not Required...), # and the message lists each missing name. assert not isinstance(exc.value, RequiredParameterMissingError) msg = str(exc.value) - assert "billingCustomer" in msg - assert "shippingCustomer" in msg assert "article" in msg + assert "operatingCountry" in msg def test_validate_required_parameters_accepts_grouped_group_type_as_satisfying_requirement(): validator = _validator_for(KlarnaBuilder) params = [ - Parameter(name="FirstName", value="Jane", group_type="billingCustomer"), - Parameter(name="FirstName", value="John", group_type="shippingCustomer"), Parameter(name="Identifier", value="A1", group_type="article"), + Parameter(name="OperatingCountry", value="NL"), ] - # All three required group_types are present via grouped parameters. + # The grouped ``article`` requirement plus the plain operatingCountry are met. validator.validate_required_parameters(params, action="Reserve")