Skip to content

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

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

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

Conversation

@david-ruiz-cko

Copy link
Copy Markdown
Contributor

This pull request introduces several new payment-related data models and improves the documentation and type annotations for existing models in the CheckoutSdk::Payments module. The changes enhance clarity, ensure better alignment with the payment API specification, and add support for new payment attributes such as accommodation and aggregator data. The most important changes are grouped below:

New Payment Data Models

  • Added AccommodationData and related classes (AccommodationAddress, AccommodationGuest, AccommodationRoom, AccommodationPhone) to represent accommodation booking information in payments.
  • Introduced Aggregator class to capture payment aggregator details.
  • Added PartnerCustomerRiskData class for merchant-specific key-value risk data.

New Enumerations and Types

  • Added PanProcessedType, ProcessingCardType, and AchServiceType enums to specify PAN preference, card processing type, and ACH service type, respectively. [1] [2] [3]

Improvements to Airline Data Models

  • Enhanced AirlineData documentation to clarify the usage and cardinality of the passenger attribute, and updated type annotations for its attributes.
  • Improved Passenger and PassengerAddress documentation, specifying required formats and adding examples. [1] [2]
  • Updated FlightLegDetails to provide detailed attribute documentation and align property names and types with the API specification (e.g., class_of_travelling, stop_over_code).

Type Annotation and Documentation Improvements

  • Standardized array type annotations in ProcessingData (e.g., Array(String) instead of Array<String>), and improved documentation for accommodation and airline data attributes. [1] [2] [3]

Dependency Management

  • Updated payments.rb to require the newly added files and remove an obsolete require statement for sender/ticket. [1] [2]

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

agent-wall-e Bot commented Sep 28, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • security_sensitive_path:lib/checkout_sdk/payments/pan_processed_type.rb

Operational gates

  • ✅ jira_ticket (INT-1702)
  • ✅ independent_review

Files analysed: 18


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
security_sensitive_path — lib/checkout_sdk/payments/pan_processed_type.rb classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

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: Looks good to me

This PR still needs a human approval — wall-e cannot auto-approve it. For what it's worth, I read the diff and found nothing I'd block on.

Adds accommodation and aggregator data models, fixes two misnamed FlightLegDetails attributes (service_class→class_of_travelling, stopover_code→stop_over_code), moves Ticket out of the sender namespace, and adds thorough serialization and integration specs covering all the changes.

What I checked

  • The two renamed FlightLegDetails attributes (class_of_travelling, stop_over_code) are verified both in the serialization spec (asserting the old names are absent from the wire body) and the integration spec, so a regression would be caught.
  • The Ticket class is correctly moved from sender/ticket.rb to ticket.rb at the Payments module level, the old require is removed, the new one added, and the class definition is identical in module nesting — no naming collision or double-define risk.
  • The passenger cardinality documentation is unusually detailed and internally consistent: the table of accepted shapes, the nil/empty-array rejection note, and the matching serialization tests ('omits passenger entirely when it is never assigned') all agree.
  • AccommodationRoom.rate and number_of_nights_at_room_rate are documented and implemented as String, with a comment explaining the divergence from PaymentSetupAccommodationRoom — the asymmetry is intentional and flagged.
  • ProcessingCardType uses lowercase 'credit'/'debit' deliberately, the spec comment calls this out explicitly, and the test locks the values, making accidental normalisation detectable.
  • The processing_settings_new_types_spec asserts that partner_customer_risk_data is a single object (not an array), which matches the documented spec disagreement and the modelled class — consistent.
  • The integration spec hits the live sandbox (default_sdk.payments.request_payment) and would fail in CI if credentials are unavailable; this is normal for integration specs but a reviewer should confirm the CI environment handles this correctly.
  • The diff is truncated (processing_settings.rb new attributes not fully shown), so the attr_accessor declarations for surcharge_amount, foreign_retailer_amount, reconciliation_id, pan_preference, provision_network_token, card_type, service_type, aggregator, and partner_customer_risk_data cannot be fully verified — but the serialization spec exercises all nine and would fail if any were missing.

⚠️ 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 29, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • security_sensitive_path:lib/checkout_sdk/payments/pan_processed_type.rb

Operational gates

  • ✅ jira_ticket (INT-1702)
  • ✅ independent_review

Files analysed: 19


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
security_sensitive_path — lib/checkout_sdk/payments/pan_processed_type.rb classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

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 a47e302 into master Sep 29, 2026
5 checks passed
@david-ruiz-cko
david-ruiz-cko deleted the feature/INT-1702 branch September 29, 2026 15:28
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