Skip to content

fix(setups): align the payment setup request shape with the spec - #254

Open
rodrigojara-cko wants to merge 1 commit into
mainfrom
feat/INT-1725-payment-setup-shape-gaps
Open

rodrigojara-cko wants to merge 1 commit into
mainfrom
feat/INT-1725-payment-setup-shape-gaps

Conversation

@rodrigojara-cko

Copy link
Copy Markdown

Summary

The PaymentSetup schema nests the card fields under payment_methods.card.details and types the Apple Pay and Google Pay token as an object. The SDK sent the card fields at the top level of the card object and the wallet token as a string, so the API ignored them. This adds the spec shape, keeps the old card attributes working as deprecated aliases that write into the new shape, and adds the missing auto_confirm, metadata and instrument.type.

Changes

  • checkout_sdk/payments/setups/setups.py:
    • Card: adds details (CardDetails, the 11 card fields), token (CardToken), schemes and preferred_scheme. The flat attributes (number, last4, bin, scheme, expiry_month, expiry_year, name, cvv, stored, expires_on, allow_update) are deprecated: they keep working, emit a DeprecationWarning, and store their value under details, so it is sent where the API reads it.
    • ApplePay, GooglePay: token is now a WalletToken (id, expires_on). The flat expires_on is deprecated and moves under token. Adds schemes and preferred_scheme.
    • New enums PaymentSetupCardScheme (17 values) and GooglePayPreferredScheme (visa, mastercard). Card and Apple Pay reuse PreferredSchema.
    • PaymentMethods.auto_confirm, PaymentSetupsRequest.metadata, and the read-only PaymentSetupInstrument.type.
    • Card, ApplePay and GooglePay serialize their own attributes (to_json), so each value is sent once, under details or token, and serialization never triggers the deprecation warnings.
  • tests/payments/setups/payment_setups_serialization_test.py: literal wire keys, roundtrip and spec-shaped bodies for every new property, each deprecated attribute (sent only under details/token, warning emitted) and enum completeness.

API Reference

  • POST /payments/setups
  • PUT /payments/setups/{id}
  • GET /payments/setups/{id}
  • POST /payments/setups/{id}/confirm/{payment_method_name}

Breaking changes

Forced by the API, so this is a minor release, to be called out in the release notes: ApplePay.token and GooglePay.token change from str to WalletToken. A string token was never accepted by the API, so nothing that worked stops working. Every other replaced attribute is kept as a deprecated alias.

README

No change needed.

@agent-wall-e

agent-wall-e Bot commented Oct 15, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

  • exceeds_bounded_scope:277>250

Operational gates

  • ✅ jira_ticket (INT-1725)
  • ✅ independent_review

Files analysed: 2


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Oct 15, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
exceeds_bounded_scope — 277>250 classifying §2.1 M8 More than 250 non-test, non-doc, non-lockfile lines changed.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@agent-wall-e

agent-wall-e Bot commented Oct 15, 2026

Copy link
Copy Markdown

🟠 Advisory review: Concerns worth a look

This PR needs a human approval. Before you give it, these are the things I'd want resolved.

The PR restructures payment setup request bodies to match the API spec (nesting card fields under details, making wallet tokens objects). The logic is broadly correct but there are concrete bugs in the _moved_attribute helper and the _SerializesOwnAttributes.to_json that will cause silent failures or incorrect serialization.

