Skip to content

feat: model the onboarding bank, articles and shareholder documents - #361

Merged
armando-rodriguez-cko merged 1 commit into
masterfrom
feat/INT-1691-onboarding-document-types
Aug 18, 2026
Merged

feat: model the onboarding bank, articles and shareholder documents#361
armando-rodriguez-cko merged 1 commit into
masterfrom
feat/INT-1691-onboarding-document-types

Conversation

@armando-rodriguez-cko

Copy link
Copy Markdown
Contributor

What

Models the three onboarding documents that carry a document type, giving each its own class plus type constants: bank_verification, articles_of_association and shareholder_structure.

Reported internally, same merchant as the payment instrument document type: bank_statement was missing for documents.bank_verification.type on entity onboarding. This is a different enum in a different place from the payment instrument one.

Verified

In the spec: documents.bank_verification.type is a single-value enum accepting only bank_statement, and it appears on the EEA, GB and US variants, company and sole trader alike, on schema 2.0 and 3.0. Not EEA/GB only, as originally reported.

In the code, this is a PHP-only gap: .NET, Java, Python, Ruby and Go all already expose BankVerificationType.

Why it read as "missing"

The field was typed as the generic Document, whose docblock says @var string value of DocumentType. That is the identity document enum (passport, national identity card, driving license). So the docblock itself pointed the caller at the wrong enum. The value was never rejected by the API, it just could not be found where the SDK said to look.

articles_of_association and shareholder_structure had exactly the same problem and are required on the company full variants, so they were included: the merchant hits them next.

Approach

Follows the CompanyVerification / CompanyVerificationType precedent already in lib/Checkout/Accounts/.

Tests

The existing round-trip test now builds the three documents through the new classes, so it proves the retyped fields still serialize to the same JSON. A new test asserts none of the values are in the identity DocumentType — that is what stops them being merged into it the next time someone reports one as missing.

898 tests green, lint and code sniffer clean.

Not breaking

Additive. The three fields were untyped Document holders and PHP does not enforce the docblock, so existing code that set ->type to a literal keeps working unchanged.

Refs INT-1691.

…ocuments

Second report, internal, same merchant as InstrumentDocumentType: bank_statement was
missing for documents.bank_verification.type on entity onboarding. This is a different
enum from the payment instrument one, in a different place.

Verified in the spec: documents.bank_verification.type is a single-value enum accepting
only bank_statement, and it appears on the EEA, GB and US variants, company and sole
trader alike, on schema 2.0 and 3.0. Not EEA/GB only, as reported.

Verified in the code that this is a PHP-only gap: net, java, python, ruby and go all
already expose BankVerificationType. Here the field was typed as the generic Document,
whose docblock reads "value of DocumentType", pointing the caller at the identity enum.
The docblock itself sent people to the wrong enum, which is why the value read as
missing rather than merely unconstrained.

articles_of_association and shareholder_structure had the same problem and are required
on the company full variants, so the merchant would hit them next. All three now have
their own class plus type constants, following the CompanyVerification and
CompanyVerificationType precedent already in this directory.

The existing round-trip test now builds those three through the new classes, so it
proves the retyped fields still serialize to the same JSON. A new test asserts none of
the values are in the identity DocumentType, which is what stops them being merged into
it the next time someone reports one as missing.

898 tests green, lint and code sniffer clean.

Refs INT-1691.
@agent-wall-e

agent-wall-e Bot commented Aug 18, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

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

Classification reasons

  • no_low_class_matched
  • prod_source_modified

Operational gates

  • ✅ jira_ticket (INT-1691)
  • ✅ independent_review

Files analysed: 9


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Aug 18, 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
no_low_class_matched informational §2.2 (fall-through) None of the deterministic Low classes (§2.2.3, §2.2.4, §2.2.7, docs-only) applied; classifier fell through to LLM evaluation.
prod_source_modified informational §2.1 M7 (informational) At least one file is non-doc, non-test, non-IaC — i.e. application source code was modified.

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 Aug 18, 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 three typed document classes (BankVerification, ArticlesOfAssociation, ShareholderStructure) with their own type-constant classes to replace the mistyped generic Document fields in OnboardSubEntityDocuments; the change is additive, serialization-compatible, and backed by updated round-trip and separation tests.

What I checked

  • The new classes (BankVerification, ArticlesOfAssociation, ShareholderStructure) each expose exactly type and front, matching the shape of the generic Document fields they replace, so existing serialized output is unchanged.
  • The round-trip test now constructs all three documents via the new classes and verifies the full JSON payload, confirming no regression in field names or values.
  • The new OnboardingDocumentTypeTest guards against future conflation with Common\DocumentType by asserting the onboarding values are absent from the identity enum — a sensible regression anchor.
  • ArticlesOfAssociationType includes both memorandum_of_association and articles_of_association, consistent with the spec description and the class docblock.
  • BankVerificationType deliberately exposes a single value (bank_statement), with a clear docblock explaining why it is separate from both DocumentType and InstrumentDocumentType — no concern there.
  • No callers of the old Document-typed fields are visible in this diff; as stated the change is additive and PHP does not enforce docblock types, so existing literal assignments remain valid.

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

@sonarqubecloud

Copy link
Copy Markdown

@armando-rodriguez-cko
armando-rodriguez-cko requested a review from a team August 18, 2026 14:49
@armando-rodriguez-cko
armando-rodriguez-cko merged commit 65f8f8c into master Aug 18, 2026
4 checks passed
@armando-rodriguez-cko
armando-rodriguez-cko deleted the feat/INT-1691-onboarding-document-types branch August 18, 2026 15:24
armando-rodriguez-cko added a commit that referenced this pull request Aug 19, 2026
- Add InstrumentDocumentType with bank_statement for bank account payment instrument documents (#360)
- Model the onboarding bank_verification, articles_of_association and shareholder_structure documents as their own classes with their own type constants, instead of the generic Document whose docblock pointed at the identity DocumentType (#361)
- Revert build-master.yml to push-only with its original release guard, and build-pull-request.yml to its pre-#362 content; from #362 only the archive download URLs and the checksum verification remain, which is what fixed the 404
armando-rodriguez-cko added a commit that referenced this pull request Aug 19, 2026
- Add InstrumentDocumentType with bank_statement for bank account payment instrument documents (#360)
- Model the onboarding bank_verification, articles_of_association and shareholder_structure documents as their own classes with their own type constants, instead of the generic Document whose docblock pointed at the identity DocumentType (#361)
armando-rodriguez-cko added a commit that referenced this pull request Aug 19, 2026
- Add InstrumentDocumentType with bank_statement for bank account payment instrument documents (#360)
- Model the onboarding bank_verification, articles_of_association and shareholder_structure documents as their own classes with their own type constants, instead of the generic Document whose docblock pointed at the identity DocumentType (#361)
armando-rodriguez-cko added a commit that referenced this pull request Aug 19, 2026
- Add InstrumentDocumentType with bank_statement for bank account payment instrument documents (#360)
- Model the onboarding bank_verification, articles_of_association and shareholder_structure documents as their own classes with their own type constants, instead of the generic Document whose docblock pointed at the identity DocumentType (#361)
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