Skip to content

Feature/INT-1702 - Airline and accommodation sub-tree model alignment - #384

Merged
david-ruiz-cko merged 3 commits into
masterfrom
feature/INT-1702
Sep 29, 2026
Merged

david-ruiz-cko merged 3 commits into
masterfrom
feature/INT-1702

Conversation

@david-ruiz-cko

Copy link
Copy Markdown
Contributor

This pull request introduces comprehensive improvements to the PHP SDK's payment and processing data models, focusing on documentation clarity, alignment with API specifications, and enhanced support for accommodation and airline payment features. The changes include detailed docblocks for classes and properties, deprecation notices for outdated fields, and new or updated models for handling accommodation and airline data across different payment flows.

Documentation and Specification Alignment

  • Added or improved docblocks for nearly all major payment-related classes (e.g., AccommodationData, AccommodationGuest, AccommodationRoom, Passenger, FlightLegDetails, airline data classes) to clarify their purpose and usage. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]
  • Updated property annotations to use correct array/object types, added [Optional] markers, and provided detailed notes on API quirks, expected types, and backward compatibility. [1] [2] [3] [4] [5] [6]

Accommodation and Airline Data Enhancements

  • Added new properties to AccommodationData (e.g., property_phone, customer_service_phone) and introduced the AccommodationPhone model to support richer accommodation contact details. [1] [2]
  • Refined airline and accommodation data handling in processing and payment context classes, ensuring correct property names and types per API requirements (e.g., plan instead of billing_plan, ticket/passenger as objects not arrays, use of shared models). [1] [2] [3]

Deprecations and Backwards Compatibility

  • Added deprecation notices for properties and classes no longer supported by the API (e.g., service_class in FlightLegDetails, PaymentContextsPartnerCustomerRiskData), with guidance on preferred alternatives. [1] [2]
  • Clarified which properties are retained for backwards compatibility and which are ignored by the gateway. [1] [2]

Schema and Model Corrections

  • Updated several models to use the correct local versions (e.g., PaymentSetupAccommodationAddress, PaymentSetupAccommodationRoom) where the schema differs from the main payment models. [1] [2]
  • Improved handling and documentation of airline and accommodation arrays in processing settings, payment contexts, and main payment requests. [1] [2] [3]

New Models

  • Introduced AccommodationPhone to encapsulate property and customer service phone details for accommodations.

These changes significantly improve the maintainability, correctness, and clarity of the SDK's payment data models, making integration with the Checkout API more predictable and robust.

@david-ruiz-cko
david-ruiz-cko requested a review from a team September 28, 2026 09:42
@agent-wall-e

agent-wall-e Bot commented Sep 28, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

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

Classification reasons

  • exceeds_bounded_scope:452>250

Operational gates

  • ✅ jira_ticket (INT-1702)
  • ✅ independent_review

Files analysed: 26


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Sep 28, 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 — 452>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 Sep 28, 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.

This PR renames properties and changes types across several SDK model classes to align with API specifications, adds new models, and includes serialization tests. The changes are well-documented and mostly correct, but there are a few concrete concerns a reviewer should verify.

Concerns

  • Breaking change without major version bump: PaymentContextsAirlineData::$tickets → $ticket and ::$passengers → $passenger, plus Industry::$airline_data → $airline and ::$accommodation_data → $accommodation, plus PaymentContextsProcessing::$billing_plan → $plan are all silent breaking changes for any caller using the old property names — PHP public properties mean existing callers silently set a property that no longer exists on the serialized output, and there is no deprecation period for the old names unlike the service_class deprecation.
  • The date '2026-09-25' appears repeatedly in comments and test docblocks as the sandbox verification date — this is a future date (relative to 2024/2025), which is suspicious and should be verified as correct or corrected.
  • AccommodationData::$property_phone and ::$customer_service_phone are typed as AccommodationPhone[] (arrays) in the docblock, but the PR description says these are phone details for a single property/customer-service contact — if the API expects a single object here, the array type annotation and corresponding usage guidance would be wrong.
  • The PaymentContextsProcessing class now imports and uses AccommodationData from Checkout\Payments namespace, but the diff for Setups/Common/Industry/AccommodationData establishes that the setups endpoint requires a different schema (PaymentSetupAccommodationAddress, PaymentSetupAccommodationRoom); PaymentContextsProcessing using the base AccommodationData is likely correct for /payment-contexts but this cross-namespace sharing should be confirmed against the actual API spec.
  • The truncated test in AirlineDataSerializationTest verifies exact key ordering via array_keys() assertions — this is fragile if PHP's JSON serialization order ever changes or properties are added, but more importantly the truncation means the full test coverage cannot be assessed.
  • The integration test shouldCreateHostedPaymentsPageSessionWithAirlineData in HostedPaymentsIntegrationTest does not appear to have a @test annotation in the diff header (it does have one in the body), but the existing test shouldCreateAndGetHostedPaymentsPageDetails lacks @test too — consistent with the existing pattern, so likely fine, but worth confirming the test runner picks it up.
  • The ProcessingSettings monetary fields (discount_amount, shipping_amount, tax_amount, duty_amount, shipping_tax_amount, original_order_amount) are changed from @var int to @var float — this is a documentation-only fix in PHP (no runtime enforcement), so existing callers sending integers continue to work, which is correct and non-breaking.

⚠️ 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

@agent-wall-e

agent-wall-e Bot commented Sep 28, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

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

Classification reasons

  • exceeds_bounded_scope:458>250

Operational gates

  • ✅ jira_ticket (INT-1702)
  • ✅ independent_review

Files analysed: 26


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Sep 28, 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 — 458>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

@david-ruiz-cko
david-ruiz-cko requested a review from a team September 29, 2026 09:06
@agent-wall-e

agent-wall-e Bot commented Sep 29, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

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

Classification reasons

  • exceeds_bounded_scope:477>250

Operational gates

  • ✅ jira_ticket (INT-1702)
  • ✅ independent_review

Files analysed: 27


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Sep 29, 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 — 477>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

@david-ruiz-cko
david-ruiz-cko merged commit 3e6b2d1 into master Sep 29, 2026
6 checks passed
@david-ruiz-cko
david-ruiz-cko deleted the feature/INT-1702 branch September 29, 2026 15:29
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.

2 participants