Concerns

  • In _moved_attribute, the setter checks if not hasattr(self, container) before creating the container, but container is the string name of an attribute (e.g. 'details'), while hasattr(self, container) will return True if the attribute exists as a class annotation or a property on the class, not just if it has been set on the instance — meaning the guard may silently fail to initialize the nested object and then setattr(getattr(self, container), name, value) will crash with an AttributeError on the uninitialized object.
  • In _SerializesOwnAttributes.to_json, vars(self) returns the instance __dict__, which only contains attributes explicitly assigned to the instance. If details or token have never been assigned (e.g. a fresh Card() with no properties set), those nested objects won't appear, which is fine. However, when a nested object (e.g. CardDetails) itself does not inherit _SerializesOwnAttributes, json.dumps via JsonSerializer will serialize it — but to_json returns a plain dict of the outer object's vars, so the inner objects are handed off as raw Python objects to JsonSerializer. Whether JsonSerializer handles them correctly is not shown in this diff; if it reflects attributes via vars() or a custom encoder path, it may work, but this dependency on invisible code is a risk the reviewer should verify.
  • The deprecated setter in _moved_attribute calls setattr(self, container, container_type()) to initialize the container, but container_type() calls the container class with no arguments. If CardDetails or WalletToken have a constructor that requires arguments, this will raise — the diff shows no __init__ for these classes, so it appears safe, but the reviewer should confirm no __init__ is added elsewhere in the (truncated) file.
  • The expires_on field on CardDetails is serialized from a datetime object (e.g. datetime(2026, 6, 1, 10, 0, 0)), and the test asserts the wire value is '2026-06-01T10:00:00'. Since to_json returns vars(self) as a plain dict, the datetime value will be in the dict as a Python datetime — whether it gets serialized as that ISO string depends entirely on JsonSerializer, which is not in this diff.
  • The test test_card_round_trip_keeps_every_proper is truncated in the diff, so it cannot be fully reviewed; if it tests that deprecated setters round-trip through to_json correctly (i.e., values end up under details not at the top level), this is a critical correctness check that can't be confirmed.
  • The PaymentSetupInstrument.type field is marked readOnly in a comment but there is no enforcement — it can be written by callers, which could cause API errors if accidentally set on a request body. This is a minor footgun but worth noting.

⚠️ The diff was too large to read in full, so this review covers only part of the change.


This is not an approval. wall-e cannot auto-approve this PR — it is an opinion to help whoever does. Advisory review · us.anthropic.claude-sonnet-4-6 · wall-e 2026.06.19-02

The PaymentSetup schema (POST /payments/setups, PUT and GET
/payments/setups/{id}, POST /payments/setups/{id}/confirm/{payment_method_name})
nests the card fields under payment_methods.card.details and declares the
Apple Pay and Google Pay token as an object. The SDK sent the card fields
at the top level of the card object and the wallet token as a string, so the
API ignored them.

- Card: add details (CardDetails), token (CardToken), schemes and
  preferred_scheme. The flat card attributes (number, last4, bin, scheme,
  expiry_month, expiry_year, name, cvv, stored, expires_on, allow_update)
  are deprecated: they keep working, emit a DeprecationWarning, and store
  their value under details, so it is sent where the API reads it.
- ApplePay and GooglePay: token is now a WalletToken object (id,
  expires_on), the shape the API expects; the flat expires_on is deprecated
  and moves under token. Add schemes and preferred_scheme.
- PaymentMethods: add auto_confirm. PaymentSetupsRequest: add metadata.
- PaymentSetupInstrument: add the read-only type.
- New enums PaymentSetupCardScheme and GooglePayPreferredScheme; card and
  Apple Pay reuse PreferredSchema, whose values match.

The wallet token type change from str to object is forced by the API: a
string token was never accepted, so this ships in a minor release.
@rodrigojara-cko
rodrigojara-cko force-pushed the feat/INT-1725-payment-setup-shape-gaps branch from 84c6e99 to 2559798 Compare October 15, 2026 14:44
@agent-wall-e

agent-wall-e Bot commented Oct 15, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

  • exceeds_bounded_scope:277>250

Operational gates

  • ✅ jira_ticket (INT-1725)
  • ✅ independent_review

Files analysed: 2


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Oct 15, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
exceeds_bounded_scope — 277>250 classifying §2.1 M8 More than 250 non-test, non-doc, non-lockfile lines changed.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant