Skip to content

feat: Add external_id to Customer and Subscription index - #542

Open
vankiru wants to merge 2 commits into
mainfrom
feat/customers-subscriptions-search-external-id
Open

feat: Add external_id to Customer and Subscription index#542
vankiru wants to merge 2 commits into
mainfrom
feat/customers-subscriptions-search-external-id

Conversation

@vankiru

@vankiru vankiru commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds docs for the external_id filter to customer and subscription index. This is the part of:

@sarkissianraffi sarkissianraffi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified against the API code: the customers index permits external_id as a filter, and the subscriptions side is served by SubscriptionsQuery's with_external_id. Both params documented here are real behavior the spec was missing. Bundle in sync, lint gate passes.

One coordination note inline about overlap with #557. Approving.

Comment thread src/resources/subscriptions.yaml Outdated
parameters:
- $ref: '../parameters/page.yaml'
- $ref: '../parameters/per_page.yaml'
- name: external_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#557 (opened later) adds this same parameter to the same endpoint, plus the overridden filter. Whichever PR merges second needs a rebase to avoid a duplicate entry. Simplest path: land this one, then #557 rebases to keep only overridden here.

@sarkissianraffi

Copy link
Copy Markdown
Contributor

This branch went CONFLICTING after the last merge round, so here is a conflict-free rebase, ready to take: feat/customers-search-external-id-rebased, one commit on top of current main.

What the resolution keeps and drops, verified param-by-param against the bundled spec:

  • /subscriptions external_id: dropped from this PR. docs(subscriptions): document overridden and external_id filters on GET /subscriptions #557 landed it first with the same semantics, plus overridden. Keeping both would have produced a duplicate parameter entry.
  • /customers external_id: dropped too. Main gained it meanwhile.
  • /customers/{external_customer_id}/subscriptions external_id: kept — this is the surviving contribution of this PR. Main does not have it, and the endpoint is served by the same SubscriptionIndex concern that permits the filter, so it is real behavior worth documenting.

Bundle regenerated and in sync. Gates pass at main's exact baseline (0 errors, 23 warnings). Verified every one of the three endpoints now carries external_id exactly once.

Reset this branch to feat/customers-search-external-id-rebased (or cherry-pick its commit) and my approval stands. No regression: everything this PR set out to document is on main one way or another once this lands.

…the /customers and /subscriptions halves that landed on main and in #557
@sarkissianraffi

Copy link
Copy Markdown
Contributor

Update: resolved the conflict directly on this branch instead, via a merge commit from main (your commits untouched, no rewrite). The interim branch from my previous comment is deleted.

Resolution, verified param-by-param on the rebuilt bundle: the /subscriptions and /customers halves were dropped because main carries both meanwhile (#557 and main respectively); the external_id filter on GET /customers/{external_customer_id}/subscriptions is kept and is now this PR's net diff, 14 lines with the bundle. Each of the three endpoints carries the param exactly once. Gates pass at main's baseline: 0 errors, 23 warnings.

GitHub now reports the PR mergeable. My approval stands.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants