diff --git a/AGENTS.md b/AGENTS.md index 1bf2a17..d6ffb57 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -127,7 +127,7 @@ If direct-vs-import was not given to you in the task, stop and ask the operator. When multiple accounts or profiles are active, commands write a hint to stderr so you know which account is being targeted: ``` -[account: 9901287 | profile: admin | env: test] +[account: 9900000 | profile: admin | env: test] ``` The environment is included in the hint only when credentials span multiple environments or the active environment is non-default. Single-environment users (e.g. customers on prod only) won't see it. @@ -214,6 +214,17 @@ band transcription create --wait # blocks until t All `--wait` commands support `--timeout `. Exit code 5 on timeout. +**`--timeout` bounds when polling stops, not wall-clock duration.** The +underlying poll loop (`internal/cmdutil/poll.go`) checks the deadline +*between* poll attempts, not during one, and always sleeps the full +`--interval` rather than whatever time remains before the deadline. So a +call can run past `--timeout` by up to one poll interval plus one in-flight +request — and if that late attempt happens to succeed, the command exits +**0**, not 5, seconds after the timeout you asked for. Concretely: a 5s +poll interval with `--timeout 1` can still succeed at t≈5s. Do not treat +`--timeout` as a precise deadline; treat it as a lower bound on how long the +CLI will keep trying. + ## Output **Always use `--plain` when parsing CLI output.** Default JSON reflects Bandwidth's API structure with deep nesting. `--plain` flattens it: @@ -446,6 +457,18 @@ only be created on ACTIVE realms; retry after 'band sip realm get ' report ACTIVE"` and exits **1**. (In the narrow case where the API itself rejects the create in a race, error 23022 surfaces instead — see the errors table below.) +**10DLC (Registration Center, direct):** +``` +account + auth (Registration Center feature + Campaign Management role) + └─→ customer-profile create (one profile per brand — 1:1, not reusable) + └─→ tendlc brand create --wait (must reach VERIFIED or VETTED_VERIFIED) + └─→ tendlc campaign create (not yet in the CLI) +``` + +See [10DLC Brands](#10dlc-brands) for `brand create`'s flag matrix, `--wait` +semantics, and the customer-profile pre-flight; [10DLC Vettings](#10dlc-vettings) +for the optional vetting step some brand/campaign combinations require. + --- ### Diagnose: What state am I in? @@ -828,7 +851,7 @@ These commands query the Registration Center API for 10DLC campaign and phone nu **Important:** These commands are for **import customers** — accounts that register campaigns through TCR and import them to Bandwidth. They require the **Campaign Management role** on your API credential and the **Registration Center feature** on your account. -**Direct customers** (accounts that register campaigns directly through Bandwidth) are not yet supported by these commands. Direct registration through the CLI is planned for mid-2026. In the meantime, direct customers should use the Bandwidth App or the existing Campaign Management API. +**Direct customers** (accounts that register campaigns directly through Bandwidth) are not supported by the commands on this page — those remain import-only. Direct customers register brands and order vettings through the CLI via [`band tendlc brand`](#10dlc-brands) and [`band tendlc vetting`](#10dlc-vettings). Campaign registration for direct customers is not yet in the CLI; in the meantime, use the Bandwidth App or the existing Campaign Management API for that step. A 403 from `band tendlc` can mean: credential lacks the Campaign Management role, account doesn't have Registration Center, account is a direct customer, or messaging isn't enabled. The CLI parses the API response and gives a specific message for each case. @@ -836,7 +859,7 @@ A 403 from `band tendlc` can mean: credential lacks the Campaign Management role ```bash band tendlc number +19195551234 --plain -# → { "phoneNumber": "+19195551234", "campaignId": "CA3XKE1", "status": "SUCCESS", "brandId": "B1DER2J", ... } +# → { "phoneNumber": "+19195551234", "campaignId": "CA3XKE1", "status": "SUCCESS", "brandId": "BEXMPL5", ... } ``` Status values: `SUCCESS` (ready to send), `PROCESSING` (pending), `FAILURE` (registration failed). @@ -845,7 +868,7 @@ Status values: `SUCCESS` (ready to send), `PROCESSING` (pending), `FAILURE` (reg ```bash band tendlc campaigns --plain -# → [{ "campaignId": "CA3XKE1", "status": "SUCCESS", "brandId": "B1DER2J", ... }, ...] +# → [{ "campaignId": "CA3XKE1", "status": "SUCCESS", "brandId": "BEXMPL5", ... }, ...] ``` ### List all registered numbers (with filters) @@ -885,8 +908,9 @@ band tnoption assign +19195551234 --campaign-id CA3XKE1 --wait A customer profile is required to register a 10DLC brand, and **a profile backs exactly one brand** — reusing a profile ID on a second brand fails with `cannot be assigned to another brand`. Create a fresh profile per brand. The -prerequisite chain is **customer profile → brand → campaign**; brand and -campaign registration still happen in the Bandwidth App, not the CLI. +prerequisite chain is **customer profile → brand → campaign**: brand +registration now happens in the CLI via [`band tendlc brand create`](#10dlc-brands); +campaign registration still happens in the Bandwidth App. Requires the **Customer Profiles Access role** — check with `band auth status --plain`. @@ -894,10 +918,10 @@ Requires the **Customer Profiles Access role** — check with `band auth status ```bash band customer-profile create --name "Acme Corp" --plain -# → {"accountId":"9901287","addressId":null,"contact":null,"createdDate":"...","id":"622t7KB9oZkl9kQob0b8el","modifiedDate":"...","name":"Acme Corp","softDeleted":false,"totalCampaigns":0,"version":0,"website":null} +# → {"accountId":"9900000","addressId":null,"contact":null,"createdDate":"...","id":"ExampleProfileId000002","modifiedDate":"...","name":"Acme Corp","softDeleted":false,"totalCampaigns":0,"version":0,"website":null} band customer-profile list --all --plain # walks every page; cannot combine with --offset -band customer-profile get 622t7KB9oZkl9kQob0b8el --plain +band customer-profile get ExampleProfileId000002 --plain ``` Keys come back alphabetical because the payload is a Go map — don't expect a @@ -932,8 +956,8 @@ concurrent edit between the read and the write is caught by the API's version check and exits **4** — retry the command. ```bash -band customer-profile update 622t7KB9oZkl9kQob0b8el --name "New Name" --plain -band customer-profile update 622t7KB9oZkl9kQob0b8el --website "" --plain # clears the website +band customer-profile update ExampleProfileId000002 --name "New Name" --plain +band customer-profile update ExampleProfileId000002 --website "" --plain # clears the website ``` ### Delete is a soft delete @@ -943,9 +967,9 @@ humans share one contract. The record leaves listings but stays retrievable by ID with `softDeleted: true`, and `restore` brings it back — no confirm needed. ```bash -band customer-profile delete 622t7KB9oZkl9kQob0b8el --confirm --plain -# → {"deleted":true,"id":"622t7KB9oZkl9kQob0b8el","restore":"band customer-profile restore 622t7KB9oZkl9kQob0b8el"} -band customer-profile restore 622t7KB9oZkl9kQob0b8el --plain +band customer-profile delete ExampleProfileId000002 --confirm --plain +# → {"deleted":true,"id":"ExampleProfileId000002","restore":"band customer-profile restore ExampleProfileId000002"} +band customer-profile restore ExampleProfileId000002 --plain ``` Note these are two different fields on two different resources, not a typo of @@ -963,10 +987,519 @@ first — the profile snapshot lives under `data`, and `version`, `operation`, Observed `metadata.operation` values: `CREATED`, `UPDATED`, `DELETED`. ```bash -band customer-profile history list 622t7KB9oZkl9kQob0b8el --plain -band customer-profile history get 622t7KB9oZkl9kQob0b8el 1 --plain +band customer-profile history list ExampleProfileId000002 --plain +band customer-profile history get ExampleProfileId000002 1 --plain +``` + +## 10DLC Brands + +`band tendlc brand` registers and manages 10DLC brands for **direct** customers +(accounts that register with TCR through Bandwidth directly, not through +import). A brand needs a customer profile first — see +[Customer Profiles](#customer-profiles) — and a profile backs exactly one +brand. Requires the **Registration Center** feature and the **Campaign +Management** role. + +### Eligibility + +Check before doing anything else: + +```bash +band tendlc status --plain +``` + +This is a probe, not a live brand/vetting command, and it is deliberately +gentle: every 403 it can classify — role missing, Registration Center not +enabled, campaign management not enabled, or an unrecognized 403 — is a +**successful answer to the question "can I do this?"**, so it exits **0** with +`{"access":"unavailable", ...}`. See [10DLC capability](#10dlc-capability-tri-state-not-boolean) +for the full `reason` table. Only `probe_failed` (rate limit, 5xx, transport +error) is worth retrying. + +That gentle handling is specific to `status`. Once you move on to an actual +`brand`/`vetting` command, a 403 there is **not** a probe answer — it is the +command failing — and it maps to exit **4** via the same +`FeatureLimitError`/`roleGateError` path used everywhere else in this CLI. +Escalate to the account manager; do not retry. + +### The two IDs + +Every brand has two identifiers, and they are not interchangeable in what +they guarantee: + +- **`bandwidthId`** exists from the moment of the 202 that accepts the create. +- **`brandId`** is assigned by TCR once registration completes, and is `null` + until then. A brand that never registers never gets one. + +Every `brand`/`vetting` command that takes an ID accepts either. This matters +for anything that keys off `brandId`: a script or agent that reads `brandId` +right after `create` and treats a missing/`null` value as an error will +misfire on every brand still mid-registration — `bandwidthId` is the only ID +guaranteed to exist immediately. + +```bash +band tendlc brand get BEXMPL1 --plain # TCR brandId +band tendlc brand get WEXAMPLE01 --plain # bandwidthId — same brand +``` + +Both return the same 46-key object, including `bandwidthId`, `brandId`, +`brandIdentityStatus`, `brandRelationship`, `universalEin`, `imported`, and +`vertical` (a live value observed was `PROFESSIONAL`, which is absent from the +published enum). + +### `create` + +```bash +band tendlc brand create --customer-profile-id CP123 --brand-type PRIVATE_PROFIT \ + --display-name "Acme Corp" --company-name "Acme Corporation" \ + --street "123 Main St" --city Raleigh --state NC --postal-code 27601 \ + --country-code-a3 USA --phone +18885551234 --email ops@acme.com \ + --vertical RETAIL --ein 123456789 --ein-issuing-country-code-a3 USA --wait +``` + +There is **no `--country` flag** — the API derives `country` server-side from +`--country-code-a3`, so passing one would just be dead input. Likewise there +is no separate country flag for the EIN; `--ein-issuing-country-code-a3` is +the only knob. + +**Every brand type** requires these 9 fields plus `--brand-type` itself: +`--customer-profile-id`, `--display-name`, `--street`, `--city`, `--state`, +`--postal-code`, `--country-code-a3`, `--phone`, `--email`. Beyond that, the +required set is per-`brandType`. This matrix is sourced from +`internal/tendlc/brandoptions.go`, which is the measured truth — the API's +own schema gets required-ness wrong in both directions, so this is derived +from the API's actual 400 responses, not the published spec: + +| `--brand-type` | Additional required flags | +|---|---| +| `PRIVATE_PROFIT` | `--company-name`, `--vertical`, `--ein`, `--ein-issuing-country-code-a3` | +| `NON_PROFIT` | same four as `PRIVATE_PROFIT` | +| `GOVERNMENT` | same four as `PRIVATE_PROFIT` | +| `PUBLIC_PROFIT` | the same four, **plus** `--stock-symbol`, `--stock-exchange`, `--website`, `--business-contact-email` | +| `SOLE_PROPRIETOR` | none enforced by the CLI beyond the 9 common fields (see note) | + +**`SOLE_PROPRIETOR` is deliberately under-validated.** Its field rules sit +behind an account-level gate no available test account could get past, so +they were never observable. The CLI does not invent rules for it — doing so +risks rejecting a request the API would actually accept. Expect the API's own +400 to enforce anything beyond the 9 common fields for this type. The +`--first-name`, `--last-name`, `--mobile-phone`, and `--ip-address` flags +exist and are almost certainly what a sole-proprietor registration needs, but +they are optional at the CLI layer. + +Validation aggregates every violation into one error, the way the API +reports every violation in one 400 — no request is made: + +``` +$ band tendlc brand create --plain +Error: missing required flags: --brand-type, --city, --country-code-a3, --customer-profile-id, --display-name, --email, --phone, --postal-code, --state, --street + +$ band tendlc brand create --brand-type PUBLIC_PROFIT --plain +Error: missing required flags: --business-contact-email, --stock-exchange, --stock-symbol, --website +``` + +**The `customerProfileId` trap.** Measured against production: `POST /brands` +does **not** reject an invalid or typo'd `customerProfileId`. It silently +discards it and returns **202**, creating an orphan brand with no profile +association — a brand that can never verify. To prevent this, `brand create` +reads the customer profile named by `--customer-profile-id` before it submits +anything. Only a definitive 404 stops the create: + +``` +$ band tendlc brand create --customer-profile-id NOT_A_REAL_PROFILE_ID --brand-type PRIVATE_PROFIT ... --plain +Error: customer profile "NOT_A_REAL_PROFILE_ID" not found — run 'band customer-profile list' to see valid profile IDs (API error 404: {"errors":[{"type":"not found","description":"Customer profile not found, id: NOT_A_REAL_PROFILE_ID","source":{"POINTER":"/id"}}],"links":[]}) +$ echo $? +3 +``` + +Brand count was measured 10 before and 10 after this refusal — nothing was +created. If the pre-flight itself **cannot run** — most commonly because the +caller has Campaign Management but not the separate **Customer Profiles +Access** role — the CLI does not block the create on a check it has no +permission to perform. It warns on stderr and proceeds: + +``` +warning: could not verify customer profile "CP123" before creating the brand (...); proceeding anyway +``` + +An agent that sees this warning should not assume the profile association +took — verify it afterward by checking `accounts[0].customerProfileId` on +the resulting brand (`band tendlc brand get --plain`). + +**`--wait` semantics.** `brandIdentityStatus` reads `UNVERIFIED` for the +entire registration window; `REGISTERING`, the enum's documented in-progress +value, was never observed on the read path in live testing. That means +`UNVERIFIED` cannot distinguish "TCR hasn't answered yet" from "TCR rejected +it" — so the CLI keeps polling on `UNVERIFIED` rather than treating it as a +failure, and only surfaces it at timeout, with the last-seen status attached. +Two brands submitted with byte-identical payloads measured very differently: + +- `WEXAMPLE02`: read `UNVERIFIED` at t=3s, flipped to `VERIFIED` at t=46s +- `WEXAMPLE03`: read `UNVERIFIED` at t=3s, still `UNVERIFIED` at t=275s, no TCR response in its history at all + +**A timeout (exit 5) is not a failure.** Tell the caller plainly: re-check +with `band tendlc brand get ` (or `brand history ` for the raw TCR +timeline) rather than treating the timeout as a rejected brand. The only +state that *is* a business failure is `ERROR`, which exits **4** — everything +else not in `{VERIFIED, VETTED_VERIFIED, SELF_DECLARED, ERROR}` is treated as +still-pending. + +| Outcome | Exit | What's on stdout | +|---|---|---| +| Reached `VERIFIED` / `VETTED_VERIFIED` / `SELF_DECLARED` | 0 | The full brand object (46 keys), including `bandwidthId` | +| Reached `ERROR` | 4 | The full brand object, plus a remediation message on stderr pointing at `brand refresh` | +| `--wait` exceeded `--timeout`, still pending | 5 | The synthetic receipt (below), with `lastSeenStatus` | +| Transport/decode failure mid-poll | Whatever `ExitCodeForError` maps the underlying error to (not necessarily 5 — only an actual deadline-exceeded times out as 5) | The synthetic receipt (below) | +| No `--wait` | 0 | The synthetic receipt, immediately after the 202 | + +**The receipt guarantee.** On every path that reaches a 202 accept, stdout +carries `bandwidthId` somewhere in valid JSON — that ID is the one thing that +cannot be recovered any other way if the command exits without printing it. + +**This guarantee covers every path the command itself takes — it does not +cover an interrupt.** `cmd/root.go`'s `Execute()` runs with no context, and +the CLI does not install a `SIGINT` handler anywhere (no `signal.Notify` / +`signal.NotifyContext` in the codebase), so the cancellation branch in +`awaitTerminal` that exists specifically to emit this receipt can never +actually fire from a real Ctrl-C. Press Ctrl-C during `--wait` after the 202 +has landed and the process dies immediately with no `bandwidthId` on stdout. +If that happens, recover with `band tendlc brand list --customer-profile-id-contains +` to find the brand that was accepted. This applies CLI-wide, not just to +`tendlc` — it is a pre-existing, repo-wide gap, tracked separately from this +PR. + +Without `--wait`, or on a timeout/transport failure with `--wait`, that's the +literal synthetic receipt: `{"bandwidthId": "...", "status": "accepted", +"resume": "band tendlc brand get ", "brandId": "..." (if already known)}` +— plus, on a `--wait` timeout/error, `"note"` (pointing at `get`/`history` +for a brand parked at `UNVERIFIED`) and `"lastSeenStatus"` (if a status was +ever successfully read). On success or business failure (`ERROR`), stdout +instead carries the **real, full brand object** returned by the API — not +the synthetic receipt — but `bandwidthId` is still one of its keys either +way. **This is a reconstruction, not a captured run** — forcing a real +timeout would mean paying for another brand creation just to let it sit +unverified, so no live timeout receipt was captured. The shape below is +assembled field-by-field from what `buildAcceptedReceipt` and `awaitTerminal` +actually emit, not copied from a real invocation: + +```json +{ + "bandwidthId": "WEXAMPLE03", + "status": "accepted", + "resume": "band tendlc brand get WEXAMPLE03", + "note": "if this timed out at UNVERIFIED, the brand may still be registering with TCR rather than having failed. Check 'band tendlc brand get WEXAMPLE03' for its current status, or 'band tendlc brand history WEXAMPLE03' for the full history.", + "lastSeenStatus": "UNVERIFIED" +} +``` + +### `--confirm`-gated writes, and what each costs + +`--confirm` is a flag, never a prompt — agents and humans share one +contract. Missing it is exit **6** (`FlagError`) with **zero write +requests** — but `brand update` is a partial exception: see the callout +below the table. + +**`brand create` is billable but deliberately has no `--confirm`.** Measured +against production: the brand registration fee is billed about six +seconds after the 202, before verification resolves, and is charged even for +a brand that never ends up verifying — see the fee note in +[10DLC Brands](#10dlc-brands). Every other billable or destructive write here +gates behind `--confirm`; `create` does not, because it requires roughly a +dozen explicit required flags and cannot be triggered by accident the way a +bare ` --confirm` command can. This is a deliberate product call, not an +oversight — do not read the omission as one. + +| Command | Cost / consequence | +|---|---| +| `brand delete --confirm` | Permanent. Requires every campaign on the brand to be deactivated first. See the cascade correction below. | +| `brand reverify --confirm` | $4 fee. Resets `brandIdentityStatus` toward re-registration (documented as `REGISTERING`; in practice this reads back as `UNVERIFIED`, per the `--wait` note above). | +| `brand update --confirm ...` | Only required when an identity-affecting field changes: `--company-name`, `--brand-type`, `--ein`, or `--ein-issuing-country-code-a3` (possible $4 fee + reset to re-registration, and rejected outright if the brand has an active campaign or an active Standard/Enhanced/Political vetting), `--mobile-phone` (sets identity status to `UNVERIFIED`), or `--business-contact-email` on a `PUBLIC_PROFIT` brand (revokes Auth+ compliance — regaining it needs a new `AUTHPLUS` vetting and another 2FA email round-trip). | +| `vetting request --confirm` | Billable order placed with an external vetting provider (see [10DLC Vettings](#10dlc-vettings)). | + +**`brand update`'s refusal is not zero-request — it costs one GET.** Unlike +every other confirm gate here, `update` performs a real `GetBrand` call +*before* it can even decide whether `--confirm` is required: the +business-contact-email/PUBLIC_PROFIT condition in +`IdentityFieldsChanged` needs the brand's current type, which is only known +once that GET returns (see the source comment on this in +`cmd/tendlc/brand_update.go` — it deliberately warns against moving the +check earlier to "fix" this). So an unconfirmed `brand update` on an +identity-affecting field still makes one live GET against your account's +shared rate-limit budget; it is only the **write** (the PUT) that never +happens without `--confirm`. Do not treat it as a free dry-run. + +Example refusals — `delete` and `reverify` make zero requests; `update` +makes the one GET described above, but never the write: + +``` +$ band tendlc brand delete BEXMPL1 --plain +Error: this permanently deletes brand BEXMPL1. It cannot be undone, it deletes the brand in TCR for direct accounts, and it requires every campaign on the brand to be deactivated first. It does NOT delete the associated customer profile (measured against production — the documented cascade does not happen); remove that separately with 'band customer-profile delete ' if you no longer need it. Pass --confirm to proceed. + +$ band tendlc brand reverify BEXMPL1 --plain +Error: reverifying brand BEXMPL1 incurs a $4 fee and resets brandIdentityStatus toward re-registration; it reads back as UNVERIFIED until TCR responds. Pass --confirm to proceed. + +$ band tendlc brand update BEXMPL1 --company-name "New Name" --plain +Error: changing company-name on brand BEXMPL1 resubmits it for identity verification: this may incur a $4 fee and resets brandIdentityStatus toward re-registration (it reads back as UNVERIFIED until TCR responds). If the brand has an active campaign or an active Standard/Enhanced/Political vetting, the API will reject the change outright. Pass --confirm to proceed. +``` + +`brand resend-2fa` and `brand refresh` take no `--confirm` — neither is +destructive nor billable. + +**The delete cascade correction.** The endpoint's own docs say deleting a +brand cascades to delete its backing customer profile. Measured against +production: **it does not.** Two test brands were deleted and both backing +profiles remained retrievable afterward with `softDeleted: false`. Delete the +profile separately with `band customer-profile delete ` if you don't need +it — otherwise it's an orphan that can never back another brand (a profile +is 1:1 with a brand for life). Also note: deletion takes roughly **40 +seconds** to be reflected in `brand list`/`brand get`; `delete --wait` polls +until the brand is actually gone (the one poll in this whole command set +where a 404 on the follow-up read means success, not "not ready yet"). + +**The receipt's `deleted` field is honest, not optimistic.** The DELETE call +only ACCEPTS the request — it does not confirm the brand is actually gone. +So `deleted` starts `false` and stays `false` until a 404 on the follow-up +read proves it. Without `--wait`, that's every invocation: + +``` +$ band tendlc brand delete WEXAMPLE02 --confirm --plain +{ + "deleted": false, + "id": "WEXAMPLE02", + "note": "delete accepted but not yet confirmed: production takes roughly 40s to actually remove the brand, so it may still appear in 'brand list' or 'brand get' until then. Confirm with 'band tendlc brand get WEXAMPLE02' — a 404 means it is gone.", + "status": "accepted" +} +$ echo $? +0 +``` + +With `--wait`, `deleted` flips to `true` (and `note` is dropped) only once +the follow-up read actually 404s: + +``` +$ band tendlc brand delete WEXAMPLE02 --confirm --wait --plain +{ + "deleted": true, + "id": "WEXAMPLE02", + "status": "accepted" +} +$ echo $? +0 +``` + +A `--wait` timeout (exit **5**) prints the identical unconfirmed receipt as +the no-`--wait` case above — `deleted: false`, the `note`, exit 5 — never +`deleted: true`. A receipt that claims completion while the exit code says +"gave up waiting" would be worse than no receipt at all: an agent branching +on `deleted` must not be able to see `true` paired with a non-zero exit. + +### `update` is read-modify-write, with no version field + +The API replaces the whole record on update, so the CLI reads the brand +first and re-sends it with your changes applied — fields you don't pass are +preserved. Unlike customer profiles, **brands have no optimistic-locking +token at all**: there is no version check on the PUT. A concurrent edit that +lands between the CLI's read and its write is silently lost — whichever +write reaches the API last wins, with no conflict error to catch it. + +**`update` prints an acceptance receipt, not the updated brand.** Measured +against production: `PUT /brands/{brandId}` returns a bare `{bandwidthId, +brandId}` acceptance, not the resource, and the change itself takes roughly +**5 minutes** to be reflected — a `website` change submitted at ~14:55 was +still absent from the brand at t+48s and only showed up in `modifiedDate` +and the activity log at 15:00:37. So `brand update` prints the same +`{bandwidthId, brandId, status: "accepted", resume}` receipt shape +`create`/`refresh` use, plus a `note` naming the latency: + +```bash +band tendlc brand update BEXMPL1 --website "https://acme.example" --plain ``` +```json +{ + "bandwidthId": "WEXAMPLE01", + "brandId": "BEXMPL1", + "status": "accepted", + "resume": "band tendlc brand get WEXAMPLE01", + "note": "this is an acceptance, not the updated brand: production takes about 5 minutes to apply the change (modifiedDate and the history log lag behind), so an immediate 'brand get' may still show the pre-update value. Check 'band tendlc brand get WEXAMPLE01' again shortly, or 'band tendlc brand history WEXAMPLE01' for confirmation." +} +``` + +This is also why `update` has no `--wait`: this measurement confirms that +decision rather than merely motivating it — a poll here would hit a brand +still holding its pre-update state and report success before the change +actually took effect. + +### `list` vs `get`: projection, not nullability + +`brand list` returns a **12-key summary projection**; `brand get` returns +**46 keys**. A field missing from `list` output is not null on the brand — +it's simply outside the listing projection. Use `get` for the full resource. + +``` +$ band tendlc brand list --plain --limit 2 +showing 2 of 10 brands; pass --all to fetch every page # <- stderr +[ + { + "accounts": [{"accountId": "9900000", "customerProfileId": "ExampleProfileId000001"}], + "authenticationStatus": "ACTIVE", + "bandwidthId": "WEXAMPLE04", + "brandId": "BEXMPL4", + "brandIdentityStatus": "VERIFIED", + "brandType": "PUBLIC_PROFIT", + "businessContactEmail": "ops@example.com", + "companyName": "Bandwidth", + "createdDate": "2026-05-28T21:00:16.048Z", + "displayName": "Another Auth+ Test, Bandwidth", + "modifiedDate": "2026-05-28T21:00:16.048Z", + "website": "bandwidth.com" + } +] +``` + +There is deliberately **no `--bandwidth-id` filter**. Measured against +production: the API accepts a `bandwidthId[eq]` filter and silently ignores +it, returning every brand rather than filtering — so the CLI doesn't expose +a flag that would lie about filtering. Use `brand get ` to +fetch one directly instead. + +**None of `brand list`'s filters use `eq` on the wire, even the ones that +look like exact-match flags.** Measured against a 10-brand test account: +`eq` is accepted and silently dropped on every field tried — +`brandId[eq]`, `customerProfileId[eq]`, `brandIdentityStatus[eq]`, and +`brandType[eq]` all returned all 10 brands, the same failure documented +above for `bandwidthId`. `contains` is the only operator that actually +filters, so: + +- `--brand-id-contains` and `--customer-profile-id-contains` are named for + what they do — a substring match. `--brand-id-contains BEXMPL1` also + matches `BEXMPL12`; use `brand get ` when you need exactly one brand. +- `--identity-status` and `--brand-type` filter on closed enums, where a bare + substring match has a sharper trap: `brandIdentityStatus[contains]=VERIFIED` + matches `VERIFIED`, `VETTED_VERIFIED`, **and** `UNVERIFIED` — the opposite + of what was asked for. The CLI sends `contains` to the server and then + filters the response client-side for an exact match, so these two flags + keep exact-match semantics despite the API having no `eq` that works. + +`brand history ` is a free-text activity log, newest first, with no +version-per-entry the way customer profiles have: + +``` +$ band tendlc brand history BEXMPL1 --plain --limit 2 +showing 2 of 7 history entries; pass --all to fetch every page # <- stderr +[ + {"createdDate": "2026-06-17T19:37:16.927Z", "message": "Successfully updated brand BEXMPL1 for account 9900000"}, + {"createdDate": "2026-06-17T18:10:48.526Z", "message": "BRAND_IDENTITY_STATUS_UPDATE received from TCR with new status UNVERIFIED"} +] +``` + +A real registration timeline, from live testing, newest first: + +``` +{"createdDate": "2026-08-19T13:39:04.542Z", "message": "BRAND_IDENTITY_STATUS_UPDATE received from TCR with new status VERIFIED"} +{"createdDate": "2026-08-19T13:38:24.988Z", "message": "Brand billed for account 9900000 brandId BEXMPL3 sku BRAND-FEE-EXAMPLE"} +{"createdDate": "2026-08-19T13:38:18.725Z", "message": "Successfully created brand BEXMPL3 for account 9900000 and bandwidthId WEXAMPLE02"} +{"createdDate": "2026-08-19T13:38:17.589Z", "message": "Registering brand for account 9900000 bandwidthId WEXAMPLE02"} +``` + +Note the fee lands ~6 seconds after creation — long before verification +resolves either way. **The brand fee is billed at creation, not at +verification**, and is charged even for a brand that never ends up +verifying. Factor that into any retry logic: a create you retry blind after +an ambiguous failure risks a second bill, not just a second brand. + +## 10DLC Vettings + +`band tendlc vetting` orders and records third-party vettings against a +brand. Requires the same Registration Center feature and Campaign Management +role as `brand`. + +**Vettings are brand-scoped, not campaign-scoped.** Every command here takes +a **brand ID** as its first positional, not a vetting ID — there is no +campaign vetting endpoint in either spec. A campaign only exposes a +read-only, derived `vettingStatus`; re-evaluating a campaign directly is a +different, separate future command (`nudge`), not covered here. + +### `list` + +``` +$ band tendlc vetting list BEXMPL2 --plain +[ + { + "bandwidthId": "WEXAMPLE05", + "createdDate": "2026-06-17T17:55:13Z", + "evpId": "AEGIS", + "reasons": [ + "Submitted Address Line 1 cannot be verified against government or business sources.", + "Submitted Postal Code cannot be verified against government or business sources.", + "Submitted company name does not match state business registration records." + ], + "vettedDate": "2026-06-17T17:55:13Z", + "vettingClass": "STANDARD", + "vettingDetails": {}, + "vettingId": "00000000-1111-2222-3333-444444444444", + "vettingScore": 88, + "vettingStatus": "ACTIVE", + "vettingToken": "eyJ...redacted..." + } +] +``` + +Note `vettingStatus: "ACTIVE"` — the live success value, and it appears in +**neither** the brand-vetting enum nor the campaign `vettingStatus` enum. +Unknown/undocumented statuses are treated as still-pending everywhere in +this command set: only `ACTIVE` classifies as success and only `FAILED` / +`EXPIRED` classify as business failure, so a status this CLI has never seen +keeps polling under `--wait` rather than being reported as either outcome. + +**A naming quirk to handle, not normalize.** `list` returns the ID under +`bandwidthId`; the `POST .../vettings` 202 accept returns it as +`vettingBandwidthId` instead. The CLI preserves whichever key name the API +actually used in its own receipts rather than renaming it to one canonical +field — so when you read a vetting receipt, check for both key names. + +### `request` — billable, `--confirm`-gated + +```bash +band tendlc vetting request BEXMPL1 --evp AEGIS --class STANDARD --confirm --plain +band tendlc vetting request BEXMPL1 --evp AEGIS --class STANDARD --confirm --wait --plain +``` + +`--evp` accepts `AEGIS`, `CV`, `WMC` — a small, stable, fully documented enum. + +`--class` accepts `STANDARD`, `ENHANCED`, `POLITICAL`, `AUTHPLUS`, and +**`RCS`**. `RCS` is absent from the published `enumVettingClass`, but +production accepts it — confirmed by pairing each class with an invalid +`evpId` and observing which produced a class-level error rather than an +evp-level one. Do not drop it to match the spec; production honors it. + +This places a real, billable order with an external vetting provider, so +`--confirm` is required — missing it is exit 6, zero requests: + +``` +$ band tendlc vetting request BEXMPL1 --evp AEGIS --class STANDARD --plain +Error: requesting a STANDARD vetting from AEGIS for brand BEXMPL1 is a billable order placed with an external vetting provider. Pass --confirm to proceed. +``` + +With `--wait`, the receipt is `{: id, "brandId": ..., "status": +"accepted", "check": "band tendlc vetting list "}`, printed +immediately without `--wait` or on a timeout/transport error with it. Unlike +`brand create --wait`, there is no `lastSeenStatus` carried on a vetting +timeout — the vetting poll target doesn't track a last-observed status, so a +timed-out `vetting request --wait` tells you to re-check with `vetting list` +rather than showing you the last status inline. + +### `import` — not billable, no `--confirm` + +```bash +band tendlc vetting import BEXMPL1 V123 --evp AEGIS --plain +band tendlc vetting import BEXMPL1 V123 --evp AEGIS --vetting-token TOK123 --plain +``` + +Recording a vetting that was already performed outside Bandwidth costs +nothing and places no new order, so — unlike `request` — this takes no +`--confirm`. + ## Toll-Free Verification (TFV) These commands manage toll-free number verification via the Athena v2 API. A 403 means the TFV role isn't enabled on the credential — contact your Bandwidth account manager to enable it. @@ -1229,7 +1762,7 @@ Every error in this table exits **4**, regardless of the HTTP status the API use - **No real-time call control.** The CLI can initiate calls and query state, but cannot receive or respond to mid-call callbacks. Dynamic call control requires a separate callback-handling server. - **No message delivery confirmation.** The CLI verifies your setup is correct before sending (app-location link, callback URL, campaign), but it cannot confirm whether a message was actually delivered. Delivery status (`message-delivered`, `message-failed`) arrives via webhooks on your callback server. The CLI's `message get` and `message list` return metadata only — not delivery status. - **No message content retrieval.** Bandwidth does not store message bodies. After sending, the message text is gone forever. `message get` and `message list` return timestamps, direction, and segment counts only. -- **10DLC: read + assign only.** The CLI can list campaigns, check number registration status, diagnose failures (`band tendlc`), and assign numbers to campaigns (`band tnoption assign`). It cannot create campaigns or register brands — those require the Bandwidth App. The CLI checks that a number is on a campaign and blocks sends if it's not. +- **10DLC: brand and vetting registration, campaigns are read + assign only.** The CLI can register and manage brands (`band tendlc brand`) and order/record vettings (`band tendlc vetting`) for direct customers. For campaigns, it can list them, check number registration status, diagnose failures (`band tendlc`), and assign numbers to campaigns (`band tnoption assign`) — but it cannot create campaigns; that still requires the Bandwidth App. The CLI checks that a number is on a campaign and blocks sends if it's not. - **TFV is check-and-submit.** The CLI can check toll-free verification status and submit new requests (`band tfv`), but cannot approve or expedite reviews — those happen on the carrier side. - **Porting is port-IN only.** `band portin` covers the six end-to-end flows that complete via the public API: TF validation, on-net domestic, automated off-net (Level 3), TF Phase 1 (gated), bulk, and lifecycle ops (notes, supp, cancel, history, doc upload). Out of scope: port-out (no public API), manual TF, internal TF, NASC manual override, and international ports — these need ops or the Dashboard. `band portin create` exits 4 if the account doesn't have `TOLL_FREE_AUTOMATION_PHASE_1` for a TF order. `band portin supp` defends against the documented Bandwidth API behavior where a supp returns 200 on PUT but error code 7300 on the next GET (Neustar never received it) — exits 1 with a clear message rather than silently succeeding. - **10DLC, TFV, and short code commands are role-gated.** A 403 can mean the credential lacks the required role (Campaign Management, TFV), the account doesn't have the Registration Center feature, or messaging isn't enabled. The CLI provides a diagnostic message — if it says "access denied," escalate to the Bandwidth account manager rather than retrying. diff --git a/README.md b/README.md index 4a46b8d..ae8ff29 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ That's it. The CLI validates your credentials, figures out which accounts you ca ```sh band auth status # see which account is active and what else is available band auth switch # pick a different account interactively -band auth switch 9901287 # or jump straight to one by ID +band auth switch 9900000 # or jump straight to one by ID ``` You can also pass `--account-id` to any command to override the active account for a single call. @@ -267,7 +267,7 @@ band customer-profile list --plain band customer-profile get --plain ``` -Brand and campaign registration still happen in the Bandwidth App. See [AGENTS.md](AGENTS.md) for the full command reference, including update, delete/restore, and version history. +If you register directly with TCR (not through import), brand registration now happens in the CLI too — see `band tendlc brand create` above. Campaign registration still happens in the Bandwidth App either way. See [AGENTS.md](AGENTS.md#customer-profiles) for the full customer-profile command reference, including update, delete/restore, and version history. ### 10DLC campaigns (local numbers) @@ -280,7 +280,7 @@ band tendlc number +19195551234 --plain # check a specific number band tendlc campaigns --plain # list campaigns on your account ``` -Campaign and brand registration happen in the Bandwidth App — see [dev.bandwidth.com](https://dev.bandwidth.com/docs/messaging/campaign-management/) for the full guide. Once you have a campaign, assign numbers to it with `band tnoption assign`. +These two commands are for **import** customers (accounts that register campaigns through TCR and import them to Bandwidth) — campaign registration for them still happens in the Bandwidth App; see [dev.bandwidth.com](https://dev.bandwidth.com/docs/messaging/campaign-management/) for the full guide. **Direct** customers register brands via `band tendlc brand create` (above); campaign registration for direct customers isn't in the CLI yet. Once you have a campaign either way, assign numbers to it with `band tnoption assign`. ### Toll-free verification (toll-free numbers) @@ -477,6 +477,25 @@ Sub-accounts (formerly known as sites) are the top-level container. Locations (f | `band tnoption get ` | Check the status of a TN Option Order | | `band tnoption list` | List TN Option Orders (filter by `--status`, `--tn`) | +### 10DLC brands and vettings (direct customers) + +`band tendlc brand` and `band tendlc vetting` register and manage 10DLC brands for accounts that register directly with TCR (not through import). Requires the Registration Center feature and Campaign Management role — check with `band tendlc status --plain`. A brand needs a customer profile first (`band customer-profile create`); see [AGENTS.md](AGENTS.md#10dlc-brands) for the full flag matrix, `--wait` semantics, and exit codes. + +| Command | What it does | +|---------|-------------| +| `band tendlc brand create --customer-profile-id --brand-type ...` | Register a brand (required flags vary by `--brand-type`; `--wait` blocks until VERIFIED/VETTED_VERIFIED) | +| `band tendlc brand list` | List brands (filter by `--customer-profile-id-contains`, `--brand-id-contains`, `--identity-status`, `--brand-type`, name substrings) | +| `band tendlc brand get ` | Get one brand — accepts either the `bandwidthId` or the TCR `brandId` | +| `band tendlc brand history ` | Show a brand's activity log | +| `band tendlc brand update ` | Update a brand (read-modify-write; `--confirm` required when an identity-affecting field changes) | +| `band tendlc brand delete ` | Permanently delete a brand (`--confirm` required; does not delete the backing customer profile) | +| `band tendlc brand reverify ` | Resubmit a brand for identity verification ($4 fee; `--confirm` required) | +| `band tendlc brand resend-2fa ` | Re-send the Auth+ business 2FA verification email | +| `band tendlc brand refresh ` | Re-pull a brand's current state from TCR | +| `band tendlc vetting list ` | List the external vettings recorded against a brand | +| `band tendlc vetting request --evp --class ` | Order a new external vetting (billable; `--confirm` required) | +| `band tendlc vetting import --evp ` | Record a vetting already performed outside Bandwidth (not billable, no `--confirm`) | + ### SIP trunk authentication | Command | What it does | diff --git a/cmd/tendlc/async.go b/cmd/tendlc/async.go new file mode 100644 index 0000000..dd0bafd --- /dev/null +++ b/cmd/tendlc/async.go @@ -0,0 +1,181 @@ +package tendlc + +import ( + "fmt" + "time" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" + tendlcsvc "github.com/Bandwidth/cli/internal/tendlc" +) + +// pollTarget describes what to poll and how to interpret it. +type pollTarget struct { + // Noun names the resource in messages: "brand", "vetting". + Noun string + // Fetch reads the resource. found=false means the API answered 404, which + // is "not readable yet" for creates and "done" for deletes — see + // GoneIsDone. An error is a hard transport or decode failure. + Fetch func() (obj map[string]any, found bool, err error) + // Classify maps a fetched resource to an outcome. Called only when found. + Classify func(obj map[string]any) tendlcsvc.StateClass + // Remediate returns operator-facing next steps for a business-failure + // state. Optional; "" prints nothing. + Remediate func(obj map[string]any) string + // LastSeenStatus extracts a short status string from a fetched object, + // for inclusion in the timeout/error receipt (see awaitTerminal). This is + // what lets a caller who times out see what was last observed instead of + // a receipt frozen at call time. + // + // Optional; nil for polls with nothing meaningful to report — a delete + // poll (GoneIsDone) never reaches this, since a found object there is a + // still-existing resource, not a status to describe. Leave it nil rather + // than returning "": an empty string is treated as "nothing observed + // yet" and is omitted from the receipt, so a real empty status and "not + // wired up" must not be conflated by supplying one where there's nothing + // to say. + LastSeenStatus func(obj map[string]any) string + // GoneIsDone makes a 404 the success condition, for delete polls. + GoneIsDone bool +} + +// awaitTerminal polls until the resource settles, then writes stdout itself. +// +// It owns stdout on every path because of the one rule that matters here: +// after a 202 the write may already have succeeded, so the accepted resource's +// ID is the single piece of information that cannot be recovered if this +// command exits without printing it. A wrapped error alone prints no +// structured receipt, which is exactly the failure this function exists to +// prevent — modeled on cmd/quickstart's failWithPartial. +// +// receipt is what gets printed on every non-success outcome. It must already +// contain the ID from the 202 and, where one exists, a resume command. +func awaitTerminal(cmd *cobra.Command, t pollTarget, receipt map[string]any, timeout, interval time.Duration) error { + format, plain := cmdutil.OutputFlags(cmd) + + // emitReceipt writes the partial-result receipt. A failure to write it is + // deliberately swallowed in favor of the original error: the caller needs + // to know the operation failed more than it needs to know stdout was + // closed, and returning the write error would bury the real cause. + // + // This prints via output.Stdout, not output.StdoutAuto, deliberately: the + // receipt is a value we built ourselves, not a raw API envelope, so it + // never needs FlattenResponse's envelope-unwrapping. That matters because + // FlattenResponse treats ANY single-key map as a wrapper and unwraps it to + // its bare value — a receipt of just {"bandwidthId": "WABC"} would print + // as the bare string "WABC" instead of a JSON object, which is exactly + // the "no structured receipt" failure this function exists to prevent. + + // lastStatus holds the most recent status LastSeenStatus reported, so a + // timeout or mid-poll transport failure can say what was last observed + // instead of printing a receipt frozen at call time. It stays "" for + // targets with no LastSeenStatus (or that never got a successful, + // classifiable fetch), and emitReceipt below leaves it out of the + // receipt entirely in that case rather than adding an empty field. + var lastStatus string + + emitReceipt := func() { + if lastStatus != "" { + receipt["lastSeenStatus"] = lastStatus + } + _ = output.Stdout(format, receipt) + } + + result, err := cmdutil.Poll(cmdutil.PollConfig{ + Context: cmd.Context(), + Interval: interval, + Timeout: timeout, + Check: func() (bool, interface{}, error) { + obj, found, err := t.Fetch() + if err != nil { + return false, nil, err + } + if !found { + // 404: done for a delete poll, not-ready-yet for anything else. + return t.GoneIsDone, nil, nil + } + if t.GoneIsDone { + return false, nil, nil + } + if t.LastSeenStatus != nil { + if s := t.LastSeenStatus(obj); s != "" { + lastStatus = s + } + } + switch t.Classify(obj) { + case tendlcsvc.StateSuccess, tendlcsvc.StateFailure: + return true, obj, nil + default: + return false, nil, nil + } + }, + }) + if err != nil { + // Timeout, cancellation, or a transport failure after acceptance. All + // three get the receipt: the write may have landed. + emitReceipt() + return err + } + + if result == nil { + // A delete poll that found the resource gone. There is no final + // resource to print, so the receipt IS the success output. + emitReceipt() + return nil + } + + final, ok := result.(map[string]any) + if !ok { + emitReceipt() + return fmt.Errorf("polling %s returned an unexpected shape (%T)", t.Noun, result) + } + + if t.Classify(final) == tendlcsvc.StateFailure { + // final is real API data (not a synthetic receipt), so StdoutAuto's + // flatten-on-plain behavior is correct here — see emitReceipt above + // for why receipts print differently. + // + // The write error is reported but does not replace the return value: + // this is a business failure regardless of whether the write + // succeeded, and returning the bare write error here would drop the + // ConflictError classification (exit 4) along with the remediation + // text below, silently downgrading a failed brand/vetting into + // "something went wrong printing it." + if err := output.StdoutAuto(format, plain, final); err != nil { + cmd.PrintErrln(fmt.Sprintf("writing result: %v", err)) + } + msg := fmt.Sprintf("%s did not complete successfully", t.Noun) + if t.Remediate != nil { + if r := t.Remediate(final); r != "" { + msg = r + } + } + cmd.PrintErrln(msg) + return &cmdutil.ConflictError{Message: msg} + } + + // final is real API data, not a synthetic receipt — see emitReceipt above + // for why receipts go through output.Stdout instead of StdoutAuto. + return output.StdoutAuto(format, plain, final) +} + +// fetchBrand adapts a brand read into pollTarget.Fetch, translating a 404 into +// found=false rather than an error. +func fetchBrand(svc *tendlcsvc.Service, brandID string) func() (map[string]any, bool, error) { + return func() (map[string]any, bool, error) { + env, err := svc.GetBrand(brandID) + if err != nil { + if isNotFound(err) { + return nil, false, nil + } + return nil, false, err + } + obj, err := env.Object() + if err != nil { + return nil, false, err + } + return obj, true, nil + } +} diff --git a/cmd/tendlc/async_test.go b/cmd/tendlc/async_test.go new file mode 100644 index 0000000..322f24d --- /dev/null +++ b/cmd/tendlc/async_test.go @@ -0,0 +1,336 @@ +package tendlc + +import ( + "bytes" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + tendlcsvc "github.com/Bandwidth/cli/internal/tendlc" + "github.com/Bandwidth/cli/internal/testutil" +) + +// asyncTestCmd returns a bare command carrying the output flags awaitTerminal +// reads, plus a buffer capturing its stderr. +// +// stdout is NOT captured here: internal/output writes to os.Stdout directly, +// not to cmd.OutOrStdout(), so cmd.SetOut would silently capture nothing and +// every stdout assertion would pass against an empty string. Use +// testutil.CaptureStdout around the awaitTerminal call instead — the same +// mechanism cmd/customerprofile's runCmd uses. +// +// The command is its own root, which is what cmdutil.OutputFlags needs: it +// resolves --format and --plain through cmd.Root(). +func asyncTestCmd() (*cobra.Command, *bytes.Buffer) { + var errBuf bytes.Buffer + c := &cobra.Command{Use: "x"} + c.Flags().String("format", "json", "") + c.Flags().Bool("plain", true, "") + c.SetOut(io.Discard) + c.SetErr(&errBuf) + return c, &errBuf +} + +// runAwait calls awaitTerminal with stdout captured. +func runAwait(t *testing.T, cmd *cobra.Command, tgt pollTarget, receipt map[string]any, + timeout, interval time.Duration) (stdout string, err error) { + t.Helper() + stdout = testutil.CaptureStdout(t, func() { + err = awaitTerminal(cmd, tgt, receipt, timeout, interval) + }) + return stdout, err +} + +func decodeReceipt(t *testing.T, s string) map[string]any { + t.Helper() + var m map[string]any + if err := json.Unmarshal([]byte(strings.TrimSpace(s)), &m); err != nil { + t.Fatalf("stdout is not JSON (%v): %q", err, s) + } + return m +} + +func TestAwaitTerminalSuccessPrintsFinalResource(t *testing.T) { + cmd, _ := asyncTestCmd() + target := pollTarget{ + Noun: "brand", + Fetch: func() (map[string]any, bool, error) { + return map[string]any{"bandwidthId": "WABC", "brandIdentityStatus": "VERIFIED"}, true, nil + }, + Classify: func(o map[string]any) tendlcsvc.StateClass { + s, _ := o["brandIdentityStatus"].(string) + return tendlcsvc.ClassifyBrandIdentity(s) + }, + } + out, err := runAwait(t, cmd, target, map[string]any{"bandwidthId": "WABC"}, time.Second, time.Millisecond) + if err != nil { + t.Fatalf("want success, got %v", err) + } + got := decodeReceipt(t, out) + if got["brandIdentityStatus"] != "VERIFIED" { + t.Errorf("stdout should be the final resource, got %v", got) + } +} + +// A business failure is exit 4, and the resource still reaches stdout — +// the caller needs to see which state it settled in. +// +// This uses ERROR, not UNVERIFIED: ERROR is the only brandIdentityStatus that +// still classifies as StateFailure. UNVERIFIED moved to StatePending because +// production reports it for both "still registering" and "rejected" alike +// (see ClassifyBrandIdentity's comment) — it now polls to timeout instead of +// landing here. +func TestAwaitTerminalBusinessFailureExitsFourWithResource(t *testing.T) { + cmd, errBuf := asyncTestCmd() + target := pollTarget{ + Noun: "brand", + Fetch: func() (map[string]any, bool, error) { + return map[string]any{"bandwidthId": "WABC", "brandIdentityStatus": "ERROR"}, true, nil + }, + Classify: func(o map[string]any) tendlcsvc.StateClass { + s, _ := o["brandIdentityStatus"].(string) + return tendlcsvc.ClassifyBrandIdentity(s) + }, + Remediate: func(o map[string]any) string { + s, _ := o["brandIdentityStatus"].(string) + return tendlcsvc.BrandRemediation(s) + }, + } + out, err := runAwait(t, cmd, target, map[string]any{"bandwidthId": "WABC"}, time.Second, time.Millisecond) + if err == nil { + t.Fatal("want an error for a business-failure state") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitConflict { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitConflict) + } + got := decodeReceipt(t, out) + if got["bandwidthId"] != "WABC" { + t.Errorf("stdout must still carry the resource, got %v", got) + } + if !strings.Contains(errBuf.String(), "refresh") { + t.Errorf("stderr should carry state-specific remediation, got %q", errBuf.String()) + } +} + +// The whole point of the runner: a timeout must not lose the ID. +func TestAwaitTerminalTimeoutStillEmitsReceipt(t *testing.T) { + cmd, _ := asyncTestCmd() + target := pollTarget{ + Noun: "brand", + Fetch: func() (map[string]any, bool, error) { + return map[string]any{"bandwidthId": "WABC", "brandIdentityStatus": "REGISTERING"}, true, nil + }, + Classify: func(o map[string]any) tendlcsvc.StateClass { + s, _ := o["brandIdentityStatus"].(string) + return tendlcsvc.ClassifyBrandIdentity(s) + }, + } + receipt := map[string]any{"bandwidthId": "WABC", "resume": "band tendlc brand get WABC"} + out, err := runAwait(t, cmd, target, receipt, 20*time.Millisecond, time.Millisecond) + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitTimeout { + t.Errorf("exit code = %d, want %d (timeout)", code, cmdutil.ExitTimeout) + } + got := decodeReceipt(t, out) + if got["bandwidthId"] != "WABC" { + t.Fatalf("timeout must still print the ID, got %v", got) + } + if got["resume"] != "band tendlc brand get WABC" { + t.Errorf("timeout receipt must carry a resume command, got %v", got) + } + // This target sets no LastSeenStatus func — the field must be optional + // and absent, not empty-stringed in, so a delete poll (which has no + // status to report) never gets a bogus lastSeenStatus:"" in its receipt. + if _, ok := got["lastSeenStatus"]; ok { + t.Errorf("a target with no LastSeenStatus must not add the field, got %v", got) + } +} + +// The whole reason UNVERIFIED now polls to timeout instead of failing fast: +// the timeout receipt must say what was last observed, so an agent that +// times out can tell "still might verify" from "definitely broken" instead +// of reading a receipt that was frozen at call time and says nothing about +// the brand's actual state. +func TestAwaitTerminalTimeoutIncludesLastSeenStatus(t *testing.T) { + cmd, _ := asyncTestCmd() + target := pollTarget{ + Noun: "brand", + Fetch: func() (map[string]any, bool, error) { + return map[string]any{"bandwidthId": "WABC", "brandIdentityStatus": "UNVERIFIED"}, true, nil + }, + Classify: func(o map[string]any) tendlcsvc.StateClass { + s, _ := o["brandIdentityStatus"].(string) + return tendlcsvc.ClassifyBrandIdentity(s) + }, + LastSeenStatus: func(o map[string]any) string { + s, _ := o["brandIdentityStatus"].(string) + return s + }, + } + receipt := map[string]any{"bandwidthId": "WABC", "resume": "band tendlc brand get WABC"} + out, err := runAwait(t, cmd, target, receipt, 20*time.Millisecond, time.Millisecond) + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitTimeout { + t.Errorf("exit code = %d, want %d (timeout)", code, cmdutil.ExitTimeout) + } + got := decodeReceipt(t, out) + if got["bandwidthId"] != "WABC" { + t.Fatalf("timeout must still print the ID, got %v", got) + } + if got["lastSeenStatus"] != "UNVERIFIED" { + t.Fatalf("timeout receipt must carry the last observed status, got %v", got) + } +} + +// A transport failure DURING polling is the nastiest case: the write was +// accepted, so exiting with only an error loses the ID forever. +func TestAwaitTerminalPollTransportErrorStillEmitsReceipt(t *testing.T) { + cmd, _ := asyncTestCmd() + boom := &api.APIError{StatusCode: 500, Body: "upstream exploded"} + target := pollTarget{ + Noun: "brand", + Fetch: func() (map[string]any, bool, error) { return nil, false, boom }, + Classify: func(map[string]any) tendlcsvc.StateClass { return tendlcsvc.StatePending }, + } + out, err := runAwait(t, cmd, target, map[string]any{"bandwidthId": "WABC"}, time.Second, time.Millisecond) + if err == nil { + t.Fatal("want the poll error surfaced") + } + if !errors.Is(err, boom) && !strings.Contains(err.Error(), "upstream exploded") { + t.Errorf("original error must be preserved, got %v", err) + } + got := decodeReceipt(t, out) + if got["bandwidthId"] != "WABC" { + t.Fatalf("a poll failure must still print the ID, got %v", got) + } +} + +// For create-style polls a 404 means "not readable yet", not "gone". +func TestAwaitTerminalNotFoundIsPendingForCreates(t *testing.T) { + cmd, _ := asyncTestCmd() + calls := 0 + target := pollTarget{ + Noun: "brand", + Fetch: func() (map[string]any, bool, error) { + calls++ + if calls < 3 { + return nil, false, nil + } + return map[string]any{"bandwidthId": "WABC", "brandIdentityStatus": "VERIFIED"}, true, nil + }, + Classify: func(o map[string]any) tendlcsvc.StateClass { + s, _ := o["brandIdentityStatus"].(string) + return tendlcsvc.ClassifyBrandIdentity(s) + }, + } + if _, err := runAwait(t, cmd, target, map[string]any{"bandwidthId": "WABC"}, time.Second, time.Millisecond); err != nil { + t.Fatalf("404 before readiness must keep polling, got %v", err) + } + if calls < 3 { + t.Errorf("polled %d times, want at least 3", calls) + } +} + +// For delete-style polls a 404 is the success condition. No command treats +// 404 as both, which is why this is a field on the target rather than a +// heuristic. +func TestAwaitTerminalGoneIsDoneForDeletes(t *testing.T) { + cmd, _ := asyncTestCmd() + target := pollTarget{ + Noun: "brand", + GoneIsDone: true, + Fetch: func() (map[string]any, bool, error) { return nil, false, nil }, + Classify: func(map[string]any) tendlcsvc.StateClass { return tendlcsvc.StatePending }, + } + receipt := map[string]any{"bandwidthId": "WABC", "deleted": true} + out, err := runAwait(t, cmd, target, receipt, time.Second, time.Millisecond) + if err != nil { + t.Fatalf("want success when the resource is gone, got %v", err) + } + got := decodeReceipt(t, out) + if got["deleted"] != true { + t.Errorf("delete success should print the receipt, got %v", got) + } +} + +// fetchBrandStubServer returns a stub that answers GET .../brands/{id} with +// the given status code and body, and a Service pointed at it — the same +// api.NewClientNoAuth seam used by cmd/tendlc/status_test.go. +func fetchBrandStubServer(t *testing.T, code int, body string) *tendlcsvc.Service { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(code) + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + return tendlcsvc.NewService(api.NewClientNoAuth(srv.URL), "9901287") +} + +// fetchBrand is the sole translator of a real 404 into pollTarget's +// found=false — the mechanism the create-vs-delete GoneIsDone contract +// depends on — so all three outcomes need direct coverage against a real +// HTTP response, not just the classifier logic above it. +func TestFetchBrandFoundReturnsObject(t *testing.T) { + svc := fetchBrandStubServer(t, http.StatusOK, `{"data":{"bandwidthId":"WABC","brandIdentityStatus":"VERIFIED"}}`) + obj, found, err := fetchBrand(svc, "WABC")() + if err != nil { + t.Fatalf("want no error, got %v", err) + } + if !found { + t.Fatal("want found=true for a 200 response") + } + if obj["bandwidthId"] != "WABC" { + t.Errorf("obj = %v, want bandwidthId WABC", obj) + } +} + +// A 404 must translate to found=false with NO error — that translation is +// the whole point of fetchBrand. An error here would make a create poll fail +// immediately on a resource that just isn't readable yet, instead of +// retrying until it appears or the poll times out. +func TestFetchBrandNotFoundIsFoundFalseNotError(t *testing.T) { + svc := fetchBrandStubServer(t, http.StatusNotFound, `{"errors":[{"description":"brand not found"}]}`) + obj, found, err := fetchBrand(svc, "WABC")() + if err != nil { + t.Fatalf("want no error for a 404, got %v", err) + } + if found { + t.Fatal("want found=false for a 404") + } + if obj != nil { + t.Errorf("want a nil object for a 404, got %v", obj) + } +} + +func TestFetchBrandServerErrorIsError(t *testing.T) { + svc := fetchBrandStubServer(t, http.StatusInternalServerError, `{"errors":[{"description":"boom"}]}`) + _, found, err := fetchBrand(svc, "WABC")() + if err == nil { + t.Fatal("want an error for a 500") + } + if found { + t.Error("want found=false alongside the error") + } +} + +// A 200 with a non-object data field (e.g. an array) is a malformed +// response, not "not ready yet". Reporting it as found=false would make a +// create poll spin silently until timeout instead of failing fast on a +// response shape it can never recover from. +func TestFetchBrandMalformedDataIsError(t *testing.T) { + svc := fetchBrandStubServer(t, http.StatusOK, `{"data":[{"bandwidthId":"WABC"}]}`) + _, found, err := fetchBrand(svc, "WABC")() + if err == nil { + t.Fatal("want an error when data is not an object") + } + if found { + t.Error("want found=false alongside the error") + } +} diff --git a/cmd/tendlc/brand.go b/cmd/tendlc/brand.go new file mode 100644 index 0000000..af0d9fa --- /dev/null +++ b/cmd/tendlc/brand.go @@ -0,0 +1,26 @@ +package tendlc + +import ( + "github.com/spf13/cobra" +) + +// brandCmd is the `band tendlc brand` parent. +var brandCmd = &cobra.Command{ + Use: "brand", + Short: "Manage 10DLC brands", + Long: `Register and manage 10DLC brands. + +A brand needs a customer profile first, and a profile backs exactly one brand: +'band customer-profile create' then 'band tendlc brand create'. A brand must +reach VERIFIED or VETTED_VERIFIED before it can carry campaigns. + +Brands have two IDs. bandwidthId exists immediately; brandId is assigned by TCR +and is null until registration completes. Commands here accept either. + +Requires the Registration Center feature and the Campaign Management role. +Check with 'band tendlc status --plain'.`, +} + +func init() { + Cmd.AddCommand(brandCmd) +} diff --git a/cmd/tendlc/brand_create.go b/cmd/tendlc/brand_create.go new file mode 100644 index 0000000..8093098 --- /dev/null +++ b/cmd/tendlc/brand_create.go @@ -0,0 +1,258 @@ +package tendlc + +import ( + "fmt" + "time" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + cpsvc "github.com/Bandwidth/cli/internal/customerprofile" + "github.com/Bandwidth/cli/internal/output" + tendlcsvc "github.com/Bandwidth/cli/internal/tendlc" +) + +var ( + brandCreateOpts tendlcsvc.BrandCreateOptions + brandCreateWait bool + brandCreateTimeout int +) + +// brandCreatePollInterval is how often --wait re-checks the brand while it +// registers. Brand identity verification runs through TCR, an external +// registry, so — unlike the 2-second intervals used for Bandwidth-internal +// polls elsewhere in this CLI — a slower interval is appropriate here. +const brandCreatePollInterval = 5 * time.Second + +// customerProfileService builds a customer-profile Service for the create +// pre-flight check. A package-level seam of its own, separate from this +// package's `service` seam (declared in status.go), so tests can stub the +// profile read independently of the brand-create request itself. +var customerProfileService = func(cmd *cobra.Command) (*cpsvc.Service, error) { + client, acctID, err := cmdutil.PlatformClient(cmdutil.AccountIDFlag(cmd)) + if err != nil { + return nil, err + } + return cpsvc.NewService(client, acctID), nil +} + +func init() { + f := brandCreateCmd.Flags() + f.StringVar(&brandCreateOpts.CustomerProfileID, "customer-profile-id", "", "Customer profile backing this brand (required)") + f.StringVar(&brandCreateOpts.BrandType, "brand-type", "", "Brand entity type: PRIVATE_PROFIT, PUBLIC_PROFIT, NON_PROFIT, GOVERNMENT, SOLE_PROPRIETOR (required)") + f.StringVar(&brandCreateOpts.DisplayName, "display-name", "", "Display name (required)") + f.StringVar(&brandCreateOpts.CompanyName, "company-name", "", "Legal company name") + f.StringVar(&brandCreateOpts.Street, "street", "", "Street address (required)") + f.StringVar(&brandCreateOpts.City, "city", "", "City (required)") + f.StringVar(&brandCreateOpts.State, "state", "", "State or province (required)") + f.StringVar(&brandCreateOpts.PostalCode, "postal-code", "", "Postal code (required)") + f.StringVar(&brandCreateOpts.CountryCodeA3, "country-code-a3", "", "ISO 3166-1 alpha-3 country code (required)") + f.StringVar(&brandCreateOpts.Phone, "phone", "", "Business phone number (required)") + f.StringVar(&brandCreateOpts.Email, "email", "", "Business email address (required)") + f.StringVar(&brandCreateOpts.Vertical, "vertical", "", "Industry vertical") + f.StringVar(&brandCreateOpts.EIN, "ein", "", "Employer Identification Number") + f.StringVar(&brandCreateOpts.EINIssuingCountryCodeA3, "ein-issuing-country-code-a3", "", "Country that issued the EIN") + f.StringVar(&brandCreateOpts.Website, "website", "", "Business website URL") + f.StringVar(&brandCreateOpts.StockSymbol, "stock-symbol", "", "Stock ticker symbol") + f.StringVar(&brandCreateOpts.StockExchange, "stock-exchange", "", "Stock exchange") + f.StringVar(&brandCreateOpts.BusinessContactEmail, "business-contact-email", "", "Business contact email") + f.StringVar(&brandCreateOpts.AltBusinessID, "alt-business-id", "", "Alternate business identifier (e.g. DUNS number)") + f.StringVar(&brandCreateOpts.AltBusinessIDType, "alt-business-id-type", "", "Type of the alternate business identifier") + f.StringVar(&brandCreateOpts.FirstName, "first-name", "", "Contact first name (sole proprietor)") + f.StringVar(&brandCreateOpts.LastName, "last-name", "", "Contact last name (sole proprietor)") + f.StringVar(&brandCreateOpts.MobilePhone, "mobile-phone", "", "Contact mobile phone (sole proprietor)") + f.StringVar(&brandCreateOpts.IPAddress, "ip-address", "", "IP address the form was completed from (sole proprietor)") + f.BoolVar(&brandCreateWait, "wait", false, "Block until the brand's identity status reaches a terminal state") + f.IntVar(&brandCreateTimeout, "timeout", 300, "Seconds to wait when --wait is set") + brandCmd.AddCommand(brandCreateCmd) + brandCmd.AddCommand(brandRefreshCmd) +} + +var brandCreateCmd = &cobra.Command{ + Use: "create", + Short: "Register a 10DLC brand", + Long: `Registers a 10DLC brand backed by an existing customer profile. + +A customer profile backs exactly one brand — create a fresh profile per +brand with 'band customer-profile create'; reusing one fails at brand +creation. + +Before submitting, this command reads the customer profile named by +--customer-profile-id. Measured against production, the create endpoint +itself does not reject a garbage or typo'd profile ID — it silently +discards it and creates an orphan brand with no profile association that +can never verify. So a 404 on that read stops the create here, naming the +bad ID. A 403 does not stop it: Customer Profiles Access is a role separate +from Campaign Management, so a caller entitled to create brands must not be +blocked by a check they lack permission to run — it proceeds with a +one-line warning on stderr instead. Any other pre-flight failure degrades +the same way; this check is a guard, not a gate. + +This is a non-idempotent, billable write. After an ambiguous failure — a +timeout or a dropped connection — do not blindly retry: list brands +filtered by --customer-profile-id-contains and reconcile against what you +submitted first. Retrying blind risks a second brand against the same profile.`, + Example: ` band tendlc brand create --customer-profile-id CP123 --brand-type PRIVATE_PROFIT \ + --display-name "Acme Corp" --company-name "Acme Corporation" \ + --street "123 Main St" --city Raleigh --state NC --postal-code 27601 \ + --country-code-a3 USA --phone +18885551234 --email ops@acme.com \ + --vertical RETAIL --ein 123456789 --ein-issuing-country-code-a3 USA + + band tendlc brand create --customer-profile-id CP123 --brand-type PRIVATE_PROFIT \ + ... --wait --timeout 600`, + // No positional args: this is a non-idempotent, billable create, so a + // stray positional (e.g. a typo'd second word meant for another flag) + // must be rejected rather than silently ignored and creating an + // unintended brand — see TestBrandCommandsRejectStrayPositionals's + // comment on the PR 2 incident this guards against. + Args: cobra.NoArgs, + // Required-ness is enforced in RunE, not via MarkFlagRequired: cobra + // rejects before RunE, which reports one flag at a time and would block a + // future interactive prompt from filling them in. + RunE: func(cmd *cobra.Command, args []string) error { + if err := tendlcsvc.ValidateBrandCreate(brandCreateOpts); err != nil { + return err + } + svc, err := service(cmd) + if err != nil { + return err + } + if err := preflightCustomerProfile(cmd, brandCreateOpts.CustomerProfileID); err != nil { + return err + } + + env, err := svc.CreateBrand(tendlcsvc.BuildBrandCreateRequest(brandCreateOpts)) + if err != nil { + return roleGateError(err, "Campaign Management") + } + + receipt, bandwidthID, err := buildAcceptedReceipt(cmd, env) + if err != nil { + return err + } + + if !brandCreateWait { + format, _ := cmdutil.OutputFlags(cmd) + return output.Stdout(format, receipt) + } + + target := pollTarget{ + Noun: "brand", + Fetch: fetchBrand(svc, bandwidthID), + Classify: func(o map[string]any) tendlcsvc.StateClass { + status, _ := o["brandIdentityStatus"].(string) + return tendlcsvc.ClassifyBrandIdentity(status) + }, + Remediate: func(o map[string]any) string { + status, _ := o["brandIdentityStatus"].(string) + return tendlcsvc.BrandRemediation(status) + }, + LastSeenStatus: func(o map[string]any) string { + status, _ := o["brandIdentityStatus"].(string) + return status + }, + } + // UNVERIFIED now polls to timeout instead of failing fast (see + // ClassifyBrandIdentity), which makes a timeout the normal way a + // truly-failed registration surfaces. The receipt has to carry that + // weight, so a note is added directly to it here — advisory only, + // and it never reaches the success or business-failure (ERROR) output + // paths, only the timeout/transport-error receipt (see awaitTerminal's + // emitReceipt). + receipt["note"] = "if this timed out at UNVERIFIED, the brand may still be registering with TCR " + + "rather than having failed. Check 'band tendlc brand get " + bandwidthID + + "' for its current status, or 'band tendlc brand history " + bandwidthID + "' for the full history." + return awaitTerminal(cmd, target, receipt, time.Duration(brandCreateTimeout)*time.Second, brandCreatePollInterval) + }, +} + +var brandRefreshCmd = &cobra.Command{ + Use: "refresh ", + Short: "Re-pull a brand's current state from TCR", + Long: `Refreshes a brand by re-pulling its current state from TCR — for import +and direct customers alike. + +This is not a create: it posts to the same POST /brands endpoint create +uses, but a body containing only brandId is what makes it a refresh instead +of a new registration. Sending any other key would turn it back into a +create, so nothing else is added. Use it after making a change directly in +TCR, or to pick up a brandId that was still null when the brand was +created.`, + Example: ` band tendlc brand refresh BGJR2BA --plain`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + svc, err := service(cmd) + if err != nil { + return err + } + env, err := svc.CreateBrand(tendlcsvc.BuildBrandRefreshRequest(args[0])) + if err != nil { + return roleGateError(err, "Campaign Management") + } + receipt, _, err := buildAcceptedReceipt(cmd, env) + if err != nil { + return err + } + format, _ := cmdutil.OutputFlags(cmd) + return output.Stdout(format, receipt) + }, +} + +// preflightCustomerProfile reads the customer profile named by profileID +// before create writes the brand — see brandCreateCmd's Long for why. It +// degrades rather than blocks: only a definitive 404 (the profile does not +// exist) stops the create. A 403, a failure to build the profile service, or +// any other error is reported on stderr and swallowed, because this check is +// a guard against a specific typo, not a gate on the whole command. +func preflightCustomerProfile(cmd *cobra.Command, profileID string) error { + cpSvc, err := customerProfileService(cmd) + if err == nil { + _, err = cpSvc.Get(profileID) + } + if err == nil { + return nil + } + if isNotFound(err) { + return fmt.Errorf("customer profile %q not found — run 'band customer-profile list' to see valid profile IDs (%w)", profileID, err) + } + cmd.PrintErrf("warning: could not verify customer profile %q before creating the brand (%v); proceeding anyway\n", profileID, err) + return nil +} + +// buildAcceptedReceipt turns a brand-write acceptance response — the POST +// /brands 202 that both create and refresh get, or the PUT /brands/{id} +// response update gets — into the receipt shape all three print: +// {bandwidthId, brandId (if present), status, resume}. bandwidthId exists +// immediately; brandId is assigned by TCR and is commonly absent until +// registration completes, so it is omitted entirely rather than sent as null. +// +// If the response carries no bandwidthId, there is no ID to poll or resume +// with, so the caller must not proceed. This prints whatever the body +// actually was via output.Stdout, not StdoutAuto: env.Data is real API data, +// not a synthetic receipt, but it can still be a single-key map (the orphan +// brand's {"accounts":[...]} body is exactly one), and FlattenResponse +// unwraps any single-key map — under --plain that would drop the key and +// print a bare array instead of the object it came from. See async.go's +// emitReceipt for the same reasoning applied to synthetic receipts. +func buildAcceptedReceipt(cmd *cobra.Command, env *api.Envelope) (receipt map[string]any, bandwidthID string, err error) { + obj, objErr := env.Object() + bandwidthID, _ = obj["bandwidthId"].(string) + if objErr != nil || bandwidthID == "" { + format, _ := cmdutil.OutputFlags(cmd) + if writeErr := output.Stdout(format, env.Data); writeErr != nil { + cmd.PrintErrln(fmt.Sprintf("writing response: %v", writeErr)) + } + return nil, "", fmt.Errorf("brand response did not include a bandwidthId") + } + + receipt = map[string]any{ + "bandwidthId": bandwidthID, + "status": "accepted", + "resume": "band tendlc brand get " + bandwidthID, + } + if brandID, ok := obj["brandId"].(string); ok && brandID != "" { + receipt["brandId"] = brandID + } + return receipt, bandwidthID, nil +} diff --git a/cmd/tendlc/brand_create_test.go b/cmd/tendlc/brand_create_test.go new file mode 100644 index 0000000..f702d00 --- /dev/null +++ b/cmd/tendlc/brand_create_test.go @@ -0,0 +1,420 @@ +package tendlc + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + cpsvc "github.com/Bandwidth/cli/internal/customerprofile" +) + +// validPrivateProfitArgs returns a full, valid PRIVATE_PROFIT flag set for +// `brand create`, so each test only needs to state what it adds or omits. +func validPrivateProfitArgs(extra ...string) []string { + args := []string{ + "brand", "create", + "--customer-profile-id", "CP123", + "--brand-type", "PRIVATE_PROFIT", + "--display-name", "Acme Corp", + "--company-name", "Acme Corporation", + "--street", "123 Main St", + "--city", "Raleigh", + "--state", "NC", + "--postal-code", "27601", + "--country-code-a3", "USA", + "--phone", "+18885551234", + "--email", "ops@acme.com", + "--vertical", "RETAIL", + "--ein", "123456789", + "--ein-issuing-country-code-a3", "USA", + } + return append(args, extra...) +} + +// stubProfileServer answers every request with the given status and body, +// for exercising the create pre-flight's customer-profile read. +func stubProfileServer(t *testing.T, code int, body string) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(code) + _, _ = w.Write([]byte(body)) + }) +} + +// stubProfileService swaps the customerProfileService seam to point at srv, +// restoring the original after the test. Kept independent of runBrandCmd's +// `service` swap because the pre-flight check is its own seam — see +// customerProfileService's doc comment in brand_create.go — so a test can +// stub the profile read and the brand-create request separately. +func stubProfileService(t *testing.T, srv *httptest.Server) { + t.Helper() + orig := customerProfileService + customerProfileService = func(cmd *cobra.Command) (*cpsvc.Service, error) { + if srv == nil { + t.Fatal("pre-flight made a request but no profile stub was provided") + } + return cpsvc.NewService(api.NewClientNoAuth(srv.URL), "9901287"), nil + } + t.Cleanup(func() { customerProfileService = orig }) +} + +// stubBrandCreateCapturing answers POST /brands with a 202 carrying +// bandwidthID, recording each request's raw JSON body so a test can assert +// exactly what was sent. +func stubBrandCreateCapturing(t *testing.T, bandwidthID string) (*httptest.Server, *[]string) { + var bodies []string + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + b, _ := io.ReadAll(r.Body) + bodies = append(bodies, string(b)) + w.WriteHeader(http.StatusAccepted) + _, _ = fmt.Fprintf(w, `{"data":{"bandwidthId":%q}}`, bandwidthID) + }) + return srv, &bodies +} + +// stubBrandCreateThenPoll answers POST /brands with a 202 accepting +// bandwidthID, and GET /brands/{id} with identityStatus on every poll — +// enough for a --wait test that settles on the very first poll, so it never +// has to sleep out the poll interval. +func stubBrandCreateThenPoll(t *testing.T, bandwidthID, identityStatus string) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + w.WriteHeader(http.StatusAccepted) + _, _ = fmt.Fprintf(w, `{"data":{"bandwidthId":%q}}`, bandwidthID) + return + } + _, _ = fmt.Fprintf(w, `{"data":{"bandwidthId":%q,"brandIdentityStatus":%q}}`, bandwidthID, identityStatus) + }) +} + +func decodeStdout(t *testing.T, s string) map[string]any { + t.Helper() + var m map[string]any + if err := json.Unmarshal([]byte(strings.TrimSpace(s)), &m); err != nil { + t.Fatalf("stdout is not JSON (%v): %q", err, s) + } + return m +} + +// Test 1: every missing required flag is named in one exit-6 error, and +// nothing is ever sent — the fake `service` and `customerProfileService` +// closures below both call t.Fatal if invoked, so this test cannot pass by +// accident even if RunE regressed to calling one of them before validating. +func TestBrandCreateMissingRequiredFlagsAggregate(t *testing.T) { + stubProfileService(t, nil) + _, _, err := runBrandCmd(t, nil, "brand", "create") + if err == nil { + t.Fatal("want an error for missing required flags") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + for _, want := range []string{"customer-profile-id", "display-name", "street", "city", + "state", "postal-code", "country-code-a3", "phone", "email", "brand-type"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error = %q, missing flag %q", err.Error(), want) + } + } +} + +// Test 2: PUBLIC_PROFIT additionally requires the four schema-optional +// fields. Supplying everything else but those four must still fail, exit 6, +// with zero requests made. +func TestBrandCreatePublicProfitRequiresFourExtraFlags(t *testing.T) { + stubProfileService(t, nil) + args := []string{ + "brand", "create", + "--customer-profile-id", "CP123", + "--brand-type", "PUBLIC_PROFIT", + "--display-name", "Acme Corp", + "--company-name", "Acme Corporation", + "--street", "123 Main St", + "--city", "Raleigh", + "--state", "NC", + "--postal-code", "27601", + "--country-code-a3", "USA", + "--phone", "+18885551234", + "--email", "ops@acme.com", + "--vertical", "RETAIL", + "--ein", "123456789", + "--ein-issuing-country-code-a3", "USA", + } + _, _, err := runBrandCmd(t, nil, args...) + if err == nil { + t.Fatal("want an error for missing PUBLIC_PROFIT-only flags") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + for _, want := range []string{"stock-symbol", "stock-exchange", "website", "business-contact-email"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error = %q, missing flag %q", err.Error(), want) + } + } +} + +// Test 3: a valid create posts the built body and prints the 202 receipt +// carrying bandwidthId. +func TestBrandCreatePostsBuiltBodyAndPrintsReceipt(t *testing.T) { + stubProfileService(t, stubProfileServer(t, http.StatusOK, `{"data":{"id":"CP123"}}`)) + srv, bodies := stubBrandCreateCapturing(t, "WNEW1") + + out, _, err := runBrandCmd(t, srv, validPrivateProfitArgs()...) + if err != nil { + t.Fatalf("brand create: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want exactly one POST, got %d", len(*bodies)) + } + var sent map[string]any + if err := json.Unmarshal([]byte((*bodies)[0]), &sent); err != nil { + t.Fatalf("request body is not JSON: %v", err) + } + if sent["customerProfileId"] != "CP123" || sent["displayName"] != "Acme Corp" || sent["brandType"] != "PRIVATE_PROFIT" { + t.Errorf("posted body = %v, missing built fields", sent) + } + + got := decodeStdout(t, out) + if got["bandwidthId"] != "WNEW1" { + t.Errorf("stdout = %v, want bandwidthId WNEW1", got) + } + if got["status"] != "accepted" { + t.Errorf("stdout = %v, want status accepted", got) + } +} + +// Test 4: --wait polls to VERIFIED and prints the final brand resource, not +// the synthetic receipt. +func TestBrandCreateWaitPollsToVerified(t *testing.T) { + stubProfileService(t, stubProfileServer(t, http.StatusOK, `{"data":{"id":"CP123"}}`)) + srv := stubBrandCreateThenPoll(t, "WNEW1", "VERIFIED") + + out, _, err := runBrandCmd(t, srv, validPrivateProfitArgs("--wait", "--timeout", "5")...) + if err != nil { + t.Fatalf("brand create --wait: %v", err) + } + got := decodeStdout(t, out) + if got["brandIdentityStatus"] != "VERIFIED" { + t.Errorf("stdout = %v, want the final VERIFIED resource", got) + } + if got["bandwidthId"] != "WNEW1" { + t.Errorf("stdout = %v, want bandwidthId WNEW1", got) + } +} + +// Test 5: --wait on a brand stuck at UNVERIFIED does NOT exit 4 — measured +// on production, UNVERIFIED is what a brand reports both while still +// registering and when genuinely rejected, and the two are indistinguishable +// from this field alone (see ClassifyBrandIdentity). So it polls to timeout +// (exit 5) instead of failing fast, and the timeout receipt carries the +// last-seen status plus a note pointing at 'brand get'/'brand history' — +// that receipt is now the only way a UNVERIFIED brand's outcome surfaces. +// --timeout 0 makes the deadline already past by the first poll, so this +// doesn't wait out the real 5s poll interval. +func TestBrandCreateWaitUnverifiedTimesOutWithLastSeenStatusAndNote(t *testing.T) { + stubProfileService(t, stubProfileServer(t, http.StatusOK, `{"data":{"id":"CP123"}}`)) + srv := stubBrandCreateThenPoll(t, "WNEW1", "UNVERIFIED") + + out, _, err := runBrandCmd(t, srv, validPrivateProfitArgs("--wait", "--timeout", "0")...) + if err == nil { + t.Fatal("want a timeout error for a brand stuck at UNVERIFIED") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitTimeout { + t.Errorf("exit code = %d, want %d (timeout, not conflict)", code, cmdutil.ExitTimeout) + } + got := decodeStdout(t, out) + if got["bandwidthId"] != "WNEW1" { + t.Fatalf("timeout must still print the ID, got %v", got) + } + if got["lastSeenStatus"] != "UNVERIFIED" { + t.Errorf("timeout receipt must carry the last-seen status, got %v", got) + } + if note, _ := got["note"].(string); !strings.Contains(note, "brand get") || !strings.Contains(note, "brand history") { + t.Errorf("timeout receipt note must point at both 'brand get' and 'brand history', got %v", got["note"]) + } +} + +// Test 6: a --wait timeout exits 5 and still emits a receipt carrying +// bandwidthId and a resume command — the whole point of awaitTerminal. +// --timeout 0 makes the deadline already past by the time the first poll +// returns, so this fails fast instead of waiting out the real poll interval. +func TestBrandCreateWaitTimeoutStillEmitsReceipt(t *testing.T) { + stubProfileService(t, stubProfileServer(t, http.StatusOK, `{"data":{"id":"CP123"}}`)) + srv := stubBrandCreateThenPoll(t, "WNEW1", "REGISTERING") + + out, _, err := runBrandCmd(t, srv, validPrivateProfitArgs("--wait", "--timeout", "0")...) + if err == nil { + t.Fatal("want a timeout error") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitTimeout { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitTimeout) + } + got := decodeStdout(t, out) + if got["bandwidthId"] != "WNEW1" { + t.Fatalf("timeout must still print the ID, got %v", got) + } + if got["resume"] != "band tendlc brand get WNEW1" { + t.Errorf("timeout receipt must carry a resume command, got %v", got) + } +} + +// Test 6b: --wait on a brand that settles ERROR still exits 4 (business +// failure) with remediation on stderr — ERROR is the one brandIdentityStatus +// that stayed StateFailure when UNVERIFIED moved to StatePending (see +// ClassifyBrandIdentity), so this is the only cmd-level coverage left of +// that path now that UNVERIFIED no longer takes it. +func TestBrandCreateWaitErrorExitsConflictWithRemediation(t *testing.T) { + stubProfileService(t, stubProfileServer(t, http.StatusOK, `{"data":{"id":"CP123"}}`)) + srv := stubBrandCreateThenPoll(t, "WNEW1", "ERROR") + + out, errOut, err := runBrandCmd(t, srv, validPrivateProfitArgs("--wait", "--timeout", "5")...) + if err == nil { + t.Fatal("want an error for a brand that settles ERROR") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitConflict { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitConflict) + } + got := decodeStdout(t, out) + if got["brandIdentityStatus"] != "ERROR" { + t.Errorf("stdout = %v, want the ERROR resource still printed", got) + } + if !strings.Contains(errOut, "refresh") { + t.Errorf("stderr should carry ERROR remediation, got %q", errOut) + } +} + +// Test 7: a pre-flight 404 on the customer profile stops the create at exit +// 3 and makes NO request to /brands. The create stub below fails the +// assertion (not the test directly, since t.Fatal from the server's +// goroutine is unsafe) by recording any request it receives. +func TestBrandCreatePreflight404BlocksCreate(t *testing.T) { + stubProfileService(t, stubProfileServer(t, http.StatusNotFound, `{"errors":[{"description":"customer profile not found"}]}`)) + srv, bodies := stubBrandCreateCapturing(t, "WSHOULD-NOT-EXIST") + + _, _, err := runBrandCmd(t, srv, validPrivateProfitArgs()...) + if err == nil { + t.Fatal("want an error when the customer profile pre-flight 404s") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitNotFound { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitNotFound) + } + if !strings.Contains(err.Error(), "CP123") { + t.Errorf("error = %q, want it to name the profile ID", err.Error()) + } + if len(*bodies) != 0 { + t.Errorf("want zero requests to /brands, got %d", len(*bodies)) + } +} + +// Test 8: a pre-flight 403 proceeds with the create anyway and warns on +// stderr — Customer Profiles Access is a role separate from Campaign +// Management, so a caller entitled to create brands must not be blocked by a +// check they lack permission to run. +func TestBrandCreatePreflight403ProceedsWithWarning(t *testing.T) { + stubProfileService(t, stubProfileServer(t, http.StatusForbidden, `{"errors":[{"description":"does not have access rights"}]}`)) + srv, bodies := stubBrandCreateCapturing(t, "WNEW1") + + out, errOut, err := runBrandCmd(t, srv, validPrivateProfitArgs()...) + if err != nil { + t.Fatalf("brand create: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want the create to proceed despite the 403 pre-flight, got %d requests", len(*bodies)) + } + got := decodeStdout(t, out) + if got["bandwidthId"] != "WNEW1" { + t.Errorf("stdout = %v, want the create receipt", got) + } + if !strings.Contains(errOut, "CP123") { + t.Errorf("stderr = %q, want a warning naming the profile ID", errOut) + } +} + +// Test 9: a stray positional is rejected by cobra.NoArgs before RunE ever +// runs, so no request is made to either seam. +func TestBrandCreateRejectsStrayPositional(t *testing.T) { + stubProfileService(t, nil) + srv, bodies := stubBrandCreateCapturing(t, "WNEW1") + + _, _, err := runBrandCmd(t, srv, append([]string{"brand", "create", "STRAY"}, + validPrivateProfitArgs()[2:]...)...) + if err == nil { + t.Fatal("want an argument error for a stray positional") + } + if len(*bodies) != 0 { + t.Errorf("want zero requests, got %d", len(*bodies)) + } +} + +// Test 10: refresh posts exactly {"brandId": "BGJR2BA"} — no other keys. A +// refresh body carrying any extra key turns it back into a create. +func TestBrandRefreshPostsExactBody(t *testing.T) { + srv, bodies := stubBrandCreateCapturing(t, "WET8JUY8H0") + + out, _, err := runBrandCmd(t, srv, "brand", "refresh", "BGJR2BA") + if err != nil { + t.Fatalf("brand refresh: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want exactly one POST, got %d", len(*bodies)) + } + var sent map[string]any + if err := json.Unmarshal([]byte((*bodies)[0]), &sent); err != nil { + t.Fatalf("request body is not JSON: %v", err) + } + if len(sent) != 1 || sent["brandId"] != "BGJR2BA" { + t.Errorf("posted body = %v, want exactly {\"brandId\":\"BGJR2BA\"}", sent) + } + got := decodeStdout(t, out) + if got["bandwidthId"] != "WET8JUY8H0" { + t.Errorf("stdout = %v, want the refresh receipt", got) + } +} + +// Test 11: refresh offers no --wait flag. It writes against a brand usually +// already in a terminal state, so "poll until settled" would return +// immediately and report success before the change applied. +func TestBrandRefreshHasNoWaitFlag(t *testing.T) { + if f := brandRefreshCmd.Flags().Lookup("wait"); f != nil { + t.Errorf("brandRefreshCmd has a --wait flag, want none: %+v", f) + } +} + +// Test 12: THE load-bearing regression test for buildAcceptedReceipt's +// no-bandwidthId fallback. This is the exact shape production sends for an +// orphan brand (see brandCreateCmd's Long): a single-key body, +// {"accounts":[...]}, with no bandwidthId anywhere. Before the fallback was +// switched from output.StdoutAuto to output.Stdout, --plain would run this +// through FlattenResponse, which unwraps ANY single-key map — silently +// dropping the "accounts" key and printing a bare array instead of the +// object it came from. +func TestBrandCreateNoBandwidthIDPrintsBodyWithKeyPreserved(t *testing.T) { + stubProfileService(t, stubProfileServer(t, http.StatusOK, `{"data":{"id":"CP123"}}`)) + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"data":{"accounts":[{"accountId":"9901287"}]}}`)) + }) + + out, _, err := runBrandCmd(t, srv, validPrivateProfitArgs("--plain")...) + if err == nil { + t.Fatal("want an error when the response carries no bandwidthId") + } + if !strings.Contains(err.Error(), "bandwidthId") { + t.Errorf("error = %q, want it to name the missing bandwidthId", err.Error()) + } + got := decodeStdout(t, out) + accounts, ok := got["accounts"] + if !ok { + t.Fatalf("stdout = %q, want the \"accounts\" key preserved, not unwrapped to a bare array", out) + } + if arr, ok := accounts.([]any); !ok || len(arr) != 1 { + t.Errorf("stdout accounts = %v, want a one-element array", accounts) + } +} diff --git a/cmd/tendlc/brand_delete.go b/cmd/tendlc/brand_delete.go new file mode 100644 index 0000000..4c91e69 --- /dev/null +++ b/cmd/tendlc/brand_delete.go @@ -0,0 +1,121 @@ +package tendlc + +import ( + "time" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" +) + +var ( + brandDeleteConfirm bool + brandDeleteWait bool + brandDeleteTimeout int +) + +func init() { + f := brandDeleteCmd.Flags() + f.BoolVar(&brandDeleteConfirm, "confirm", false, "Required. Confirms the brand should be deleted.") + f.BoolVar(&brandDeleteWait, "wait", false, "Block until the brand is gone") + f.IntVar(&brandDeleteTimeout, "timeout", 300, "Seconds to wait when --wait is set") + brandCmd.AddCommand(brandDeleteCmd) +} + +var brandDeleteCmd = &cobra.Command{ + Use: "delete ", + Short: "Permanently delete a 10DLC brand", + Long: `Permanently deletes a 10DLC brand. This cannot be undone. + +Deleting a brand deletes the brand in TCR for direct accounts, and requires +every campaign on the brand to be deactivated first — the API rejects the +delete otherwise. + +The endpoint docs say this cascades to delete the backing customer profile. +Measured on production: it does not. After deleting two test brands, both +backing profiles remained retrievable with softDeleted:false. The profile is +NOT deleted by this command — if you no longer need it, remove it separately +with 'band customer-profile delete '. A profile backs exactly one brand, +so an orphaned one left behind after a brand delete cannot be reused. + +Requires --confirm. The DELETE only ACCEPTS the request — production takes +roughly 40 seconds to actually remove the brand, so the receipt's "deleted" +field is false until that is confirmed. Without --wait, it stays false; +confirm manually with 'band tendlc brand get ' (a 404 means it is gone). +With --wait, this polls until the brand is gone — a 404 on the follow-up read +IS success here, the only place in this command set where that is true — and +only then does "deleted" flip to true. A --wait timeout (exit 5) prints the +same unconfirmed receipt as no-wait; it never claims deleted:true merely +because --wait gave up waiting.`, + Example: ` band tendlc brand delete BGJR2BA --confirm --plain + band tendlc brand delete BGJR2BA --confirm --wait --timeout 60 --plain`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := requireConfirm(brandDeleteConfirm, + "this permanently deletes brand "+args[0]+". It cannot be undone, it deletes the brand in TCR "+ + "for direct accounts, and it requires every campaign on the brand to be deactivated first. "+ + "It does NOT delete the associated customer profile (measured against production — the "+ + "documented cascade does not happen); remove that separately with "+ + "'band customer-profile delete ' if you no longer need it. Pass --confirm to proceed."); err != nil { + return err + } + + svc, err := service(cmd) + if err != nil { + return err + } + if err := svc.DeleteBrand(args[0]); err != nil { + return roleGateError(err, "Campaign Management") + } + + // The DELETE returning is only an accept, not a completion — measured + // against production, it takes roughly 40 seconds before the brand + // actually disappears from 'brand list'/'brand get'. So "deleted" + // starts false and stays false unless and until a 404 on the + // follow-up read actually proves it happened: not on the strength of + // the 202 alone, and not merely because --wait gave up waiting. + receipt := map[string]any{ + "id": args[0], + "deleted": false, + "status": "accepted", + "note": "delete accepted but not yet confirmed: production takes roughly 40s to actually " + + "remove the brand, so it may still appear in 'brand list' or 'brand get' until then. " + + "Confirm with 'band tendlc brand get " + args[0] + "' — a 404 means it is gone.", + } + + if !brandDeleteWait { + format, _ := cmdutil.OutputFlags(cmd) + return output.Stdout(format, receipt) + } + + // GoneIsDone: true — a 404 on the follow-up read means the delete has + // actually taken effect. This is the only poll in the whole tendlc + // command set that treats 404 as success rather than "not ready yet", + // which is exactly why the field is explicit on pollTarget instead of + // inferred from the noun. + // + // confirmedFetch wraps fetchBrand so the receipt only ever claims + // deleted:true at the exact moment a 404 proves it. awaitTerminal + // prints this same receipt map on BOTH the confirmed-success path and + // the --timeout path, so if the deadline arrives before this ever + // fires, deleted is still false and the note above is still there — + // the timeout receipt (exit 5) never contradicts its own exit code. + fetch := fetchBrand(svc, args[0]) + confirmedFetch := func() (map[string]any, bool, error) { + obj, found, ferr := fetch() + if ferr == nil && !found { + receipt["deleted"] = true + delete(receipt, "note") + } + return obj, found, ferr + } + + target := pollTarget{ + Noun: "brand", + Fetch: confirmedFetch, + GoneIsDone: true, + } + return awaitTerminal(cmd, target, receipt, time.Duration(brandDeleteTimeout)*time.Second, brandCreatePollInterval) + }, +} diff --git a/cmd/tendlc/brand_delete_test.go b/cmd/tendlc/brand_delete_test.go new file mode 100644 index 0000000..deeef0b --- /dev/null +++ b/cmd/tendlc/brand_delete_test.go @@ -0,0 +1,169 @@ +package tendlc + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// stubBrandDeleteServer answers DELETE /brands/{id} with deleteStatus (and an +// empty body, matching a 204), and any subsequent GET (used by --wait's +// follow-up poll) with 404 — the delete having actually taken effect. It +// records every request method it sees. +func stubBrandDeleteServer(t *testing.T, deleteStatus int) (*httptest.Server, *[]string) { + t.Helper() + var methods []string + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + methods = append(methods, r.Method) + if r.Method == http.MethodDelete { + w.WriteHeader(deleteStatus) + return + } + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"errors":[{"description":"brand not found"}]}`)) + }) + return srv, &methods +} + +// stubBrandDeleteStillExistsServer answers DELETE with deleteStatus, but +// every subsequent GET with 200 and a body proving the brand is still there +// — matching production's real ~40s propagation delay, where the brand +// outlives the DELETE response for a while. Used to exercise the --wait +// timeout path, where the follow-up read never 404s before the deadline. +func stubBrandDeleteStillExistsServer(t *testing.T, deleteStatus int) (*httptest.Server, *[]string) { + t.Helper() + var methods []string + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + methods = append(methods, r.Method) + if r.Method == http.MethodDelete { + w.WriteHeader(deleteStatus) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"data":{"brandId":"BGJR2BA","bandwidthId":"WET8JUY8H0"}}`)) + }) + return srv, &methods +} + +// Test 10: delete without --confirm exits 6 and makes ZERO HTTP requests — +// runBrandCmd's `service` seam Fatals if it is ever invoked with a nil +// server, so this fails loudly (not silently) if the confirm gate ever moves +// after the service/DELETE call. +func TestBrandDeleteWithoutConfirmMakesNoRequests(t *testing.T) { + _, _, err := runBrandCmd(t, nil, "brand", "delete", "BGJR2BA") + if err == nil { + t.Fatal("want an error when --confirm is missing") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + if !strings.Contains(err.Error(), "cannot be undone") { + t.Errorf("error = %q, want it to say the delete cannot be undone", err.Error()) + } + // The endpoint docs claim the delete cascades to the customer profile. + // Measured against production: it does not — both test brands' backing + // profiles remained retrievable (softDeleted:false) after the brand was + // deleted. The refusal message must not repeat that false claim, and + // must instead tell the caller to remove the profile separately. + if strings.Contains(err.Error(), "also deletes") || strings.Contains(err.Error(), "AND its associated") { + t.Errorf("error = %q, must not claim the delete cascades to the customer profile", err.Error()) + } + if !strings.Contains(err.Error(), "does NOT delete the associated customer profile") { + t.Errorf("error = %q, want it to state the profile is NOT deleted", err.Error()) + } + if !strings.Contains(err.Error(), "customer-profile delete") { + t.Errorf("error = %q, want it to point at 'band customer-profile delete' to remove the profile separately", err.Error()) + } +} + +// Test 11: delete --confirm without --wait issues the DELETE and prints an +// honest, unconfirmed receipt. Production takes ~40s to actually remove the +// brand, so "deleted" must be false here — it has not been confirmed, only +// accepted — and the receipt must point the caller at how to confirm it. +func TestBrandDeleteWithConfirmIssuesDeleteAndPrintsReceipt(t *testing.T) { + srv, methods := stubBrandDeleteServer(t, http.StatusNoContent) + + out, _, err := runBrandCmd(t, srv, "brand", "delete", "BGJR2BA", "--confirm", "--plain") + if err != nil { + t.Fatalf("brand delete --confirm: %v", err) + } + if len(*methods) != 1 || (*methods)[0] != http.MethodDelete { + t.Fatalf("want exactly one DELETE, got %v", *methods) + } + got := decodeStdout(t, out) + if got["id"] != "BGJR2BA" { + t.Errorf("stdout = %v, want id BGJR2BA", got) + } + if got["deleted"] != false { + t.Errorf("stdout = %v, want deleted false — accepted is not confirmed, and there was no --wait to confirm it", got) + } + if got["status"] != "accepted" { + t.Errorf("stdout = %v, want status accepted", got) + } + note, _ := got["note"].(string) + if !strings.Contains(note, "brand get") { + t.Errorf("note = %q, want it to point at how to confirm completion", note) + } +} + +// Test 12: delete --confirm --wait, where the follow-up read 404s, exits 0 +// and prints deleted:true ONLY now that the 404 actually confirmed it. +// GoneIsDone is the one place in this command set where a 404 means success +// rather than "not ready yet". +func TestBrandDeleteWaitTreats404AsSuccess(t *testing.T) { + srv, methods := stubBrandDeleteServer(t, http.StatusNoContent) + + out, _, err := runBrandCmd(t, srv, "brand", "delete", "BGJR2BA", "--confirm", "--wait", "--timeout", "5", "--plain") + if err != nil { + t.Fatalf("brand delete --confirm --wait: %v", err) + } + if len(*methods) < 2 || (*methods)[0] != http.MethodDelete || (*methods)[1] != http.MethodGet { + t.Fatalf("want a DELETE then at least one GET, got %v", *methods) + } + got := decodeStdout(t, out) + if got["id"] != "BGJR2BA" { + t.Errorf("stdout = %v, want id BGJR2BA", got) + } + if got["deleted"] != true { + t.Errorf("stdout = %v, want deleted true — the follow-up 404 confirmed it", got) + } + if _, present := got["note"]; present { + t.Errorf("stdout = %v, want no unconfirmed-delete note once the 404 confirmed completion", got) + } +} + +// Test 13: delete --confirm --wait --timeout 0, where the follow-up read +// never 404s before the deadline (matching production's real ~40s +// propagation delay), exits 5 (timeout) and must NOT claim deleted:true — a +// receipt that contradicts its own exit code is the exact bug this guards. +func TestBrandDeleteWaitTimeoutKeepsReceiptHonest(t *testing.T) { + srv, methods := stubBrandDeleteStillExistsServer(t, http.StatusNoContent) + + out, _, err := runBrandCmd(t, srv, "brand", "delete", "BGJR2BA", "--confirm", "--wait", "--timeout", "0", "--plain") + if err == nil { + t.Fatal("want a timeout error") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitTimeout { + t.Errorf("exit code = %d, want %d (timeout)", code, cmdutil.ExitTimeout) + } + if len(*methods) < 2 || (*methods)[0] != http.MethodDelete || (*methods)[1] != http.MethodGet { + t.Fatalf("want a DELETE then at least one GET, got %v", *methods) + } + got := decodeStdout(t, out) + if got["id"] != "BGJR2BA" { + t.Errorf("stdout = %v, want id BGJR2BA", got) + } + if got["deleted"] != false { + t.Errorf("stdout = %v, want deleted false — the timeout means completion was never confirmed, and exit 5 must not be paired with deleted:true", got) + } + if got["status"] != "accepted" { + t.Errorf("stdout = %v, want status accepted", got) + } + note, _ := got["note"].(string) + if !strings.Contains(note, "brand get") { + t.Errorf("note = %q, want it to point at how to confirm completion", note) + } +} diff --git a/cmd/tendlc/brand_get.go b/cmd/tendlc/brand_get.go new file mode 100644 index 0000000..fc75c4f --- /dev/null +++ b/cmd/tendlc/brand_get.go @@ -0,0 +1,40 @@ +package tendlc + +import ( + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" +) + +func init() { brandCmd.AddCommand(brandGetCmd) } + +var brandGetCmd = &cobra.Command{ + Use: "get ", + Short: "Get a 10DLC brand", + Long: `Shows one brand, including every field the summary projection in +'brand list' omits. + +Brands have two IDs: bandwidthId exists immediately, while brandId is assigned +by TCR and is null until registration completes. Either identifier works +here — pass whichever one you have.`, + Example: ` band tendlc brand get BGJR2BA --plain + band tendlc brand get WET8JUY8H0 --plain`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + svc, err := service(cmd) + if err != nil { + return err + } + env, err := svc.GetBrand(args[0]) + if err != nil { + return roleGateError(err, "Campaign Management") + } + obj, err := env.Object() + if err != nil { + return err + } + format, plain := cmdutil.OutputFlags(cmd) + return output.StdoutAuto(format, plain, obj) + }, +} diff --git a/cmd/tendlc/brand_history.go b/cmd/tendlc/brand_history.go new file mode 100644 index 0000000..211e722 --- /dev/null +++ b/cmd/tendlc/brand_history.go @@ -0,0 +1,76 @@ +package tendlc + +import ( + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" +) + +var ( + brandHistoryLimit int + brandHistoryOffset int + brandHistoryAll bool +) + +func init() { + f := brandHistoryCmd.Flags() + f.IntVar(&brandHistoryLimit, "limit", 50, "Page size") + f.IntVar(&brandHistoryOffset, "offset", 0, "Pagination offset") + f.BoolVar(&brandHistoryAll, "all", false, "Fetch every page (cannot be combined with --offset)") + brandCmd.AddCommand(brandHistoryCmd) +} + +var brandHistoryCmd = &cobra.Command{ + Use: "history ", + Short: "Show a brand's activity log", + Long: `Lists a brand's activity log: free-text {createdDate, message} entries, +newest first. Always returns an array. + +Unlike customer profiles, brands have no versioned snapshots and no +per-version fetch — this is the only history view for a brand. + +Either the TCR brandId or the Bandwidth bandwidthId works as the positional.`, + Example: ` band tendlc brand history BGJR2BA --plain + band tendlc brand history BGJR2BA --all --plain`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if brandHistoryAll && cmd.Flags().Changed("offset") { + return cmdutil.NewFlagError("--all fetches every page, so it cannot be combined with --offset") + } + svc, err := service(cmd) + if err != nil { + return err + } + format, plain := cmdutil.OutputFlags(cmd) + + if !brandHistoryAll { + env, err := svc.BrandHistory(args[0], brandHistoryLimit, brandHistoryOffset) + if err != nil { + return roleGateError(err, "Campaign Management") + } + items, err := env.List() + if err != nil { + return err + } + warnIfTruncated(cmd, env, brandHistoryOffset, len(items), "history entries") + return output.StdoutPlainList(format, plain, items) + } + + var all []any + err = api.ForEachPage(func(limit, offset int) (*api.Envelope, error) { + return svc.BrandHistory(args[0], limit, offset) + }, brandHistoryLimit, func(batch []any) error { + all = append(all, batch...) + return nil + }) + if err != nil { + return roleGateError(err, "Campaign Management") + } + if all == nil { + all = []any{} + } + return output.StdoutPlainList(format, plain, all) + }, +} diff --git a/cmd/tendlc/brand_identity.go b/cmd/tendlc/brand_identity.go new file mode 100644 index 0000000..e6ff7a0 --- /dev/null +++ b/cmd/tendlc/brand_identity.go @@ -0,0 +1,89 @@ +package tendlc + +import ( + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" +) + +var brandReverifyConfirm bool + +func init() { + f := brandReverifyCmd.Flags() + f.BoolVar(&brandReverifyConfirm, "confirm", false, "Required. Confirms the $4 reverification fee.") + brandCmd.AddCommand(brandReverifyCmd) + brandCmd.AddCommand(brandResend2FACmd) +} + +var brandReverifyCmd = &cobra.Command{ + Use: "reverify ", + Short: "Resubmit a brand for identity verification", + Long: `Resubmits a brand for identity verification. + +This incurs a $4 fee and resets brandIdentityStatus toward re-registration. +Production documents this as REGISTERING, but the field reads back as +UNVERIFIED until TCR responds — see 'band tendlc brand get '. The +endpoint returns 204 with no body — there is no ID or resource to poll, so +there is no --wait here. The brand's own status is the signal. + +Requires --confirm.`, + Example: ` band tendlc brand reverify BGJR2BA --confirm --plain`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := requireConfirm(brandReverifyConfirm, + "reverifying brand "+args[0]+" incurs a $4 fee and resets brandIdentityStatus toward "+ + "re-registration; it reads back as UNVERIFIED until TCR responds. Pass --confirm to proceed."); err != nil { + return err + } + + svc, err := service(cmd) + if err != nil { + return err + } + if err := svc.ReverifyBrand(args[0]); err != nil { + return roleGateError(err, "Campaign Management") + } + + receipt := map[string]any{ + "id": args[0], + "reverificationRequested": true, + "status": "accepted", + "check": "band tendlc brand get " + args[0], + } + format, _ := cmdutil.OutputFlags(cmd) + return output.Stdout(format, receipt) + }, +} + +var brandResend2FACmd = &cobra.Command{ + Use: "resend-2fa ", + Short: "Re-send the Business Authentication 2FA email", + Long: `Re-sends the Business Authentication (Auth+) 2FA email to a brand's +business contact. + +This applies to Business Authentication on PUBLIC_PROFIT brands. The +business contact has 30 days from the original request to complete +verification before the brand goes UNVERIFIED. + +Re-sending an email is neither destructive nor billable, so unlike most +writes in this command set, this does not require --confirm.`, + Example: ` band tendlc brand resend-2fa BGJR2BA --plain`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + svc, err := service(cmd) + if err != nil { + return err + } + if err := svc.Resend2FA(args[0]); err != nil { + return roleGateError(err, "Campaign Management") + } + + receipt := map[string]any{ + "id": args[0], + "emailResent": true, + } + format, _ := cmdutil.OutputFlags(cmd) + return output.Stdout(format, receipt) + }, +} diff --git a/cmd/tendlc/brand_identity_test.go b/cmd/tendlc/brand_identity_test.go new file mode 100644 index 0000000..3250f7b --- /dev/null +++ b/cmd/tendlc/brand_identity_test.go @@ -0,0 +1,136 @@ +package tendlc + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// stubBrandIdentityServer answers any request with a 204 (matching the real +// API's empty-body response for both /identity/reverify and +// /identity/resend2faEmail) and records the path of every request it sees. +func stubBrandIdentityServer(t *testing.T) (*httptest.Server, *[]string) { + t.Helper() + var paths []string + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + w.WriteHeader(http.StatusNoContent) + }) + return srv, &paths +} + +// Test: reverify without --confirm exits 6 and makes ZERO HTTP requests — +// runBrandCmd's `service` seam Fatals if it is ever invoked with a nil +// server, so this fails loudly if the confirm gate ever moves after the +// service/POST call. +func TestBrandReverifyWithoutConfirmMakesNoRequests(t *testing.T) { + _, _, err := runBrandCmd(t, nil, "brand", "reverify", "BGJR2BA") + if err == nil { + t.Fatal("want an error when --confirm is missing") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + if !strings.Contains(err.Error(), "$4") { + t.Errorf("error = %q, want it to name the $4 fee", err.Error()) + } +} + +// TestBrandReverifyWithConfirmPostsToIdentityReverify locks in the exact +// path so a copy-paste between reverify and resend-2fa (they differ only in +// the URL suffix) cannot go unnoticed. +func TestBrandReverifyWithConfirmPostsToIdentityReverify(t *testing.T) { + srv, paths := stubBrandIdentityServer(t) + + out, _, err := runBrandCmd(t, srv, "brand", "reverify", "BGJR2BA", "--confirm", "--plain") + if err != nil { + t.Fatalf("brand reverify --confirm: %v", err) + } + if len(*paths) != 1 || !strings.HasSuffix((*paths)[0], "/brands/BGJR2BA/identity/reverify") { + t.Fatalf("paths = %v, want exactly one POST to .../identity/reverify", *paths) + } + got := decodeStdout(t, out) + if got["id"] != "BGJR2BA" { + t.Errorf("stdout = %v, want id BGJR2BA", got) + } + if got["reverificationRequested"] != true { + t.Errorf("stdout = %v, want reverificationRequested true", got) + } + if got["status"] != "accepted" { + t.Errorf("stdout = %v, want status accepted", got) + } + if got["check"] != "band tendlc brand get BGJR2BA" { + t.Errorf("stdout = %v, want check pointing at brand get", got) + } +} + +// TestBrandResend2FANeedsNoConfirm asserts the opposite gate from reverify: +// re-sending an email is free and non-destructive, so no --confirm flag +// should be required, and the command should not even define one. +func TestBrandResend2FANeedsNoConfirm(t *testing.T) { + srv, paths := stubBrandIdentityServer(t) + + out, _, err := runBrandCmd(t, srv, "brand", "resend-2fa", "BGJR2BA", "--plain") + if err != nil { + t.Fatalf("brand resend-2fa: %v", err) + } + if len(*paths) != 1 || !strings.HasSuffix((*paths)[0], "/brands/BGJR2BA/identity/resend2faEmail") { + t.Fatalf("paths = %v, want exactly one POST to .../identity/resend2faEmail", *paths) + } + got := decodeStdout(t, out) + if got["id"] != "BGJR2BA" { + t.Errorf("stdout = %v, want id BGJR2BA", got) + } + if got["emailResent"] != true { + t.Errorf("stdout = %v, want emailResent true", got) + } +} + +// TestBrandIdentityCommandsRejectStrayPositionals mirrors +// TestBrandCommandsRejectStrayPositionals: both reverify and resend-2fa take +// exactly one positional, so a missing or extra one must be rejected before +// any request is made. +func TestBrandIdentityCommandsRejectStrayPositionals(t *testing.T) { + cases := [][]string{ + {"brand", "reverify"}, + {"brand", "reverify", "B1", "STRAY", "--confirm"}, + {"brand", "resend-2fa"}, + {"brand", "resend-2fa", "B1", "STRAY"}, + } + for _, args := range cases { + t.Run(strings.Join(args, " "), func(t *testing.T) { + if _, _, err := runBrandCmd(t, nil, args...); err == nil { + t.Fatal("want an argument error") + } + }) + } +} + +// TestBrandIdentity403MapsToExitFour exercises roleGateError on both +// commands: a 403 from either endpoint must map to exit 4, not the generic +// exit 1 a bare error would produce. +func TestBrandIdentity403MapsToExitFour(t *testing.T) { + body := `{"errors":[{"description":"does not have access rights"}]}` + + cases := []struct { + name string + args []string + }{ + {"reverify", []string{"brand", "reverify", "BGJR2BA", "--confirm"}}, + {"resend-2fa", []string{"brand", "resend-2fa", "BGJR2BA"}}, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + _, _, err := runBrandCmd(t, stubBrandErr(t, 403, body), tt.args...) + if err == nil { + t.Fatal("want an error on 403") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitConflict { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitConflict) + } + }) + } +} diff --git a/cmd/tendlc/brand_list.go b/cmd/tendlc/brand_list.go new file mode 100644 index 0000000..522cbf2 --- /dev/null +++ b/cmd/tendlc/brand_list.go @@ -0,0 +1,190 @@ +package tendlc + +import ( + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" +) + +var ( + brandListLimit int + brandListOffset int + brandListAll bool + brandListCustomerProfileIDContains string + brandListBrandIDContains string + brandListIdentityStatus string + brandListBrandType string + brandListCompanyNameContains string + brandListDisplayNameContains string +) + +func init() { + f := brandListCmd.Flags() + f.IntVar(&brandListLimit, "limit", 50, "Page size") + f.IntVar(&brandListOffset, "offset", 0, "Pagination offset") + f.BoolVar(&brandListAll, "all", false, "Fetch every page (cannot be combined with --offset)") + f.StringVar(&brandListCustomerProfileIDContains, "customer-profile-id-contains", "", + "Filter to brands whose customer profile ID contains this substring (e.g. 9900000 also matches 9900000-1); the API has no exact-match operator for this field, see 'brand list --help' notes") + f.StringVar(&brandListBrandIDContains, "brand-id-contains", "", + "Filter by TCR brand ID substring (e.g. BEXMPL1 also matches BEXMPL12); the API has no exact-match operator for this field, use 'brand get ' for a single brand") + f.StringVar(&brandListIdentityStatus, "identity-status", "", + "Filter by brandIdentityStatus, exact match (VERIFIED, VETTED_VERIFIED, UNVERIFIED, ERROR; REGISTERING is documented but never observed on the read path, so filtering on it returns nothing)") + f.StringVar(&brandListBrandType, "brand-type", "", "Filter by brand type, exact match") + f.StringVar(&brandListCompanyNameContains, "company-name-contains", "", "Filter by legal company name substring") + f.StringVar(&brandListDisplayNameContains, "display-name-contains", "", "Filter by display name substring") + brandCmd.AddCommand(brandListCmd) +} + +var brandListCmd = &cobra.Command{ + Use: "list", + Short: "List 10DLC brands", + Long: `Lists brands on the account. + +This is a summary projection — 12 keys per brand, versus the 46 keys 'brand +get' returns. A field that is missing here may simply not be part of the +listing projection, not null on the brand; use 'brand get ' for the full +resource. + +There is no --bandwidth-id filter: measured against production, +bandwidthId[eq] is accepted and silently ignored, returning every brand +rather than filtering. Use 'brand get ' to fetch one directly.`, + Example: ` band tendlc brand list --plain + band tendlc brand list --all --plain + band tendlc brand list --identity-status VERIFIED --plain`, + // No positional args: list takes only flags. + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + // Detected via Changed so that an explicit --offset 0 also conflicts. + if brandListAll && cmd.Flags().Changed("offset") { + return cmdutil.NewFlagError("--all fetches every page, so it cannot be combined with --offset") + } + svc, err := service(cmd) + if err != nil { + return err + } + + // Measured against production (test account, 10 brands): GET /brands + // accepts the `eq` operator on every field below and silently drops + // it -- brandId[eq]=X returns all 10 brands, not the one with that + // ID. `contains` is the only operator that actually filters there: + // + // brandId[contains] -> 1 of 10 (substring match) + // customerProfileId[contains] -> 1 of 10 (substring match) + // brandIdentityStatus[contains] -> 3 of 10 ("VETTED" matched the 3 VETTED_VERIFIED brands) + // brandType[contains] -> 5 of 10 + // companyName[contains] -> 9 of 10 (already implemented this way) + // displayName[contains] -> 4 of 10 (already implemented this way) + // + // Do not "correct" these back to OpEq: eq looks more right and is + // silently wrong -- it returns a 200 with every brand on the account + // and no error to signal the filter did nothing. This is the same + // failure already known for bandwidthId (see the "no --bandwidth-id + // filter" note above); it was wrongly assumed to be specific to that + // field. + // + // contains is a substring match, not equality, so + // --brand-id-contains and --customer-profile-id-contains are named + // for what they actually do (BEXMPL1 also matches BEXMPL12) rather + // than implying an exact-match --brand-id/--customer-profile-id that + // the API cannot perform. + // + // --identity-status and --brand-type are closed enums, so a + // contains match has a sharper trap than an arbitrary substring: + // brandIdentityStatus[contains]=VERIFIED matches VERIFIED, + // VETTED_VERIFIED, *and* UNVERIFIED -- the most obvious query a + // caller would type ("give me the VERIFIED ones") would silently + // include the exact opposite status. Because the two fields are + // closed enums, exactness is well-defined and cheap, so the results + // are narrowed server-side with contains and then filtered + // client-side (filterExactField, below) for an exact match, so the + // flag keeps its exact-match meaning even though the wire request + // underneath is contains. + var filters []api.Filter + if brandListCustomerProfileIDContains != "" { + filters = append(filters, api.Filter{Field: "customerProfileId", Op: api.OpContains, Value: brandListCustomerProfileIDContains}) + } + if brandListBrandIDContains != "" { + filters = append(filters, api.Filter{Field: "brandId", Op: api.OpContains, Value: brandListBrandIDContains}) + } + if brandListIdentityStatus != "" { + filters = append(filters, api.Filter{Field: "brandIdentityStatus", Op: api.OpContains, Value: brandListIdentityStatus}) + } + if brandListBrandType != "" { + filters = append(filters, api.Filter{Field: "brandType", Op: api.OpContains, Value: brandListBrandType}) + } + if brandListCompanyNameContains != "" { + filters = append(filters, api.Filter{Field: "companyName", Op: api.OpContains, Value: brandListCompanyNameContains}) + } + if brandListDisplayNameContains != "" { + filters = append(filters, api.Filter{Field: "displayName", Op: api.OpContains, Value: brandListDisplayNameContains}) + } + + format, plain := cmdutil.OutputFlags(cmd) + + if !brandListAll { + env, err := svc.ListBrands(brandListLimit, brandListOffset, filters) + if err != nil { + return roleGateError(err, "Campaign Management") + } + items, err := env.List() + if err != nil { + return err + } + items = filterExactField(items, "brandIdentityStatus", brandListIdentityStatus) + items = filterExactField(items, "brandType", brandListBrandType) + // warnIfTruncated runs after the exact-match narrowing so "showing + // X" reflects what is actually printed. Its "of Y" total, however, + // still comes straight from the server and reflects the broader + // contains match, not the exact one -- if --identity-status or + // --brand-type is set, Y can overstate how many brands remain on + // later pages that exactly match. --all does not have this gap: + // it walks every page before narrowing, so its count is exact. + warnIfTruncated(cmd, env, brandListOffset, len(items), "brands") + return output.StdoutPlainList(format, plain, items) + } + + var all []any + err = api.ForEachPage(func(limit, offset int) (*api.Envelope, error) { + return svc.ListBrands(limit, offset, filters) + }, brandListLimit, func(batch []any) error { + all = append(all, batch...) + return nil + }) + if err != nil { + return roleGateError(err, "Campaign Management") + } + if all == nil { + all = []any{} + } + all = filterExactField(all, "brandIdentityStatus", brandListIdentityStatus) + all = filterExactField(all, "brandType", brandListBrandType) + return output.StdoutPlainList(format, plain, all) + }, +} + +// filterExactField keeps only the items whose field is exactly value, +// dropping the rest. value == "" is a no-op (the caller didn't set that +// flag). It exists because the API only offers `contains` on +// brandIdentityStatus/brandType (see the RunE comment above), and contains +// is not precise enough for those two closed enums: VERIFIED[contains] also +// matches VETTED_VERIFIED and UNVERIFIED. Items whose field is missing or +// not a string are dropped rather than kept, since a promised exact match +// should never silently include something that couldn't be compared. +func filterExactField(items []any, field, value string) []any { + if value == "" { + return items + } + out := items[:0:0] + for _, item := range items { + m, ok := item.(map[string]any) + if !ok { + continue + } + if s, ok := m[field].(string); ok && s == value { + out = append(out, item) + } + } + return out +} diff --git a/cmd/tendlc/brand_test.go b/cmd/tendlc/brand_test.go new file mode 100644 index 0000000..a798bb1 --- /dev/null +++ b/cmd/tendlc/brand_test.go @@ -0,0 +1,402 @@ +package tendlc + +import ( + "bytes" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + tendlcsvc "github.com/Bandwidth/cli/internal/tendlc" + "github.com/Bandwidth/cli/internal/testutil" +) + +// testRoot is a single fake root, built once via testutil.NewTestRoot and +// reused by every runBrandCmd call in this package, with Cmd (the `tendlc` +// parent) as its only child. --format/--plain/--account-id/--environment +// live here, exactly as in production. +// +// It is deliberately NOT rebuilt per call — see resetFlags below and +// cmd/customerprofile/customerprofile_test.go's testRoot, which this mirrors: +// cobra caches a command's merged ancestor flags (parentsPflags) the first +// time it parses and never refreshes that cache for a different root object +// later, so a fresh-root-per-call harness would silently pin every +// package-level command (brandCmd, brandListCmd, ...) to the FIRST test's +// root. +var testRoot = testutil.NewTestRoot(Cmd) + +// resetFlags restores every flag on cmd and all its descendants to its +// default value and clears the Changed bit, so state set by one test (e.g. +// --offset from a pagination test) cannot leak into the next. +func resetFlags(cmd *cobra.Command) { + reset := func(f *pflag.Flag) { + _ = f.Value.Set(f.DefValue) + f.Changed = false + } + cmd.Flags().VisitAll(reset) + cmd.PersistentFlags().VisitAll(reset) + for _, sub := range cmd.Commands() { + resetFlags(sub) + } +} + +// newBrandStub starts an httptest.Server running handler and registers it to +// close on test cleanup. +func newBrandStub(t *testing.T, handler http.HandlerFunc) *httptest.Server { + t.Helper() + srv := httptest.NewServer(handler) + t.Cleanup(srv.Close) + return srv +} + +// stubBrandList answers any request with one brand on a single, non-truncated +// page. Good enough for tests that just need brand list to succeed. +func stubBrandList(t *testing.T) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[{"bandwidthId":"WET8JUY8H0","brandId":"BGJR2BA"}],` + + `"page":{"pageNumber":0,"pageSize":50,"totalElements":1,"totalPages":1}}`)) + }) +} + +// stubBrandListCapturing records the raw query string of every request to +// /brands so a test can assert on the deepObject filter encoding. +func stubBrandListCapturing(t *testing.T) (*httptest.Server, *[]string) { + var queries []string + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + queries = append(queries, r.URL.RawQuery) + _, _ = w.Write([]byte(`{"data":[],"page":{"pageNumber":0,"pageSize":50,"totalElements":0,"totalPages":0}}`)) + }) + return srv, &queries +} + +// stubBrandListTruncated answers with a page that reports more records exist +// than were returned, so warnIfTruncated fires. +func stubBrandListTruncated(t *testing.T) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[{"bandwidthId":"W1"}],` + + `"page":{"pageNumber":0,"pageSize":1,"totalElements":5,"totalPages":5}}`)) + }) +} + +// stubBrandGetCapturing records the request path of every request so a test +// can assert the positional ID is passed through unchanged. +func stubBrandGetCapturing(t *testing.T) (*httptest.Server, *[]string) { + var paths []string + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + _, _ = w.Write([]byte(`{"data":{"bandwidthId":"WET8JUY8H0","brandId":"BGJR2BA"}}`)) + }) + return srv, &paths +} + +// stubBrandHistory answers /brands/.../history with one free-text entry. +func stubBrandHistory(t *testing.T) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[{"createdDate":"2026-01-01T00:00:00Z",` + + `"message":"Successfully updated brand"}],` + + `"page":{"pageNumber":0,"pageSize":50,"totalElements":1,"totalPages":1}}`)) + }) +} + +// stubBrandListTwoPages serves genuinely different items on page one +// (offset 0) versus page two (offset 1), keyed off the request's offset +// query param — the same shape as +// cmd/customerprofile/customerprofile_test.go's TestListAllWalksEveryPage +// stub. totalElements=2 with pageSize=1 forces api.ForEachPage to fetch both +// pages under --all --limit 1. +func stubBrandListTwoPages(t *testing.T) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + offset := r.URL.Query().Get("offset") + if offset == "" || offset == "0" { + _, _ = w.Write([]byte(`{"data":[{"bandwidthId":"PAGE-A"}],` + + `"page":{"pageNumber":0,"pageSize":1,"totalElements":2,"totalPages":2}}`)) + return + } + _, _ = w.Write([]byte(`{"data":[{"bandwidthId":"PAGE-B"}],` + + `"page":{"pageNumber":1,"pageSize":1,"totalElements":2,"totalPages":2}}`)) + }) +} + +// stubBrandHistoryTwoPages is stubBrandListTwoPages's twin for the history +// endpoint: distinct messages per page, keyed off the offset query param. +func stubBrandHistoryTwoPages(t *testing.T) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + offset := r.URL.Query().Get("offset") + if offset == "" || offset == "0" { + _, _ = w.Write([]byte(`{"data":[{"createdDate":"2026-01-02T00:00:00Z",` + + `"message":"page one message"}],` + + `"page":{"pageNumber":0,"pageSize":1,"totalElements":2,"totalPages":2}}`)) + return + } + _, _ = w.Write([]byte(`{"data":[{"createdDate":"2026-01-01T00:00:00Z",` + + `"message":"page two message"}],` + + `"page":{"pageNumber":1,"pageSize":1,"totalElements":2,"totalPages":2}}`)) + }) +} + +// stubBrandErr answers every request with the given status code and body, +// for exercising roleGateError and other failure paths. +func stubBrandErr(t *testing.T, code int, body string) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(code) + _, _ = w.Write([]byte(body)) + }) +} + +// runBrandCmd executes one `band tendlc ` invocation against srv, +// resetting flag state and swapping the `service` seam beforehand. It +// returns stdout and stderr separately: stdout must stay parseable data, +// while truncation warnings and similar operator messages go to stderr via +// cmd.PrintErrf — see internal/output's direct os.Stdout write, which is why +// testutil.CaptureStdout is used here instead of cmd.SetOut. +func runBrandCmd(t *testing.T, srv *httptest.Server, args ...string) (stdout, stderr string, err error) { + t.Helper() + + resetFlags(testRoot) + + orig := service + service = func(cmd *cobra.Command) (*tendlcsvc.Service, error) { + if srv == nil { + t.Fatal("command made a request but no stub server was provided") + } + return tendlcsvc.NewService(api.NewClientNoAuth(srv.URL), "9901287"), nil + } + t.Cleanup(func() { service = orig }) + + testRoot.SetArgs(append([]string{Cmd.Name()}, args...)) + testRoot.SetOut(io.Discard) + var errBuf bytes.Buffer + testRoot.SetErr(&errBuf) + t.Cleanup(func() { testRoot.SetErr(io.Discard) }) + + out := testutil.CaptureStdout(t, func() { err = testRoot.Execute() }) + return out, errBuf.String(), err +} + +func TestBrandListRejectsAllWithOffset(t *testing.T) { + _, _, err := runBrandCmd(t, stubBrandList(t), "brand", "list", "--all", "--offset", "0") + if err == nil { + t.Fatal("want an error combining --all with --offset") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } +} + +func TestBrandListEncodesFiltersAsDeepObject(t *testing.T) { + srv, queries := stubBrandListCapturing(t) + if _, _, err := runBrandCmd(t, srv, "brand", "list", + "--identity-status", "VERIFIED", "--company-name-contains", "Acme"); err != nil { + t.Fatalf("brand list: %v", err) + } + q := (*queries)[0] + // eq is silently ignored by the server on every field tested (see the + // measurement recorded in brand_list.go), so every filter -- including + // the enum-valued ones -- goes over the wire as contains, not eq. + if !strings.Contains(q, "brandIdentityStatus%5Bcontains%5D=VERIFIED") { + t.Errorf("query %q missing deepObject contains filter for brandIdentityStatus", q) + } + if !strings.Contains(q, "companyName%5Bcontains%5D=Acme") { + t.Errorf("query %q missing deepObject contains filter", q) + } + if strings.Contains(q, "%5Beq%5D") { + t.Errorf("query %q uses eq, which the API silently ignores", q) + } +} + +// TestBrandListAllFiltersSendContains covers the four flags that switched +// from eq to contains: brandId, customerProfileId, brandType, and +// brandIdentityStatus (identity status is exercised above). eq is accepted +// and silently dropped by the server on all of them -- see the measurement +// in brand_list.go -- so this locks in that none of them regress back to +// eq, which "looks" more correct but returns every brand on the account. +func TestBrandListAllFiltersSendContains(t *testing.T) { + srv, queries := stubBrandListCapturing(t) + if _, _, err := runBrandCmd(t, srv, "brand", "list", + "--brand-id-contains", "BEXMPL1", + "--customer-profile-id-contains", "9900000", + "--brand-type", "PRIVATE_PROFIT"); err != nil { + t.Fatalf("brand list: %v", err) + } + q := (*queries)[0] + for _, want := range []string{ + "brandId%5Bcontains%5D=BEXMPL1", + "customerProfileId%5Bcontains%5D=9900000", + "brandType%5Bcontains%5D=PRIVATE_PROFIT", + } { + if !strings.Contains(q, want) { + t.Errorf("query %q missing %q", q, want) + } + } +} + +// stubBrandListIdentityStatusTrap serves three brands whose +// brandIdentityStatus values all contain the substring "VERIFIED": +// VERIFIED, VETTED_VERIFIED, and UNVERIFIED. It stands in for the +// production measurement in brand_list.go's RunE comment -- +// brandIdentityStatus[contains]=VERIFIED matched all three on a real +// account -- so tests can assert on the CLI's response to that trap without +// a live account. +func stubBrandListIdentityStatusTrap(t *testing.T) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[ + {"bandwidthId":"W1","brandId":"B1","brandIdentityStatus":"VERIFIED"}, + {"bandwidthId":"W2","brandId":"B2","brandIdentityStatus":"VETTED_VERIFIED"}, + {"bandwidthId":"W3","brandId":"B3","brandIdentityStatus":"UNVERIFIED"} + ],"page":{"pageNumber":0,"pageSize":50,"totalElements":3,"totalPages":1}}`)) + }) +} + +// TestBrandListIdentityStatusExcludesSubstringMatches proves the +// VERIFIED-matches-three-statuses trap is handled: the server's contains +// filter would return all three brands above (VERIFIED, VETTED_VERIFIED, +// and -- the dangerous one -- UNVERIFIED, the exact opposite of what was +// asked for), but the CLI must narrow that down to only the brand whose +// status is exactly "VERIFIED". +func TestBrandListIdentityStatusExcludesSubstringMatches(t *testing.T) { + out, _, err := runBrandCmd(t, stubBrandListIdentityStatusTrap(t), "brand", "list", + "--identity-status", "VERIFIED", "--plain") + if err != nil { + t.Fatalf("brand list: %v", err) + } + if !strings.Contains(out, `"B1"`) { + t.Errorf("stdout = %q, want the exact VERIFIED brand B1", out) + } + if strings.Contains(out, `"B2"`) { + t.Errorf("stdout = %q, must not include VETTED_VERIFIED brand B2 (contains-only match)", out) + } + if strings.Contains(out, `"B3"`) { + t.Errorf("stdout = %q, must not include UNVERIFIED brand B3 -- the exact opposite of the requested status", out) + } +} + +func TestBrandListWarnsOnTruncationViaStderrOnly(t *testing.T) { + out, errOut, err := runBrandCmd(t, stubBrandListTruncated(t), "brand", "list", "--limit", "1") + if err != nil { + t.Fatalf("brand list: %v", err) + } + if strings.Contains(out, "pass --all") { + t.Error("truncation warning leaked into stdout; stdout must stay parseable") + } + if !strings.Contains(errOut, "pass --all") { + t.Errorf("stderr should carry the truncation warning, got %q", errOut) + } +} + +// TestBrandListAllWalksEveryPage exercises the ForEachPage accumulation +// branch, mirroring cmd/customerprofile/customerprofile_test.go's +// TestListAllWalksEveryPage: the stub serves distinct items per page, and the +// assertion requires BOTH pages' items in stdout, not just a count — an +// implementation that fetched page one twice (or dropped a page) would fail +// this even though len(all) might coincidentally match. +func TestBrandListAllWalksEveryPage(t *testing.T) { + out, errOut, err := runBrandCmd(t, stubBrandListTwoPages(t), "brand", "list", "--all", "--limit", "1", "--plain") + if err != nil { + t.Fatalf("brand list --all: %v", err) + } + if !strings.Contains(out, "PAGE-A") || !strings.Contains(out, "PAGE-B") { + t.Errorf("stdout = %q, want items from both pages", out) + } + // --all walks every page, so nothing was left un-fetched. A truncation + // warning here would be a lie: it would tell the caller records remain + // when the command just fetched all of them. + if strings.Contains(errOut, "pass --all") { + t.Errorf("stderr = %q, want no truncation warning when --all already walked every page", errOut) + } +} + +func TestBrandGetAcceptsEitherIdentifier(t *testing.T) { + srv, paths := stubBrandGetCapturing(t) + for _, id := range []string{"BGJR2BA", "WET8JUY8H0"} { + if _, _, err := runBrandCmd(t, srv, "brand", "get", id); err != nil { + t.Fatalf("brand get %s: %v", id, err) + } + } + if len(*paths) != 2 || !strings.HasSuffix((*paths)[0], "/brands/BGJR2BA") || !strings.HasSuffix((*paths)[1], "/brands/WET8JUY8H0") { + t.Errorf("paths = %v; get must pass the ID through unchanged", *paths) + } +} + +func TestBrandCommandsRejectStrayPositionals(t *testing.T) { + // A stray positional on a read is harmless; on a write it is not, and the + // guard belongs on every command so the rule is not a per-command judgment + // call. PR 2 shipped a create without one and it silently created a + // resource from a typo. + cases := [][]string{ + {"brand", "list", "STRAY"}, + {"brand", "get"}, + {"brand", "get", "B1", "STRAY"}, + {"brand", "history"}, + {"brand", "history", "B1", "STRAY"}, + } + for _, args := range cases { + t.Run(strings.Join(args, " "), func(t *testing.T) { + if _, _, err := runBrandCmd(t, stubBrandList(t), args...); err == nil { + t.Fatal("want an argument error") + } + }) + } +} + +func TestBrandHistoryReturnsMessageLog(t *testing.T) { + out, _, err := runBrandCmd(t, stubBrandHistory(t), "brand", "history", "BGJR2BA") + if err != nil { + t.Fatalf("brand history: %v", err) + } + if !strings.Contains(out, "Successfully updated brand") { + t.Errorf("stdout should carry history messages, got %q", out) + } +} + +// TestBrandHistoryAllWalksEveryPage is TestBrandListAllWalksEveryPage's twin +// for `brand history --all`. +func TestBrandHistoryAllWalksEveryPage(t *testing.T) { + out, errOut, err := runBrandCmd(t, stubBrandHistoryTwoPages(t), "brand", "history", "BGJR2BA", + "--all", "--limit", "1", "--plain") + if err != nil { + t.Fatalf("brand history --all: %v", err) + } + if !strings.Contains(out, "page one message") || !strings.Contains(out, "page two message") { + t.Errorf("stdout = %q, want messages from both pages", out) + } + if strings.Contains(errOut, "pass --all") { + t.Errorf("stderr = %q, want no truncation warning when --all already walked every page", errOut) + } +} + +func TestRoleGate403MapsToExitFour(t *testing.T) { + _, _, err := runBrandCmd(t, stubBrandErr(t, 403, + `{"errors":[{"description":"does not have access rights"}]}`), "brand", "list") + if err == nil { + t.Fatal("want an error on 403") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitConflict { + t.Errorf("exit code = %d, want %d — re-authenticating cannot add a role", code, cmdutil.ExitConflict) + } +} + +// requireConfirm has no caller yet in this task — brand list/get/history are +// all reads. It is added now because a later task (brand create/delete) +// needs it, and golangci-lint's unused check fails a helper with zero +// callers; this test is that caller until the write commands land. +func TestRequireConfirmGatesOnConfirmFlag(t *testing.T) { + if err := requireConfirm(true, "should not fire"); err != nil { + t.Fatalf("requireConfirm(true, ...) = %v, want nil", err) + } + err := requireConfirm(false, "this action needs --confirm because it deletes the brand") + if err == nil { + t.Fatal("requireConfirm(false, ...) = nil, want an error") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + if !strings.Contains(err.Error(), "deletes the brand") { + t.Errorf("error = %q, want it to carry the specific consequence", err.Error()) + } +} diff --git a/cmd/tendlc/brand_update.go b/cmd/tendlc/brand_update.go new file mode 100644 index 0000000..9995828 --- /dev/null +++ b/cmd/tendlc/brand_update.go @@ -0,0 +1,221 @@ +package tendlc + +import ( + "errors" + "fmt" + "strings" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" + tendlcsvc "github.com/Bandwidth/cli/internal/tendlc" +) + +var ( + brandUpdateOpts tendlcsvc.BrandUpdateOptions + brandUpdateConfirm bool +) + +func init() { + f := brandUpdateCmd.Flags() + f.StringVar(&brandUpdateOpts.BrandType, "brand-type", "", "Brand entity type: PRIVATE_PROFIT, PUBLIC_PROFIT, NON_PROFIT, GOVERNMENT, SOLE_PROPRIETOR") + f.StringVar(&brandUpdateOpts.DisplayName, "display-name", "", "Display name") + f.StringVar(&brandUpdateOpts.CompanyName, "company-name", "", "Legal company name") + f.StringVar(&brandUpdateOpts.Street, "street", "", "Street address") + f.StringVar(&brandUpdateOpts.City, "city", "", "City") + f.StringVar(&brandUpdateOpts.State, "state", "", "State or province") + f.StringVar(&brandUpdateOpts.PostalCode, "postal-code", "", "Postal code") + f.StringVar(&brandUpdateOpts.CountryCodeA3, "country-code-a3", "", "ISO 3166-1 alpha-3 country code") + f.StringVar(&brandUpdateOpts.Phone, "phone", "", "Business phone number") + f.StringVar(&brandUpdateOpts.Email, "email", "", "Business email address") + f.StringVar(&brandUpdateOpts.Vertical, "vertical", "", "Industry vertical") + f.StringVar(&brandUpdateOpts.EIN, "ein", "", "Employer Identification Number") + f.StringVar(&brandUpdateOpts.EINIssuingCountryCodeA3, "ein-issuing-country-code-a3", "", "Country that issued the EIN") + f.StringVar(&brandUpdateOpts.Website, "website", "", "Business website URL") + f.StringVar(&brandUpdateOpts.StockSymbol, "stock-symbol", "", "Stock ticker symbol") + f.StringVar(&brandUpdateOpts.StockExchange, "stock-exchange", "", "Stock exchange") + f.StringVar(&brandUpdateOpts.BusinessContactEmail, "business-contact-email", "", "Business contact email") + f.StringVar(&brandUpdateOpts.AltBusinessID, "alt-business-id", "", "Alternate business identifier (e.g. DUNS number)") + f.StringVar(&brandUpdateOpts.AltBusinessIDType, "alt-business-id-type", "", "Type of the alternate business identifier") + f.StringVar(&brandUpdateOpts.FirstName, "first-name", "", "Contact first name (sole proprietor)") + f.StringVar(&brandUpdateOpts.LastName, "last-name", "", "Contact last name (sole proprietor)") + f.StringVar(&brandUpdateOpts.MobilePhone, "mobile-phone", "", "Contact mobile phone (sole proprietor)") + f.StringVar(&brandUpdateOpts.IPAddress, "ip-address", "", "IP address the form was completed from (sole proprietor)") + f.BoolVar(&brandUpdateConfirm, "confirm", false, "Required when the change affects the brand's identity verification, mobile-phone, or business-contact-email (PUBLIC_PROFIT).") + brandCmd.AddCommand(brandUpdateCmd) +} + +var brandUpdateCmd = &cobra.Command{ + Use: "update ", + Short: "Update a 10DLC brand", + Long: `Updates a 10DLC brand. + +The API replaces the whole record on update, so this command reads the brand +first and sends it back with your changes applied. Fields you do not pass are +preserved; passing a flag with an empty value clears that field (a field +required on every brand cannot be cleared this way). + +This command prints an acceptance receipt, not the updated brand: measured +against production, the PUT here returns a bare {bandwidthId, brandId} +acceptance, and the change itself takes roughly 5 minutes to be reflected in +'brand get' or 'brand history' — modifiedDate and the activity log both lag +behind. Re-check after a few minutes to confirm the change actually applied. + +No --wait: unlike 'brand create', this write lands against a brand that is +usually already in a terminal identity state, so polling for one here would +return immediately and report success before the change actually took +effect. The ~5 minute apply-latency above confirms this rather than merely +motivating it: a --wait here would poll a brand still holding its +pre-update state and report success before anything actually changed. + +Some changes need --confirm: changing company-name, brand-type, ein, or +ein-issuing-country-code-a3 resubmits the brand for identity verification +(may incur a $4 fee, resets brandIdentityStatus toward re-registration — +documented as REGISTERING, but it reads back as UNVERIFIED until TCR +responds — and is rejected outright if the brand has an active campaign or +an active Standard/Enhanced/Political vetting). Changing mobile-phone sets +identity status to UNVERIFIED. Changing business-contact-email on a +PUBLIC_PROFIT brand revokes Auth+ compliance.`, + Example: ` band tendlc brand update BGJR2BA --website "https://acme.example" --plain + band tendlc brand update BGJR2BA --company-name "Acme Corp 2" --confirm --plain`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + changed := map[string]bool{} + any := false + for _, name := range tendlcsvc.BrandUpdateFieldFlags { + if cmd.Flags().Changed(name) { + changed[name] = true + any = true + } + } + if !any { + return cmdutil.NewFlagError( + "nothing to update — pass at least one of " + flagList(tendlcsvc.BrandUpdateFieldFlags)) + } + + svc, err := service(cmd) + if err != nil { + return err + } + + env, err := svc.GetBrand(args[0]) + if err != nil { + return roleGateError(err, "Campaign Management") + } + current, err := env.Object() + if err != nil { + return err + } + + // The --confirm gate for identity-affecting fields fires HERE, after the + // GET above, rather than before it — this is the one confirm gate in the + // whole PR that is not entirely zero-request. That is deliberate, not an + // oversight to "fix" into a pre-GET check: IdentityFieldsChanged's + // business-contact-email/PUBLIC_PROFIT condition needs the brand's + // CURRENT type, which is only known once this GET has returned. Moving + // this check earlier to make refusal zero-request-of-any-kind would + // silently stop detecting that case — a caller changing + // business-contact-email on a PUBLIC_PROFIT brand would sail through + // with no Auth+ warning at all. The property that actually matters here + // — zero *write* requests before --confirm is satisfied — still holds: + // this gate can only ever have caused a GET, never the PUT below. + if fields := tendlcsvc.IdentityFieldsChanged(current, changed); len(fields) > 0 { + if err := requireConfirm(brandUpdateConfirm, identityConfirmMessage(args[0], fields)); err != nil { + return err + } + } + + body, err := tendlcsvc.BuildBrandUpdateRequest(current, brandUpdateOpts, changed) + if err != nil { + return err + } + + updated, err := svc.UpdateBrand(args[0], body) + if err != nil { + return brandUpdateConflictHint(args[0], err) + } + + // PUT /brands/{id} returns an ACCEPTANCE, not the updated resource — + // measured against production: the body is a bare {bandwidthId, + // brandId}, and the change itself takes roughly 5 minutes to be + // reflected in a follow-up 'brand get' or 'brand history'. Printing + // updated.Object() here would hand back that 2-key acceptance as + // though it were the brand, with nothing telling the caller a change + // is still pending — so this reuses buildAcceptedReceipt (the same + // shape 'brand create'/'brand refresh' print) and adds a note naming + // the latency, rather than printing the raw response as the result. + receipt, bandwidthID, err := buildAcceptedReceipt(cmd, updated) + if err != nil { + return err + } + receipt["note"] = "this is an acceptance, not the updated brand: production takes about 5 " + + "minutes to apply the change (modifiedDate and the history log lag behind), so an " + + "immediate 'brand get' may still show the pre-update value. Check 'band tendlc brand get " + + bandwidthID + "' again shortly, or 'band tendlc brand history " + bandwidthID + + "' for confirmation." + format, _ := cmdutil.OutputFlags(cmd) + return output.Stdout(format, receipt) + }, +} + +// identityVerificationFields are the identity-affecting flags whose +// consequence is resubmission for identity verification — as opposed to +// mobile-phone and business-contact-email, which each have a distinct +// consequence named separately below. +var identityVerificationFields = map[string]bool{ + "company-name": true, "brand-type": true, "ein": true, "ein-issuing-country-code-a3": true, +} + +// identityConfirmMessage builds the --confirm refusal for an identity- +// affecting change. fields is the (already sorted, already conditioned) +// output of IdentityFieldsChanged. Every consequence that applies is named — +// a generic "this changes something important" would tell the caller nothing +// about what they are agreeing to, and when several fields apply for +// different reasons, all of them must be named, not just the first match. +func identityConfirmMessage(brandID string, fields []string) string { + var verification []string + mobileChanged := false + businessContactEmailChanged := false + for _, f := range fields { + switch { + case identityVerificationFields[f]: + verification = append(verification, f) + case f == "mobile-phone": + mobileChanged = true + case f == "business-contact-email": + businessContactEmailChanged = true + } + } + + var parts []string + if len(verification) > 0 { + parts = append(parts, fmt.Sprintf( + "changing %s on brand %s resubmits it for identity verification: this may incur a $4 fee and resets brandIdentityStatus toward re-registration (it reads back as UNVERIFIED until TCR responds). If the brand has an active campaign or an active Standard/Enhanced/Political vetting, the API will reject the change outright.", + strings.Join(verification, ", "), brandID)) + } + if mobileChanged { + parts = append(parts, "changing mobile-phone sets identity status to UNVERIFIED.") + } + if businessContactEmailChanged { + parts = append(parts, "changing business-contact-email on a PUBLIC_PROFIT brand revokes Auth+ compliance; regaining it requires a new AUTHPLUS vetting and another 2FA email verification.") + } + parts = append(parts, "Pass --confirm to proceed.") + return strings.Join(parts, " ") +} + +// brandUpdateConflictHint maps a 409 on the update PUT to its specific cause: +// the brand exists in Bandwidth but TCR has not caught up with it yet. Any +// other error still goes through roleGateError, so a 403 on update gets the +// same targeted message every other tendlc write does. +func brandUpdateConflictHint(brandID string, err error) error { + var apiErr *api.APIError + if errors.As(err, &apiErr) && apiErr.StatusCode == 409 { + return &cmdutil.ConflictError{ + Message: "brand " + brandID + " exists in Bandwidth but not yet in TCR; run 'band tendlc brand refresh " + brandID + "' and retry", + Cause: err, + } + } + return roleGateError(err, "Campaign Management") +} diff --git a/cmd/tendlc/brand_update_test.go b/cmd/tendlc/brand_update_test.go new file mode 100644 index 0000000..71584ca --- /dev/null +++ b/cmd/tendlc/brand_update_test.go @@ -0,0 +1,321 @@ +package tendlc + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// liveBrandForUpdate mirrors the shape GET /brands/{id} actually returns — +// the same fixture internal/tendlc's own (unexported) liveBrand() test helper +// uses, reproduced here because that helper is private to internal/tendlc's +// test package. It deliberately carries "someFutureField", a key the CLI does +// not model at all, so the rename-only losslessness test below has something +// genuinely unmodeled to check for. +func liveBrandForUpdate(brandType, businessContactEmail string) map[string]any { + return map[string]any{ + "bandwidthId": "WET8JUY8H0", + "brandId": "BGJR2BA", + "brandIdentityStatus": "VERIFIED", + "brandType": brandType, + "companyName": "Bandwidth Inc", + "displayName": "Bandwidth Acceptance Test", + "street": "1000 Bandwidth Way", + "city": "Raleigh", + "state": "NC", + "postalCode": "27606", + "countryCodeA3": "USA", + "phone": "+12025551234", + "email": "npatel@bandwidth.com", + "ein": "562242657", + "einIssuingCountryCodeA3": "USA", + "vertical": "PROFESSIONAL", + "website": "https://bandwidth.com", + "stockSymbol": "BAND", + "stockExchange": "NASDAQ", + "businessContactEmail": businessContactEmail, + "someFutureField": "keep me", + } +} + +// stubBrandUpdateServer answers GET /brands/{id} with getBody wrapped in the +// standard envelope, and PUT with either putStatus's error body (if +// putStatus != 200) or the measured production acceptance shape — a bare +// {bandwidthId, brandId}, carrying getBody's own IDs, NOT an echo of the sent +// body. Measured against production: PUT /brands/{id} returns only an +// acceptance receipt, never the updated resource, so echoing the request +// back would misrepresent the real API and hide the bug this stub exists to +// catch. It still records every PUT body's raw JSON so a test can assert on +// the request the CLI actually sent, independent of what the response looks +// like. +func stubBrandUpdateServer(t *testing.T, getBody map[string]any, putStatus int) (*httptest.Server, *[]string) { + t.Helper() + getJSON, err := json.Marshal(map[string]any{"data": getBody}) + if err != nil { + t.Fatalf("marshaling GET fixture: %v", err) + } + acceptJSON, err := json.Marshal(map[string]any{ + "data": map[string]any{ + "bandwidthId": getBody["bandwidthId"], + "brandId": getBody["brandId"], + }, + }) + if err != nil { + t.Fatalf("marshaling PUT acceptance fixture: %v", err) + } + var putBodies []string + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write(getJSON) + return + } + b, _ := io.ReadAll(r.Body) + putBodies = append(putBodies, string(b)) + if putStatus != http.StatusOK { + w.WriteHeader(putStatus) + _, _ = w.Write([]byte(`{"errors":[{"description":"conflict"}]}`)) + return + } + _, _ = w.Write(acceptJSON) + }) + return srv, &putBodies +} + +// Test 1: no field flags at all is exit 6 and makes no request whatsoever — +// runBrandCmd's `service` seam Fatals if invoked with a nil server, so this +// cannot pass by accident even if the "nothing to update" check moved after +// the service/GET call. +func TestBrandUpdateNoFieldFlagsExitsSixWithZeroRequests(t *testing.T) { + _, _, err := runBrandCmd(t, nil, "brand", "update", "BGJR2BA") + if err == nil { + t.Fatal("want an error when no field flags are passed") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + if !strings.Contains(err.Error(), "nothing to update") { + t.Errorf("error = %q, want it to say nothing to update", err.Error()) + } +} + +// Test 2: THE load-bearing regression test for this task. A rename-only +// update must preserve every other field on the PUT body — including +// companyName, website, and someFutureField, a key the CLI does not model at +// all. This is not a spot check: losing any of these silently would be +// exactly the failure a customer renaming a brand must never hit. +func TestBrandUpdateRenameOnlyPreservesEveryOtherField(t *testing.T) { + srv, bodies := stubBrandUpdateServer(t, liveBrandForUpdate("PRIVATE_PROFIT", "biz@acme.com"), http.StatusOK) + + _, _, err := runBrandCmd(t, srv, "brand", "update", "BGJR2BA", "--display-name", "Renamed", "--plain") + if err != nil { + t.Fatalf("brand update: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want exactly one PUT, got %d", len(*bodies)) + } + var sent map[string]any + if err := json.Unmarshal([]byte((*bodies)[0]), &sent); err != nil { + t.Fatalf("PUT body is not JSON: %v", err) + } + if sent["displayName"] != "Renamed" { + t.Errorf("sent displayName = %v, want Renamed", sent["displayName"]) + } + if sent["companyName"] != "Bandwidth Inc" { + t.Errorf("sent companyName = %v, want it preserved as Bandwidth Inc", sent["companyName"]) + } + if sent["website"] != "https://bandwidth.com" { + t.Errorf("sent website = %v, want it preserved", sent["website"]) + } + if sent["someFutureField"] != "keep me" { + t.Errorf("sent someFutureField = %v, want the unmodeled field preserved", sent["someFutureField"]) + } +} + +// Test 3: changing --company-name without --confirm exits 6. The GET must +// have happened (the stub would otherwise never see the PUT-absence +// assertion meaningfully), but no PUT is sent. +func TestBrandUpdateCompanyNameWithoutConfirmRefuses(t *testing.T) { + srv, bodies := stubBrandUpdateServer(t, liveBrandForUpdate("PRIVATE_PROFIT", "biz@acme.com"), http.StatusOK) + + _, _, err := runBrandCmd(t, srv, "brand", "update", "BGJR2BA", "--company-name", "New Co", "--plain") + if err == nil { + t.Fatal("want an error when an identity field changes without --confirm") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + if !strings.Contains(err.Error(), "company-name") { + t.Errorf("error = %q, want it to name company-name", err.Error()) + } + if !strings.Contains(err.Error(), "$4 fee") { + t.Errorf("error = %q, want it to mention the $4 fee", err.Error()) + } + if len(*bodies) != 0 { + t.Errorf("want zero PUT requests, got %d", len(*bodies)) + } +} + +// Test 4: the same change with --confirm goes through and issues the PUT. +func TestBrandUpdateCompanyNameWithConfirmProceeds(t *testing.T) { + srv, bodies := stubBrandUpdateServer(t, liveBrandForUpdate("PRIVATE_PROFIT", "biz@acme.com"), http.StatusOK) + + _, _, err := runBrandCmd(t, srv, "brand", "update", "BGJR2BA", "--company-name", "New Co", "--confirm", "--plain") + if err != nil { + t.Fatalf("brand update --confirm: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want exactly one PUT, got %d", len(*bodies)) + } + var sent map[string]any + if err := json.Unmarshal([]byte((*bodies)[0]), &sent); err != nil { + t.Fatalf("PUT body is not JSON: %v", err) + } + if sent["companyName"] != "New Co" { + t.Errorf("sent companyName = %v, want New Co", sent["companyName"]) + } +} + +// Test 5: changing only --website needs no --confirm at all. +func TestBrandUpdateWebsiteOnlyNeedsNoConfirm(t *testing.T) { + srv, bodies := stubBrandUpdateServer(t, liveBrandForUpdate("PRIVATE_PROFIT", "biz@acme.com"), http.StatusOK) + + _, _, err := runBrandCmd(t, srv, "brand", "update", "BGJR2BA", "--website", "https://acme.example", "--plain") + if err != nil { + t.Fatalf("brand update --website: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want exactly one PUT, got %d", len(*bodies)) + } +} + +// Test 6: --business-contact-email needs no --confirm on a PRIVATE_PROFIT +// brand, but does on a PUBLIC_PROFIT one — IdentityFieldsChanged reads the +// CURRENT brand's type, not anything the caller passed. +func TestBrandUpdateBusinessContactEmailConfirmDependsOnBrandType(t *testing.T) { + t.Run("PRIVATE_PROFIT needs no confirm", func(t *testing.T) { + srv, bodies := stubBrandUpdateServer(t, liveBrandForUpdate("PRIVATE_PROFIT", "old@acme.com"), http.StatusOK) + _, _, err := runBrandCmd(t, srv, "brand", "update", "BGJR2BA", + "--business-contact-email", "new@acme.com", "--plain") + if err != nil { + t.Fatalf("brand update: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want exactly one PUT, got %d", len(*bodies)) + } + }) + + t.Run("PUBLIC_PROFIT needs confirm", func(t *testing.T) { + srv, bodies := stubBrandUpdateServer(t, liveBrandForUpdate("PUBLIC_PROFIT", "old@acme.com"), http.StatusOK) + _, _, err := runBrandCmd(t, srv, "brand", "update", "BGJR2BA", + "--business-contact-email", "new@acme.com", "--plain") + if err == nil { + t.Fatal("want an error requiring --confirm on a PUBLIC_PROFIT brand") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + if !strings.Contains(err.Error(), "Auth+") { + t.Errorf("error = %q, want it to mention Auth+", err.Error()) + } + if len(*bodies) != 0 { + t.Errorf("want zero PUT requests, got %d", len(*bodies)) + } + + if _, _, err := runBrandCmd(t, srv, "brand", "update", "BGJR2BA", + "--business-contact-email", "new@acme.com", "--confirm", "--plain"); err != nil { + t.Fatalf("brand update --confirm: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want exactly one PUT after --confirm, got %d", len(*bodies)) + } + }) +} + +// Test 7: clearing a universally-required field exits 6 before any PUT. This +// is not an identity field, so it must fail on ValidateBrandUpdate rather +// than the confirm gate. +func TestBrandUpdateClearingRequiredFieldExitsSixBeforePut(t *testing.T) { + srv, bodies := stubBrandUpdateServer(t, liveBrandForUpdate("PRIVATE_PROFIT", "biz@acme.com"), http.StatusOK) + + _, _, err := runBrandCmd(t, srv, "brand", "update", "BGJR2BA", "--display-name", "", "--plain") + if err == nil { + t.Fatal("want an error when clearing a universally-required field") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + if len(*bodies) != 0 { + t.Errorf("want zero PUT requests, got %d", len(*bodies)) + } +} + +// Test 8: update offers no --wait flag — the write lands against a brand +// usually already in a terminal state, so polling would report success +// before the change applied. +func TestBrandUpdateHasNoWaitFlag(t *testing.T) { + if f := brandUpdateCmd.Flags().Lookup("wait"); f != nil { + t.Errorf("brandUpdateCmd has a --wait flag, want none: %+v", f) + } +} + +// Test 9: a 409 on the PUT means the brand exists in Bandwidth but not yet in +// TCR — exit 4, and the message points at 'band tendlc brand refresh'. +func TestBrandUpdateConflictOnPutMapsToTCRHint(t *testing.T) { + srv, _ := stubBrandUpdateServer(t, liveBrandForUpdate("PRIVATE_PROFIT", "biz@acme.com"), http.StatusConflict) + + _, _, err := runBrandCmd(t, srv, "brand", "update", "BGJR2BA", "--website", "https://acme.example", "--plain") + if err == nil { + t.Fatal("want an error on a 409 PUT") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitConflict { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitConflict) + } + if !strings.Contains(err.Error(), "TCR") { + t.Errorf("error = %q, want it to mention TCR", err.Error()) + } + if !strings.Contains(err.Error(), "brand refresh") { + t.Errorf("error = %q, want it to suggest brand refresh", err.Error()) + } +} + +// Test 10: THE load-bearing regression test for A5. Production measured: +// PUT /brands/{id} returns only a bare {bandwidthId, brandId} acceptance, +// and the change itself takes roughly 5 minutes to apply. A successful +// update must print an acceptance receipt carrying both IDs, status +// "accepted", and a note naming the apply latency — not the raw PUT +// response as though it were the updated brand. This stub's PUT response +// deliberately does NOT echo the sent body (see stubBrandUpdateServer), so +// if the command reverted to printing updated.Object(), this test would fail +// on the missing "status"/"note" fields; and even against an echoing stub, +// the explicit check that "website" is absent from the receipt would still +// catch a regression back to printing the raw response body. +func TestBrandUpdatePrintsAcceptanceReceiptWithIDsAndLatencyNote(t *testing.T) { + srv, _ := stubBrandUpdateServer(t, liveBrandForUpdate("PRIVATE_PROFIT", "biz@acme.com"), http.StatusOK) + + out, _, err := runBrandCmd(t, srv, "brand", "update", "BGJR2BA", "--website", "https://acme.example", "--plain") + if err != nil { + t.Fatalf("brand update: %v", err) + } + got := decodeStdout(t, out) + if got["bandwidthId"] != "WET8JUY8H0" { + t.Errorf("stdout = %v, want bandwidthId WET8JUY8H0", got) + } + if got["brandId"] != "BGJR2BA" { + t.Errorf("stdout = %v, want brandId BGJR2BA", got) + } + if got["status"] != "accepted" { + t.Errorf("stdout = %v, want status accepted", got) + } + note, _ := got["note"].(string) + if !strings.Contains(note, "5") || !strings.Contains(note, "brand get") { + t.Errorf("stdout note = %q, want it to mention the ~5 minute apply latency and 'brand get'", note) + } + if _, ok := got["website"]; ok { + t.Errorf("stdout = %v, must not print the raw PUT body as though it were the updated brand", got) + } +} diff --git a/cmd/tendlc/helpers.go b/cmd/tendlc/helpers.go index 3a46063..4f3bd5a 100644 --- a/cmd/tendlc/helpers.go +++ b/cmd/tendlc/helpers.go @@ -1,9 +1,12 @@ package tendlc import ( + "errors" "fmt" "strings" + "github.com/spf13/cobra" + "github.com/Bandwidth/cli/internal/api" "github.com/Bandwidth/cli/internal/cmdutil" ) @@ -100,3 +103,40 @@ func filterNumbers(data interface{}, status, campaignID string) interface{} { } return filtered } + +// isNotFound reports whether err is an API 404. +func isNotFound(err error) bool { + var apiErr *api.APIError + return errors.As(err, &apiErr) && apiErr.StatusCode == 404 +} + +// requireConfirm enforces a --confirm gate before any HTTP request is made. +// The message must name the specific consequence — a generic "pass --confirm" +// tells an operator nothing about what they are agreeing to. +func requireConfirm(confirm bool, message string) error { + if confirm { + return nil + } + return cmdutil.NewFlagError(message) +} + +// flagList renders flag names as a "--a, --b, --c" list for error messages. +func flagList(names []string) string { + out := "" + for i, n := range names { + if i > 0 { + out += ", " + } + out += "--" + n + } + return out +} + +// warnIfTruncated tells the caller on stderr when more records exist than the +// page just returned. stdout stays clean so a pipeline sees only data. +func warnIfTruncated(cmd *cobra.Command, env *api.Envelope, offset, returned int, noun string) { + if env.Page != nil && env.Page.Truncated(offset+returned) { + cmd.PrintErrf("showing %d of %d %s; pass --all to fetch every page\n", + returned, env.Page.TotalElements, noun) + } +} diff --git a/cmd/tendlc/tendlc_test.go b/cmd/tendlc/tendlc_test.go index 505dd19..f52d779 100644 --- a/cmd/tendlc/tendlc_test.go +++ b/cmd/tendlc/tendlc_test.go @@ -1,6 +1,8 @@ package tendlc import ( + "errors" + "fmt" "testing" "github.com/Bandwidth/cli/internal/api" @@ -274,6 +276,35 @@ func TestStatusAlwaysReportsModeUnknown(t *testing.T) { } } +// isNotFound is the sole translator of a real 404 into pollTarget's +// found=false, which is the mechanism the create-vs-delete GoneIsDone +// contract in async.go depends on — it must recognize a 404 wherever it +// appears in the error chain, and reject everything else, including a nil +// error. +func TestIsNotFound(t *testing.T) { + notFound := &api.APIError{StatusCode: 404, Body: "brand not found"} + serverError := &api.APIError{StatusCode: 500, Body: "boom"} + + tests := []struct { + name string + err error + want bool + }{ + {"404 API error", notFound, true}, + {"500 API error", serverError, false}, + {"wrapped 404", fmt.Errorf("fetching brand: %w", notFound), true}, + {"plain non-API error", errors.New("connection reset"), false}, + {"nil", nil, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isNotFound(tt.err); got != tt.want { + t.Errorf("isNotFound(%v) = %v, want %v", tt.err, got, tt.want) + } + }) + } +} + func contains(s, sub string) bool { return len(s) >= len(sub) && (s == sub || len(s) > 0 && containsSubstring(s, sub)) } diff --git a/cmd/tendlc/vetting.go b/cmd/tendlc/vetting.go new file mode 100644 index 0000000..a34825a --- /dev/null +++ b/cmd/tendlc/vetting.go @@ -0,0 +1,425 @@ +package tendlc + +import ( + "fmt" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" + tendlcsvc "github.com/Bandwidth/cli/internal/tendlc" +) + +// vettingCmd is the `band tendlc vetting` parent. +// +// Every subcommand below takes a BRAND ID, not a vetting ID — vettings are +// brand-scoped, and there is no campaign vetting endpoint in either spec. A +// campaign's vettingStatus is a read-only field derived from its brand's +// vetting state; re-evaluating a campaign directly is a different command +// ('nudge'). +var vettingCmd = &cobra.Command{ + Use: "vetting", + Short: "Order and record external vettings on a 10DLC brand", + Long: `Order and record third-party vettings for a brand. + +Vettings are brand-scoped: there is no campaign vetting endpoint in either +spec. A campaign only exposes a read-only, derived vettingStatus; requesting +a new evaluation for a campaign is a different command ('nudge'). + +Every command here accepts a brand ID as its first positional, not a +vetting ID. + +Requires the Registration Center feature and the Campaign Management role.`, +} + +func init() { + Cmd.AddCommand(vettingCmd) +} + +// vettingEvpProviders are the external vetting providers the API accepts. A +// small, stable, documented enum — unlike vettingClasses below, nothing here +// is undocumented. +var vettingEvpProviders = []string{"AEGIS", "CV", "WMC"} + +// vettingClasses are the vetting classes the API accepts. +// +// RCS is included deliberately even though it is absent from the published +// enumVettingClass: production accepts it, confirmed by pairing each class +// with an invalid evpId and observing which produced a class-level error +// rather than an evp-level one. Dropping it to match the spec would reject a +// value production actually honors — do not "fix" this to match the spec. +var vettingClasses = []string{"STANDARD", "ENHANCED", "POLITICAL", "AUTHPLUS", "RCS"} + +// vettingIDKeys are the two field names production uses for a vetting's ID. +// The vettings list returns it under bandwidthId; the spec shows +// vettingBandwidthId on the POST .../vettings 202. Both are checked +// wherever a vetting object is read, and whichever key is actually present is +// preserved as-is in receipts rather than normalized to one name — see +// buildVettingReceipt. +var vettingIDKeys = []string{"bandwidthId", "vettingBandwidthId"} + +func isValidEnum(v string, enum []string) bool { + for _, e := range enum { + if e == v { + return true + } + } + return false +} + +// validateVettingRequest reports every violation of --evp/--class in one +// error, the way the API reports every violation in one 400: a missing flag +// and an invalid value on the OTHER flag are combined rather than only the +// first being surfaced. +func validateVettingRequest(evp, class string) error { + var missing []string + var invalid []string + + switch { + case evp == "": + missing = append(missing, "evp") + case !isValidEnum(evp, vettingEvpProviders): + invalid = append(invalid, "--evp must be one of: "+strings.Join(vettingEvpProviders, ", ")) + } + switch { + case class == "": + missing = append(missing, "class") + case !isValidEnum(class, vettingClasses): + invalid = append(invalid, "--class must be one of: "+strings.Join(vettingClasses, ", ")) + } + + if len(missing) == 0 && len(invalid) == 0 { + return nil + } + if len(invalid) == 0 { + return cmdutil.NewMissingFlagsError(missing) + } + msg := strings.Join(invalid, "; ") + if len(missing) > 0 { + prefixed := make([]string, len(missing)) + for i, m := range missing { + prefixed[i] = "--" + m + } + msg += "; missing required flags: " + strings.Join(prefixed, ", ") + } + return cmdutil.NewFlagError(msg) +} + +// buildVettingReceipt turns a vetting write's response (a POST .../vettings +// 202, or a PUT .../vettings/{id} response) into the receipt this command set +// prints: {: id, brandId, status, check}. +// +// idField (found internally, not returned — see below) is whichever of +// vettingIDKeys was actually present on the response: bandwidthId or +// vettingBandwidthId. It is preserved under its own name rather than +// normalized: normalizing would be tidier and would be wrong, since it would +// misreport which field the API actually sent. +// +// If the response carries neither key, there is no ID to poll or look up +// against the vettings list, so the caller must not proceed. This prints +// whatever the body actually was via output.Stdout, not StdoutAuto: env.Data +// is real API data, not a synthetic receipt, but it can still be a +// single-key map, and FlattenResponse unwraps any single-key map — under +// --plain that would drop the key and print a bare value instead of the +// object it came from. See async.go's emitReceipt for the same reasoning +// applied to synthetic receipts. +func buildVettingReceipt(cmd *cobra.Command, env *api.Envelope, brandID string) (receipt map[string]any, vettingID string, err error) { + obj, objErr := env.Object() + var idField string + for _, key := range vettingIDKeys { + if v, ok := obj[key].(string); ok && v != "" { + idField, vettingID = key, v + break + } + } + if objErr != nil || vettingID == "" { + format, _ := cmdutil.OutputFlags(cmd) + if writeErr := output.Stdout(format, env.Data); writeErr != nil { + cmd.PrintErrln(fmt.Sprintf("writing response: %v", writeErr)) + } + return nil, "", fmt.Errorf("vetting response did not include a vetting ID") + } + + receipt = map[string]any{ + idField: vettingID, + "brandId": brandID, + "status": "accepted", + "check": "band tendlc vetting list " + brandID, + } + return receipt, vettingID, nil +} + +// fetchVetting adapts a ListVettings call into pollTarget.Fetch. There is no +// GET .../vettings/{id} — the vettings list is the only read surface for a +// vetting's status — so --wait re-lists (walking every page) and returns the +// entry whose ID, under either vettingIDKeys name, matches vettingID. +func fetchVetting(svc *tendlcsvc.Service, brandID, vettingID string) func() (map[string]any, bool, error) { + return func() (map[string]any, bool, error) { + const pageSize = 100 + offset := 0 + for { + env, err := svc.ListVettings(brandID, pageSize, offset) + if err != nil { + if isNotFound(err) { + return nil, false, nil + } + return nil, false, err + } + items, err := env.List() + if err != nil { + return nil, false, err + } + for _, item := range items { + obj, ok := item.(map[string]any) + if !ok { + continue + } + for _, key := range vettingIDKeys { + if v, _ := obj[key].(string); v == vettingID { + return obj, true, nil + } + } + } + offset += len(items) + if len(items) == 0 || env.Page == nil || !env.Page.Truncated(offset) { + return nil, false, nil + } + } + } +} + +// classifyVettingObj adapts ClassifyVetting to pollTarget.Classify. +func classifyVettingObj(obj map[string]any) tendlcsvc.StateClass { + status, _ := obj["vettingStatus"].(string) + return tendlcsvc.ClassifyVetting(status) +} + +// vettingPollInterval mirrors brandCreatePollInterval: vetting decisions run +// through an external provider, not a Bandwidth-internal process, so a +// slower interval than the 2-second polls used elsewhere in the CLI is +// appropriate here too. +const vettingPollInterval = 5 * time.Second + +var ( + vettingListLimit int + vettingListOffset int + vettingListAll bool +) + +func init() { + f := vettingListCmd.Flags() + f.IntVar(&vettingListLimit, "limit", 50, "Page size") + f.IntVar(&vettingListOffset, "offset", 0, "Pagination offset") + f.BoolVar(&vettingListAll, "all", false, "Fetch every page (cannot be combined with --offset)") + vettingCmd.AddCommand(vettingListCmd) +} + +var vettingListCmd = &cobra.Command{ + Use: "list ", + Short: "List the external vettings recorded against a brand", + Long: `Lists the external vettings recorded against a brand. + +The positional here is a BRAND ID. Vettings are brand-scoped; there is no +campaign vetting endpoint.`, + Example: ` band tendlc vetting list BGJR2BA --plain + band tendlc vetting list BGJR2BA --all --plain`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if vettingListAll && cmd.Flags().Changed("offset") { + return cmdutil.NewFlagError("--all fetches every page, so it cannot be combined with --offset") + } + svc, err := service(cmd) + if err != nil { + return err + } + format, plain := cmdutil.OutputFlags(cmd) + + if !vettingListAll { + env, err := svc.ListVettings(args[0], vettingListLimit, vettingListOffset) + if err != nil { + return roleGateError(err, "Campaign Management") + } + items, err := env.List() + if err != nil { + return err + } + warnIfTruncated(cmd, env, vettingListOffset, len(items), "vettings") + return output.StdoutPlainList(format, plain, items) + } + + var all []any + err = api.ForEachPage(func(limit, offset int) (*api.Envelope, error) { + return svc.ListVettings(args[0], limit, offset) + }, vettingListLimit, func(batch []any) error { + all = append(all, batch...) + return nil + }) + if err != nil { + return roleGateError(err, "Campaign Management") + } + if all == nil { + all = []any{} + } + return output.StdoutPlainList(format, plain, all) + }, +} + +var ( + vettingRequestEvp string + vettingRequestClass string + vettingRequestConfirm bool + vettingRequestWait bool + vettingRequestTimeout int +) + +func init() { + f := vettingRequestCmd.Flags() + f.StringVar(&vettingRequestEvp, "evp", "", "External vetting provider: AEGIS, CV, WMC (required)") + f.StringVar(&vettingRequestClass, "class", "", "Vetting class: STANDARD, ENHANCED, POLITICAL, AUTHPLUS, RCS (required)") + f.BoolVar(&vettingRequestConfirm, "confirm", false, "Required. Confirms the billable order with the external vetting provider.") + f.BoolVar(&vettingRequestWait, "wait", false, "Block until the vetting reaches a terminal state") + f.IntVar(&vettingRequestTimeout, "timeout", 300, "Seconds to wait when --wait is set") + vettingCmd.AddCommand(vettingRequestCmd) +} + +var vettingRequestCmd = &cobra.Command{ + Use: "request ", + Short: "Order a new external vetting for a brand", + Long: `Orders a new third-party vetting for a BRAND — the positional here is a +brand ID, not a vetting ID; vettings are brand-scoped and there is no +campaign vetting endpoint. + +--class accepts STANDARD, ENHANCED, POLITICAL, AUTHPLUS, and RCS. RCS is not +in the published enumVettingClass, but production accepts it — confirmed by +pairing each class with an invalid evpId and observing which produced a +class-level error. + +This places a billable order with an external vetting provider, so +--confirm is required.`, + Example: ` band tendlc vetting request BGJR2BA --evp AEGIS --class STANDARD --confirm --plain + band tendlc vetting request BGJR2BA --evp AEGIS --class STANDARD --confirm --wait --plain`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := validateVettingRequest(vettingRequestEvp, vettingRequestClass); err != nil { + return err + } + brandID := args[0] + if err := requireConfirm(vettingRequestConfirm, + fmt.Sprintf("requesting a %s vetting from %s for brand %s is a billable order placed with "+ + "an external vetting provider. Pass --confirm to proceed.", + vettingRequestClass, vettingRequestEvp, brandID)); err != nil { + return err + } + + svc, err := service(cmd) + if err != nil { + return err + } + + body := map[string]any{ + "evpId": vettingRequestEvp, + "vettingClass": vettingRequestClass, + } + env, err := svc.RequestVetting(brandID, body) + if err != nil { + return roleGateError(err, "Campaign Management") + } + + receipt, vettingID, err := buildVettingReceipt(cmd, env, brandID) + if err != nil { + return err + } + + if !vettingRequestWait { + format, _ := cmdutil.OutputFlags(cmd) + return output.Stdout(format, receipt) + } + + target := pollTarget{ + Noun: "vetting", + Fetch: fetchVetting(svc, brandID, vettingID), + Classify: classifyVettingObj, + } + return awaitTerminal(cmd, target, receipt, time.Duration(vettingRequestTimeout)*time.Second, vettingPollInterval) + }, +} + +var ( + vettingImportEvp string + vettingImportVettingToken string + vettingImportWait bool + vettingImportTimeout int +) + +func init() { + f := vettingImportCmd.Flags() + f.StringVar(&vettingImportEvp, "evp", "", "External vetting provider that performed the vetting: AEGIS, CV, WMC (required)") + f.StringVar(&vettingImportVettingToken, "vetting-token", "", "Token issued by the vetting provider, if one exists") + f.BoolVar(&vettingImportWait, "wait", false, "Block until the vetting reaches a terminal state") + f.IntVar(&vettingImportTimeout, "timeout", 300, "Seconds to wait when --wait is set") + vettingCmd.AddCommand(vettingImportCmd) +} + +var vettingImportCmd = &cobra.Command{ + Use: "import ", + Short: "Record an already-performed external vetting against a brand", + Long: `Records a vetting that was already performed outside Bandwidth against a +BRAND. The first positional is the brand ID; the second is the vetting ID +assigned by the external provider. + +Recording an already-performed vetting is not billable, so unlike 'vetting +request' this takes no --confirm.`, + Example: ` band tendlc vetting import BGJR2BA V123 --evp AEGIS --plain + band tendlc vetting import BGJR2BA V123 --evp AEGIS --vetting-token TOK123 --plain`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + if vettingImportEvp == "" { + return cmdutil.NewMissingFlagsError([]string{"evp"}) + } + if !isValidEnum(vettingImportEvp, vettingEvpProviders) { + return cmdutil.NewFlagError("--evp must be one of: " + strings.Join(vettingEvpProviders, ", ")) + } + + brandID, vettingID := args[0], args[1] + + svc, err := service(cmd) + if err != nil { + return err + } + + body := map[string]any{"evpId": vettingImportEvp} + if cmd.Flags().Changed("vetting-token") { + body["vettingToken"] = vettingImportVettingToken + } + + env, err := svc.ImportVetting(brandID, vettingID, body) + if err != nil { + return roleGateError(err, "Campaign Management") + } + + receipt, respVettingID, err := buildVettingReceipt(cmd, env, brandID) + if err != nil { + return err + } + // The response's own ID may differ in name from what the caller passed + // (bandwidthId vs vettingBandwidthId), so the response's value — not the + // positional — is what --wait polls for. respVettingID is always + // non-empty here: buildVettingReceipt errors (and this function has + // already returned) whenever it would otherwise come back empty. + + if !vettingImportWait { + format, _ := cmdutil.OutputFlags(cmd) + return output.Stdout(format, receipt) + } + + target := pollTarget{ + Noun: "vetting", + Fetch: fetchVetting(svc, brandID, respVettingID), + Classify: classifyVettingObj, + } + return awaitTerminal(cmd, target, receipt, time.Duration(vettingImportTimeout)*time.Second, vettingPollInterval) + }, +} diff --git a/cmd/tendlc/vetting_test.go b/cmd/tendlc/vetting_test.go new file mode 100644 index 0000000..cc37019 --- /dev/null +++ b/cmd/tendlc/vetting_test.go @@ -0,0 +1,417 @@ +package tendlc + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// stubVettingList answers GET .../vettings with one ACTIVE vetting on a +// single, non-truncated page. +func stubVettingList(t *testing.T) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[{"bandwidthId":"V1","vettingStatus":"ACTIVE"}],` + + `"page":{"pageNumber":0,"pageSize":50,"totalElements":1,"totalPages":1}}`)) + }) +} + +// stubVettingListTruncated answers with a page reporting more records exist +// than were returned, so warnIfTruncated fires. +func stubVettingListTruncated(t *testing.T) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[{"bandwidthId":"V1","vettingStatus":"ACTIVE"}],` + + `"page":{"pageNumber":0,"pageSize":1,"totalElements":5,"totalPages":5}}`)) + }) +} + +// stubVettingRequestCapturing answers POST .../vettings with a 202 carrying +// idKey:idValue, and records each request's raw JSON body and path. +func stubVettingRequestCapturing(t *testing.T, idKey, idValue string) (*httptest.Server, *[]string, *[]string) { + var bodies, paths []string + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + b, _ := io.ReadAll(r.Body) + bodies = append(bodies, string(b)) + w.WriteHeader(http.StatusAccepted) + _, _ = fmt.Fprintf(w, `{"data":{%q:%q}}`, idKey, idValue) + }) + return srv, &bodies, &paths +} + +// stubVettingImportCapturing answers PUT .../vettings/{id} with a 202 +// carrying idKey:idValue, recording each request's body and path. +func stubVettingImportCapturing(t *testing.T, idKey, idValue string) (*httptest.Server, *[]string, *[]string) { + var bodies, paths []string + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + b, _ := io.ReadAll(r.Body) + bodies = append(bodies, string(b)) + w.WriteHeader(http.StatusAccepted) + _, _ = fmt.Fprintf(w, `{"data":{%q:%q}}`, idKey, idValue) + }) + return srv, &bodies, &paths +} + +// stubVettingRequestThenPoll answers POST .../vettings with a 202 carrying +// idKey:idValue and GET .../vettings with a single list entry whose +// bandwidthId matches idValue and whose vettingStatus is status — enough for +// a --wait test that settles on the very first poll. +func stubVettingRequestThenPoll(t *testing.T, idKey, idValue, status string) *httptest.Server { + return newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + w.WriteHeader(http.StatusAccepted) + _, _ = fmt.Fprintf(w, `{"data":{%q:%q}}`, idKey, idValue) + return + } + _, _ = fmt.Fprintf(w, `{"data":[{"bandwidthId":%q,"vettingStatus":%q}],`+ + `"page":{"pageNumber":0,"pageSize":50,"totalElements":1,"totalPages":1}}`, idValue, status) + }) +} + +// Test 1: missing --evp and --class aggregate into one exit-6 error naming +// both, and no request is made — runBrandCmd's `service` seam Fatals if +// invoked with a nil server. +func TestVettingRequestMissingRequiredFlagsAggregate(t *testing.T) { + _, _, err := runBrandCmd(t, nil, "vetting", "request", "BGJR2BA", "--confirm") + if err == nil { + t.Fatal("want an error for missing --evp and --class") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + if !strings.Contains(err.Error(), "evp") || !strings.Contains(err.Error(), "class") { + t.Errorf("error = %q, want it to name both missing flags", err.Error()) + } +} + +// Test 2: --class RCS is accepted. This is the regression guard for the +// undocumented enum value — production accepts RCS despite its absence from +// the published enumVettingClass, and this must not be "corrected" away. +func TestVettingRequestAcceptsRCSClass(t *testing.T) { + srv, bodies, _ := stubVettingRequestCapturing(t, "vettingBandwidthId", "V1") + _, _, err := runBrandCmd(t, srv, "vetting", "request", "BGJR2BA", + "--evp", "AEGIS", "--class", "RCS", "--confirm") + if err != nil { + t.Fatalf("vetting request --class RCS: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want exactly one POST, got %d", len(*bodies)) + } + var sent map[string]any + if err := json.Unmarshal([]byte((*bodies)[0]), &sent); err != nil { + t.Fatalf("request body is not JSON: %v", err) + } + if sent["vettingClass"] != "RCS" { + t.Errorf("posted body = %v, want vettingClass RCS", sent) + } +} + +// Test 3: an invalid --class exits 6 listing the valid classes, with zero +// requests made. +func TestVettingRequestRejectsInvalidClass(t *testing.T) { + _, _, err := runBrandCmd(t, nil, "vetting", "request", "BGJR2BA", + "--evp", "AEGIS", "--class", "NOT_A_CLASS", "--confirm") + if err == nil { + t.Fatal("want an error for an invalid --class") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + for _, want := range []string{"STANDARD", "ENHANCED", "POLITICAL", "AUTHPLUS", "RCS"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error = %q, want it to list valid class %q", err.Error(), want) + } + } +} + +// Test 4: an invalid --evp exits 6 listing the valid providers, with zero +// requests made. +func TestVettingRequestRejectsInvalidEvp(t *testing.T) { + _, _, err := runBrandCmd(t, nil, "vetting", "request", "BGJR2BA", + "--evp", "NOPE", "--class", "STANDARD", "--confirm") + if err == nil { + t.Fatal("want an error for an invalid --evp") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + for _, want := range []string{"AEGIS", "CV", "WMC"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error = %q, want it to list valid provider %q", err.Error(), want) + } + } +} + +// Test 4b: a missing --evp combined with an invalid --class combines into +// one error naming both. This is validateVettingRequest's one-missing/ +// one-invalid aggregation branch — the same defect class Tasks 2 and 3 both +// shipped and both got a regression guard for; this was the one instance +// left unguarded at the final whole-branch review (item C3). +func TestVettingRequestAggregatesOneMissingOneInvalidFlag(t *testing.T) { + _, _, err := runBrandCmd(t, nil, "vetting", "request", "BGJR2BA", + "--class", "NOT_A_CLASS", "--confirm") + if err == nil { + t.Fatal("want an error for a missing --evp combined with an invalid --class") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + if !strings.Contains(err.Error(), "STANDARD") { + t.Errorf("error = %q, want it to list the valid classes (the invalid --class violation)", err.Error()) + } + if !strings.Contains(err.Error(), "--evp") { + t.Errorf("error = %q, want it to also name the missing --evp flag", err.Error()) + } +} + +// Test 5: vetting request without --confirm exits 6, makes ZERO HTTP +// requests, and the message mentions the order is billable. +func TestVettingRequestWithoutConfirmMakesNoRequests(t *testing.T) { + _, _, err := runBrandCmd(t, nil, "vetting", "request", "BGJR2BA", "--evp", "AEGIS", "--class", "STANDARD") + if err == nil { + t.Fatal("want an error when --confirm is missing") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitFlagError) + } + if !strings.Contains(err.Error(), "billable") { + t.Errorf("error = %q, want it to mention the order is billable", err.Error()) + } +} + +// Test 6: with --confirm, request POSTs {"evpId":..., "vettingClass":...} and +// prints the 202 receipt on stdout. +func TestVettingRequestWithConfirmPostsBodyAndPrintsReceipt(t *testing.T) { + srv, bodies, paths := stubVettingRequestCapturing(t, "vettingBandwidthId", "V1") + out, _, err := runBrandCmd(t, srv, "vetting", "request", "BGJR2BA", + "--evp", "AEGIS", "--class", "STANDARD", "--confirm") + if err != nil { + t.Fatalf("vetting request --confirm: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want exactly one POST, got %d", len(*bodies)) + } + if !strings.HasSuffix((*paths)[0], "/brands/BGJR2BA/vettings") { + t.Errorf("path = %q, want POST to .../brands/BGJR2BA/vettings", (*paths)[0]) + } + var sent map[string]any + if err := json.Unmarshal([]byte((*bodies)[0]), &sent); err != nil { + t.Fatalf("request body is not JSON: %v", err) + } + if sent["evpId"] != "AEGIS" || sent["vettingClass"] != "STANDARD" { + t.Errorf("posted body = %v, want evpId AEGIS and vettingClass STANDARD", sent) + } + got := decodeStdout(t, out) + if got["vettingBandwidthId"] != "V1" { + t.Errorf("stdout = %v, want vettingBandwidthId V1", got) + } + if got["status"] != "accepted" { + t.Errorf("stdout = %v, want status accepted", got) + } +} + +// Test 7: import PUTs to /vettings/{vettingId} with {"evpId": ...}, and +// includes vettingToken only when the flag was actually passed. +func TestVettingImportPutsEvpAndOnlyIncludesTokenWhenPassed(t *testing.T) { + t.Run("without token", func(t *testing.T) { + srv, bodies, paths := stubVettingImportCapturing(t, "bandwidthId", "V1") + _, _, err := runBrandCmd(t, srv, "vetting", "import", "BGJR2BA", "V1", "--evp", "CV") + if err != nil { + t.Fatalf("vetting import: %v", err) + } + if len(*bodies) != 1 { + t.Fatalf("want exactly one PUT, got %d", len(*bodies)) + } + if !strings.HasSuffix((*paths)[0], "/brands/BGJR2BA/vettings/V1") { + t.Errorf("path = %q, want PUT to .../brands/BGJR2BA/vettings/V1", (*paths)[0]) + } + var sent map[string]any + if err := json.Unmarshal([]byte((*bodies)[0]), &sent); err != nil { + t.Fatalf("request body is not JSON: %v", err) + } + if sent["evpId"] != "CV" { + t.Errorf("posted body = %v, want evpId CV", sent) + } + if _, present := sent["vettingToken"]; present { + t.Errorf("posted body = %v, want no vettingToken key when --vetting-token was not passed", sent) + } + }) + + t.Run("with token", func(t *testing.T) { + srv, bodies, _ := stubVettingImportCapturing(t, "bandwidthId", "V1") + _, _, err := runBrandCmd(t, srv, "vetting", "import", "BGJR2BA", "V1", + "--evp", "CV", "--vetting-token", "TOK123") + if err != nil { + t.Fatalf("vetting import --vetting-token: %v", err) + } + var sent map[string]any + if err := json.Unmarshal([]byte((*bodies)[0]), &sent); err != nil { + t.Fatalf("request body is not JSON: %v", err) + } + if sent["vettingToken"] != "TOK123" { + t.Errorf("posted body = %v, want vettingToken TOK123", sent) + } + }) +} + +// vetting import has no --confirm: recording an already-performed vetting is +// not billable, unlike 'vetting request'. This locks in that the command +// does not define the flag at all, mirroring +// TestBrandResend2FANeedsNoConfirm. +func TestVettingImportHasNoConfirmFlag(t *testing.T) { + if f := vettingImportCmd.Flags().Lookup("confirm"); f != nil { + t.Errorf("vettingImportCmd has a --confirm flag, want none: %+v", f) + } +} + +// Test 8: vetting list on a brand with one ACTIVE vetting prints it, and the +// truncation warning goes to stderr only. +func TestVettingListPrintsEntryAndWarnsOnTruncationViaStderrOnly(t *testing.T) { + out, _, err := runBrandCmd(t, stubVettingList(t), "vetting", "list", "BGJR2BA") + if err != nil { + t.Fatalf("vetting list: %v", err) + } + if !strings.Contains(out, "ACTIVE") || !strings.Contains(out, "V1") { + t.Errorf("stdout = %q, want the ACTIVE vetting entry", out) + } + + out, errOut, err := runBrandCmd(t, stubVettingListTruncated(t), "vetting", "list", "BGJR2BA", "--limit", "1") + if err != nil { + t.Fatalf("vetting list (truncated): %v", err) + } + if strings.Contains(out, "pass --all") { + t.Error("truncation warning leaked into stdout; stdout must stay parseable") + } + if !strings.Contains(errOut, "pass --all") { + t.Errorf("stderr should carry the truncation warning, got %q", errOut) + } +} + +// Test 9: every vetting command rejects the wrong positional count, before +// any request is made. +func TestVettingCommandsRejectWrongPositionalCount(t *testing.T) { + cases := [][]string{ + {"vetting", "list"}, + {"vetting", "list", "B1", "STRAY"}, + {"vetting", "request"}, + {"vetting", "request", "B1", "STRAY"}, + {"vetting", "import"}, + {"vetting", "import", "B1"}, + {"vetting", "import", "B1", "V1", "STRAY"}, + } + for _, args := range cases { + t.Run(strings.Join(args, " "), func(t *testing.T) { + if _, _, err := runBrandCmd(t, nil, args...); err == nil { + t.Fatal("want an argument error") + } + }) + } +} + +// Test 10: the 202 receipt preserves the API's own ID field name. A stub +// returning vettingBandwidthId (the spec's documented key for the accept) +// must produce a receipt carrying vettingBandwidthId — NOT renamed to +// bandwidthId (the key the vettings list uses for the same concept). +// Normalizing would be tidier and would be wrong: it would misreport which +// field the API actually sent. +func TestVettingRequestReceiptPreservesAPIFieldName(t *testing.T) { + srv, _, _ := stubVettingRequestCapturing(t, "vettingBandwidthId", "V-XYZ") + out, _, err := runBrandCmd(t, srv, "vetting", "request", "BGJR2BA", + "--evp", "AEGIS", "--class", "STANDARD", "--confirm") + if err != nil { + t.Fatalf("vetting request: %v", err) + } + got := decodeStdout(t, out) + if got["vettingBandwidthId"] != "V-XYZ" { + t.Errorf("stdout = %v, want vettingBandwidthId V-XYZ preserved under its own name", got) + } + if _, present := got["bandwidthId"]; present { + t.Errorf("stdout = %v, want no renamed bandwidthId key", got) + } +} + +// TestVettingRequestWaitPollsToActive exercises the --wait wiring end to end: +// the 202 returns vettingBandwidthId, but the vettings list (the only read +// surface for a vetting's status) reports the same ID under bandwidthId — +// the field-name quirk this command set preserves rather than normalizes. +// --wait must still match the two up by VALUE across the differently-named +// keys and print the final ACTIVE resource. +func TestVettingRequestWaitPollsToActive(t *testing.T) { + srv := stubVettingRequestThenPoll(t, "vettingBandwidthId", "V1", "ACTIVE") + out, _, err := runBrandCmd(t, srv, "vetting", "request", "BGJR2BA", + "--evp", "AEGIS", "--class", "STANDARD", "--confirm", "--wait", "--timeout", "5") + if err != nil { + t.Fatalf("vetting request --wait: %v", err) + } + got := decodeStdout(t, out) + if got["vettingStatus"] != "ACTIVE" { + t.Errorf("stdout = %v, want the final ACTIVE resource", got) + } + if got["bandwidthId"] != "V1" { + t.Errorf("stdout = %v, want bandwidthId V1 from the list entry", got) + } +} + +// TestVettingRoleGate403MapsToExitFour exercises roleGateError on all three +// vetting commands. +func TestVettingRoleGate403MapsToExitFour(t *testing.T) { + body := `{"errors":[{"description":"does not have access rights"}]}` + cases := []struct { + name string + args []string + }{ + {"list", []string{"vetting", "list", "BGJR2BA"}}, + {"request", []string{"vetting", "request", "BGJR2BA", "--evp", "AEGIS", "--class", "STANDARD", "--confirm"}}, + {"import", []string{"vetting", "import", "BGJR2BA", "V1", "--evp", "AEGIS"}}, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + _, _, err := runBrandCmd(t, stubBrandErr(t, 403, body), tt.args...) + if err == nil { + t.Fatal("want an error on 403") + } + if code := cmdutil.ExitCodeForError(err); code != cmdutil.ExitConflict { + t.Errorf("exit code = %d, want %d", code, cmdutil.ExitConflict) + } + }) + } +} + +// THE load-bearing regression test for buildVettingReceipt's no-vetting-ID +// fallback — the equivalent, in this file, of brand_create_test.go's +// TestBrandCreateNoBandwidthIDPrintsBodyWithKeyPreserved. A response carrying +// neither bandwidthId nor vettingBandwidthId must still print the real body +// as an OBJECT, preserving its single key — not unwrap it to a bare array. +// Before the fallback was switched from output.StdoutAuto to output.Stdout, +// --plain would run this through FlattenResponse, which unwraps ANY +// single-key map, silently dropping the "orders" key and printing a bare +// array instead of the object it came from. +func TestVettingRequestNoVettingIDPrintsBodyWithKeyPreserved(t *testing.T) { + srv := newBrandStub(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"data":{"orders":[{"orderId":"O1"}]}}`)) + }) + + out, _, err := runBrandCmd(t, srv, "vetting", "request", "BGJR2BA", + "--evp", "AEGIS", "--class", "STANDARD", "--confirm", "--plain") + if err == nil { + t.Fatal("want an error when the response carries no vetting ID") + } + if !strings.Contains(err.Error(), "vetting ID") { + t.Errorf("error = %q, want it to name the missing vetting ID", err.Error()) + } + got := decodeStdout(t, out) + orders, ok := got["orders"] + if !ok { + t.Fatalf("stdout = %q, want the \"orders\" key preserved, not unwrapped to a bare array", out) + } + if arr, ok := orders.([]any); !ok || len(arr) != 1 { + t.Errorf("stdout orders = %v, want a one-element array", orders) + } +} diff --git a/internal/tendlc/brandoptions.go b/internal/tendlc/brandoptions.go new file mode 100644 index 0000000..1fbda18 --- /dev/null +++ b/internal/tendlc/brandoptions.go @@ -0,0 +1,237 @@ +package tendlc + +import ( + "sort" + "strings" + + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// BrandTypes are the entity types the API accepts. +var BrandTypes = []string{"PRIVATE_PROFIT", "PUBLIC_PROFIT", "NON_PROFIT", "GOVERNMENT", "SOLE_PROPRIETOR"} + +// BrandCreateOptions is the flag surface of `band tendlc brand create`. +// +// Field-to-flag naming is mechanical: CustomerProfileID is --customer-profile-id. +// The one field the API has and this struct does not is `country`, which is +// derived server-side from CountryCodeA3. +type BrandCreateOptions struct { + CustomerProfileID string + BrandType string + DisplayName string + CompanyName string + Street string + City string + State string + PostalCode string + CountryCodeA3 string + Phone string + Email string + Vertical string + EIN string + + EINIssuingCountryCodeA3 string + Website string + StockSymbol string + StockExchange string + AltBusinessID string + AltBusinessIDType string + BusinessContactEmail string + FirstName string + LastName string + MobilePhone string + IPAddress string +} + +// commonRequired are required for every brand type, paired as {flag, value}. +func (o BrandCreateOptions) commonRequired() [][2]string { + return [][2]string{ + {"customer-profile-id", o.CustomerProfileID}, + {"display-name", o.DisplayName}, + {"street", o.Street}, + {"city", o.City}, + {"state", o.State}, + {"postal-code", o.PostalCode}, + {"country-code-a3", o.CountryCodeA3}, + {"phone", o.Phone}, + {"email", o.Email}, + } +} + +// registeredEntityRequiredFlags are additionally required for every brand +// type except SOLE_PROPRIETOR, whose rules are unobservable on any account we +// have. Shared with ValidateBrandUpdate (brandupdate.go) so the per-type tier +// is defined exactly once for both create and update. +var registeredEntityRequiredFlags = []string{"company-name", "vertical", "ein", "ein-issuing-country-code-a3"} + +// publicProfitRequiredFlags are the four fields the schema marks optional but +// that production requires for PUBLIC_PROFIT, layered on top of +// registeredEntityRequiredFlags. Shared with ValidateBrandUpdate for the same +// reason as registeredEntityRequiredFlags. +var publicProfitRequiredFlags = []string{"stock-symbol", "stock-exchange", "website", "business-contact-email"} + +// value returns the option value for a CLI flag name understood by the +// per-type requirement tiers above. Mirrors BrandUpdateOptions.value in +// brandupdate.go. +func (o BrandCreateOptions) value(flag string) string { + switch flag { + case "company-name": + return o.CompanyName + case "vertical": + return o.Vertical + case "ein": + return o.EIN + case "ein-issuing-country-code-a3": + return o.EINIssuingCountryCodeA3 + case "stock-symbol": + return o.StockSymbol + case "stock-exchange": + return o.StockExchange + case "website": + return o.Website + case "business-contact-email": + return o.BusinessContactEmail + } + return "" +} + +// registeredEntityRequired are additionally required for every type except +// SOLE_PROPRIETOR, whose rules are unobservable on any account we have. +func (o BrandCreateOptions) registeredEntityRequired() [][2]string { + pairs := make([][2]string, len(registeredEntityRequiredFlags)) + for i, f := range registeredEntityRequiredFlags { + pairs[i] = [2]string{f, o.value(f)} + } + return pairs +} + +// publicProfitRequired are the four fields the schema marks optional but that +// production requires for PUBLIC_PROFIT. +func (o BrandCreateOptions) publicProfitRequired() [][2]string { + pairs := make([][2]string, len(publicProfitRequiredFlags)) + for i, f := range publicProfitRequiredFlags { + pairs[i] = [2]string{f, o.value(f)} + } + return pairs +} + +// ValidateBrandCreate reports every missing required flag in one error, the +// way the API reports every violation in one 400. +// +// The matrix below was derived from the API's own validation errors, not from +// the schema — the schema's required list is wrong in both directions. Only +// the four types we could observe are validated; SOLE_PROPRIETOR is gated at +// the account level, so its field rules fire behind a type check we cannot get +// past, and inventing them would reject requests the API would accept. +func ValidateBrandCreate(o BrandCreateOptions) error { + var missing []string + var invalidBrandType bool + collect := func(pairs [][2]string) { + for _, p := range pairs { + if p[1] == "" { + missing = append(missing, p[0]) + } + } + } + + collect(o.commonRequired()) + + if o.BrandType == "" { + missing = append(missing, "brand-type") + } else if !validBrandType(o.BrandType) { + invalidBrandType = true + } else { + // Only collect per-type requirements if BrandType is valid + switch o.BrandType { + case "PRIVATE_PROFIT", "NON_PROFIT", "GOVERNMENT": + collect(o.registeredEntityRequired()) + case "PUBLIC_PROFIT": + collect(o.registeredEntityRequired()) + collect(o.publicProfitRequired()) + } + } + + // If brand type is invalid, combine the enum error with any missing flags. + // Sorted the same way cmdutil.NewMissingFlagsError sorts its own list, so + // the rendering of the same missing flags doesn't depend on whether the + // brand type also happened to be invalid. + if invalidBrandType { + enumMsg := "--brand-type must be one of: " + strings.Join(BrandTypes, ", ") + if len(missing) > 0 { + sorted := append([]string(nil), missing...) + sort.Strings(sorted) + prefixedMissing := make([]string, len(sorted)) + for i, f := range sorted { + prefixedMissing[i] = "--" + f + } + return cmdutil.NewFlagError(enumMsg + "; missing required flags: " + strings.Join(prefixedMissing, ", ")) + } + return cmdutil.NewFlagError(enumMsg) + } + + if len(missing) > 0 { + return cmdutil.NewMissingFlagsError(missing) + } + return nil +} + +func validBrandType(t string) bool { + for _, bt := range BrandTypes { + if bt == t { + return true + } + } + return false +} + +// BuildBrandCreateRequest builds the POST body, omitting anything unset. An +// empty string is omitted rather than sent: absence leaves the field unset, +// while "" is a value the API validates and rejects. +func BuildBrandCreateRequest(o BrandCreateOptions) map[string]any { + body := map[string]any{} + for _, p := range [][2]string{ + {"customerProfileId", o.CustomerProfileID}, + {"brandType", o.BrandType}, + {"displayName", o.DisplayName}, + {"companyName", o.CompanyName}, + {"street", o.Street}, + {"city", o.City}, + {"state", o.State}, + {"postalCode", o.PostalCode}, + {"countryCodeA3", o.CountryCodeA3}, + {"phone", o.Phone}, + {"email", o.Email}, + {"vertical", o.Vertical}, + {"ein", o.EIN}, + {"einIssuingCountryCodeA3", o.EINIssuingCountryCodeA3}, + {"website", o.Website}, + {"stockSymbol", o.StockSymbol}, + {"stockExchange", o.StockExchange}, + {"altBusinessId", o.AltBusinessID}, + {"altBusinessIdType", o.AltBusinessIDType}, + {"businessContactEmail", o.BusinessContactEmail}, + {"firstName", o.FirstName}, + {"lastName", o.LastName}, + {"mobilePhone", o.MobilePhone}, + {"ipAddress", o.IPAddress}, + } { + if p[1] != "" { + body[p[0]] = p[1] + } + } + return body +} + +// BuildBrandRefreshRequest re-pulls an existing brand from TCR. It is the same +// POST /brands endpoint as create; a body carrying only brandId is what makes +// it a refresh, so nothing else may be added here. +// +// brandID is sent in the BODY, not the path — the one command in this +// package that puts an identifier there instead of in the URL. Measured +// against production: `brand refresh WET8JUY8H0` (a bandwidthId, not a TCR +// brandId) resolved correctly and returned {bandwidthId: WET8JUY8H0, brandId: +// BGJR2BA}, so the "every command accepts either ID" claim holds here too — +// it was previously assumed, not verified, for this one command. +func BuildBrandRefreshRequest(brandID string) map[string]any { + return map[string]any{"brandId": brandID} +} diff --git a/internal/tendlc/brandoptions_test.go b/internal/tendlc/brandoptions_test.go new file mode 100644 index 0000000..7effc83 --- /dev/null +++ b/internal/tendlc/brandoptions_test.go @@ -0,0 +1,279 @@ +package tendlc + +import ( + "errors" + "strings" + "testing" + + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// commonValid returns options satisfying every field required for all brand +// types, so each test can knock out exactly one thing. +func commonValid() BrandCreateOptions { + return BrandCreateOptions{ + CustomerProfileID: "2H6qSHb8yLCm76Dw7TAA9W", + BrandType: "PRIVATE_PROFIT", + DisplayName: "Acme Corp", + Street: "1000 Bandwidth Way", + City: "Raleigh", + State: "NC", + PostalCode: "27606", + CountryCodeA3: "USA", + Phone: "+19195551234", + Email: "ops@acme.com", + CompanyName: "Acme Corporation Inc", + Vertical: "TECHNOLOGY", + EIN: "562242657", + EINIssuingCountryCodeA3: "USA", + } +} + +func TestValidateBrandCreateAcceptsCompleteOptions(t *testing.T) { + if err := ValidateBrandCreate(commonValid()); err != nil { + t.Fatalf("want valid, got %v", err) + } +} + +// The API reports every violation at once; so must the CLI. Reporting one +// missing flag per invocation turns a single fix into nine round trips. +func TestValidateBrandCreateReportsAllMissingCommonFieldsAtOnce(t *testing.T) { + err := ValidateBrandCreate(BrandCreateOptions{BrandType: "PRIVATE_PROFIT"}) + if err == nil { + t.Fatal("want an error, got nil") + } + var fe *cmdutil.FlagError + if !errors.As(err, &fe) { + t.Fatalf("want a FlagError (exit 6), got %T", err) + } + for _, want := range []string{ + "--customer-profile-id", "--display-name", "--street", "--city", + "--state", "--postal-code", "--country-code-a3", "--phone", "--email", + } { + if !strings.Contains(err.Error(), want) { + t.Errorf("error missing %s: %s", want, err.Error()) + } + } +} + +// There is no --country flag: country is derived server-side from +// countryCodeA3. Measured 2026-08-18 — a create with countryCodeA3 and no +// country succeeds. +func TestValidateBrandCreateDoesNotRequireCountry(t *testing.T) { + if err := ValidateBrandCreate(commonValid()); err != nil { + t.Fatalf("want valid without a country field, got %v", err) + } + body := BuildBrandCreateRequest(commonValid()) + if _, present := body["country"]; present { + t.Error("body must not carry a country field; the API derives it") + } +} + +func TestValidateBrandCreatePerTypeRequirements(t *testing.T) { + tests := []struct { + name string + mutate func(*BrandCreateOptions) + wantFlags []string + wantOK bool + }{ + { + name: "PRIVATE_PROFIT needs company/vertical/ein", + mutate: func(o *BrandCreateOptions) { + o.CompanyName, o.Vertical, o.EIN, o.EINIssuingCountryCodeA3 = "", "", "", "" + }, + wantFlags: []string{"--company-name", "--vertical", "--ein", "--ein-issuing-country-code-a3"}, + }, + { + name: "NON_PROFIT has the same extra requirements", + mutate: func(o *BrandCreateOptions) { + o.BrandType = "NON_PROFIT" + o.CompanyName, o.Vertical, o.EIN, o.EINIssuingCountryCodeA3 = "", "", "", "" + }, + wantFlags: []string{"--company-name", "--vertical", "--ein", "--ein-issuing-country-code-a3"}, + }, + { + name: "GOVERNMENT requires company-name, vertical, ein, ein-issuing-country-code-a3", + mutate: func(o *BrandCreateOptions) { + o.BrandType = "GOVERNMENT" + o.CompanyName, o.Vertical, o.EIN, o.EINIssuingCountryCodeA3 = "", "", "", "" + }, + wantFlags: []string{"--company-name", "--vertical", "--ein", "--ein-issuing-country-code-a3"}, + }, + { + name: "GOVERNMENT does not require website", + mutate: func(o *BrandCreateOptions) { + o.BrandType = "GOVERNMENT" + o.Website = "" + }, + wantOK: true, + }, + { + name: "PUBLIC_PROFIT needs four more schema-optional fields", + mutate: func(o *BrandCreateOptions) { + o.BrandType = "PUBLIC_PROFIT" + }, + wantFlags: []string{"--stock-symbol", "--stock-exchange", "--website", "--business-contact-email"}, + }, + { + name: "PUBLIC_PROFIT passes once those four are set", + mutate: func(o *BrandCreateOptions) { + o.BrandType = "PUBLIC_PROFIT" + o.StockSymbol, o.StockExchange = "BAND", "NASDAQ" + o.Website, o.BusinessContactEmail = "https://acme.com", "cfo@acme.com" + }, + wantOK: true, + }, + { + // SOLE_PROPRIETOR is account-gated; its field rules could not be + // observed on any account available to us. Guessing them is how a + // validator ends up rejecting what the API would accept, so the CLI + // checks the common fields and lets the API speak for the rest. + name: "SOLE_PROPRIETOR skips type-specific validation", + mutate: func(o *BrandCreateOptions) { + o.BrandType = "SOLE_PROPRIETOR" + o.CompanyName, o.Vertical, o.EIN, o.EINIssuingCountryCodeA3 = "", "", "", "" + }, + wantOK: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := commonValid() + tt.mutate(&o) + err := ValidateBrandCreate(o) + if tt.wantOK { + if err != nil { + t.Fatalf("want valid, got %v", err) + } + return + } + if err == nil { + t.Fatal("want an error, got nil") + } + for _, want := range tt.wantFlags { + if !strings.Contains(err.Error(), want) { + t.Errorf("error missing %s: %s", want, err.Error()) + } + } + }) + } +} + +func TestValidateBrandCreateRejectsUnknownBrandType(t *testing.T) { + o := commonValid() + o.BrandType = "PRIVATE" + err := ValidateBrandCreate(o) + if err == nil { + t.Fatal("want an error for an unknown brand type") + } + if !strings.Contains(err.Error(), "PRIVATE_PROFIT") { + t.Errorf("error should list the valid types, got: %s", err.Error()) + } +} + +func TestValidateBrandCreateRequiresBrandType(t *testing.T) { + o := commonValid() + o.BrandType = "" + err := ValidateBrandCreate(o) + if err == nil || !strings.Contains(err.Error(), "--brand-type") { + t.Fatalf("want a --brand-type error, got %v", err) + } +} + +// Regression test: invalid brand type must not suppress other missing-field violations. +// BrandCreateOptions{BrandType: "PRIVATE"} (a typo) should report the invalid type +// AND the missing common fields in a single error. +func TestValidateBrandCreateAggregatesViolationsWithInvalidBrandType(t *testing.T) { + o := BrandCreateOptions{BrandType: "PRIVATE"} + err := ValidateBrandCreate(o) + if err == nil { + t.Fatal("want an error for invalid brand type and missing fields") + } + // Should mention the invalid brand type and list valid options + if !strings.Contains(err.Error(), "PRIVATE_PROFIT") { + t.Errorf("error should list valid types, got: %s", err.Error()) + } + // Should also mention at least some missing common fields + for _, want := range []string{"--customer-profile-id", "--display-name"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error missing %s: %s", want, err.Error()) + } + } +} + +// Regression test: the combined "invalid brand type + missing flags" error +// must render its missing-flag list in the same sorted order +// cmdutil.NewMissingFlagsError uses for the valid-type case — AGENTS.md +// documents the sorted rendering, and the same set of missing flags must not +// change order depending only on whether the brand type also happened to be +// invalid. +func TestValidateBrandCreateSortsMissingFlagsEvenWithInvalidBrandType(t *testing.T) { + err := ValidateBrandCreate(BrandCreateOptions{BrandType: "PRIVATE"}) + if err == nil { + t.Fatal("want an error for invalid brand type and missing fields") + } + msg := err.Error() + // commonRequired's flags, sorted alphabetically: city, country-code-a3, + // customer-profile-id, display-name, email, phone, postal-code, state, + // street. Assert each one's position is strictly increasing left to + // right — an implementation that reverted to collection order (the + // struct field order above) would fail this. + sortedFlags := []string{ + "--city", "--country-code-a3", "--customer-profile-id", "--display-name", + "--email", "--phone", "--postal-code", "--state", "--street", + } + prevIdx := -1 + for _, flag := range sortedFlags { + idx := strings.Index(msg, flag) + if idx == -1 { + t.Fatalf("error = %q, want it to contain %q", msg, flag) + } + if idx < prevIdx { + t.Errorf("error = %q, want %q sorted after the previous flag (not in struct/collection order)", msg, flag) + } + prevIdx = idx + } +} + +func TestBuildBrandCreateRequestOmitsUnsetOptionalFields(t *testing.T) { + body := BuildBrandCreateRequest(commonValid()) + + if body["displayName"] != "Acme Corp" { + t.Errorf("displayName = %v", body["displayName"]) + } + if body["einIssuingCountryCodeA3"] != "USA" { + t.Errorf("einIssuingCountryCodeA3 = %v", body["einIssuingCountryCodeA3"]) + } + // Unset optionals are omitted, not sent as "". An empty string is a value + // the API validates; absence is not. + for _, k := range []string{"website", "stockSymbol", "stockExchange", "altBusinessId", + "altBusinessIdType", "businessContactEmail", "firstName", "lastName", + "mobilePhone", "ipAddress"} { + if _, present := body[k]; present { + t.Errorf("unset optional %q must be omitted, got %v", k, body[k]) + } + } +} + +func TestBuildBrandCreateRequestIncludesSetOptionalFields(t *testing.T) { + o := commonValid() + o.Website = "https://acme.com" + o.MobilePhone = "+19195559999" + body := BuildBrandCreateRequest(o) + + if body["website"] != "https://acme.com" { + t.Errorf("website = %v", body["website"]) + } + if body["mobilePhone"] != "+19195559999" { + t.Errorf("mobilePhone = %v", body["mobilePhone"]) + } +} + +// Refresh reuses POST /brands with a body carrying only brandId. Sending +// anything else would be read as a create. +func TestBuildBrandRefreshRequestSendsOnlyBrandID(t *testing.T) { + body := BuildBrandRefreshRequest("BGJR2BA") + if len(body) != 1 || body["brandId"] != "BGJR2BA" { + t.Errorf("body = %v, want exactly {brandId: BGJR2BA}", body) + } +} diff --git a/internal/tendlc/brandupdate.go b/internal/tendlc/brandupdate.go new file mode 100644 index 0000000..22ce81e --- /dev/null +++ b/internal/tendlc/brandupdate.go @@ -0,0 +1,355 @@ +package tendlc + +import ( + "fmt" + "sort" + "strings" + + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// brandReadOnlyFields are stripped before a PUT. +// +// Production accepts all of them without complaint — measured by PUTting a +// full 46-key read response back and getting exactly one error, for the field +// deliberately corrupted to force a 400. They are stripped anyway: sending a +// server-assigned value back is meaningless, and a field that is ignored today +// may be honored tomorrow. +// +// The strip list was also confirmed safe against the opposite failure mode: +// a live `brand update --website` on a real brand was diffed key-by-key +// before and after. All 46 keys were present both times, zero were dropped, +// zero were nulled, and only `website` changed — the server regenerates every +// derived/read-only field itself. So stripping this list does not risk +// silently nulling a caller-supplied field on this full-replacement PUT. +// +// country and einIssuingCountry are here because they are DERIVED from their +// …CodeA3 counterparts, not because the API rejects them. +// +// Note the absence of "version": unlike customer profiles, brands have no +// optimistic-locking token at all. +var brandReadOnlyFields = []string{ + "accounts", "bandwidthId", "brandId", "brandIdentityStatus", "brandRelationship", + "authenticationStatus", "businessContactEmailVerifiedDate", "createdDate", + "modifiedDate", "evpVettingScore", "imported", "universalEin", "country", + "einIssuingCountry", "russell3000", "governmentEntity", "section527", + "taxExemptStatus", "politicalCommitteeLocale", "referenceId", +} + +// BrandUpdateFieldFlags are every flag `brand update` accepts, in CLI naming. +// BuildBrandUpdateRequest keys its changed map on exactly these names. +// customerProfileId is absent: the update schema does not accept it, and a +// profile backs exactly one brand for the brand's whole life. +var BrandUpdateFieldFlags = []string{ + "brand-type", "display-name", "company-name", "street", "city", "state", + "postal-code", "country-code-a3", "phone", "email", "vertical", "ein", + "ein-issuing-country-code-a3", "website", "stock-symbol", "stock-exchange", + "alt-business-id", "alt-business-id-type", "business-contact-email", + "first-name", "last-name", "mobile-phone", "ip-address", +} + +// brandFlagToField maps a CLI flag name to the JSON key it writes. +var brandFlagToField = map[string]string{ + "brand-type": "brandType", "display-name": "displayName", + "company-name": "companyName", "street": "street", "city": "city", + "state": "state", "postal-code": "postalCode", "country-code-a3": "countryCodeA3", + "phone": "phone", "email": "email", "vertical": "vertical", "ein": "ein", + "ein-issuing-country-code-a3": "einIssuingCountryCodeA3", "website": "website", + "stock-symbol": "stockSymbol", "stock-exchange": "stockExchange", + "alt-business-id": "altBusinessId", "alt-business-id-type": "altBusinessIdType", + "business-contact-email": "businessContactEmail", "first-name": "firstName", + "last-name": "lastName", "mobile-phone": "mobilePhone", "ip-address": "ipAddress", +} + +// BrandUpdateOptions is the flag surface of `band tendlc brand update`. +// Whether a field was explicitly set is tracked separately in the changed map, +// because an empty string is a legitimate value meaning "clear this". +type BrandUpdateOptions struct { + BrandType string + DisplayName string + CompanyName string + Street string + City string + State string + PostalCode string + CountryCodeA3 string + Phone string + Email string + Vertical string + EIN string + + EINIssuingCountryCodeA3 string + Website string + StockSymbol string + StockExchange string + AltBusinessID string + AltBusinessIDType string + BusinessContactEmail string + FirstName string + LastName string + MobilePhone string + IPAddress string +} + +// value returns the option value for a CLI flag name. +func (o BrandUpdateOptions) value(flag string) string { + switch flag { + case "brand-type": + return o.BrandType + case "display-name": + return o.DisplayName + case "company-name": + return o.CompanyName + case "street": + return o.Street + case "city": + return o.City + case "state": + return o.State + case "postal-code": + return o.PostalCode + case "country-code-a3": + return o.CountryCodeA3 + case "phone": + return o.Phone + case "email": + return o.Email + case "vertical": + return o.Vertical + case "ein": + return o.EIN + case "ein-issuing-country-code-a3": + return o.EINIssuingCountryCodeA3 + case "website": + return o.Website + case "stock-symbol": + return o.StockSymbol + case "stock-exchange": + return o.StockExchange + case "alt-business-id": + return o.AltBusinessID + case "alt-business-id-type": + return o.AltBusinessIDType + case "business-contact-email": + return o.BusinessContactEmail + case "first-name": + return o.FirstName + case "last-name": + return o.LastName + case "mobile-phone": + return o.MobilePhone + case "ip-address": + return o.IPAddress + } + return "" +} + +// BuildBrandUpdateRequest produces a full-replacement PUT body that cannot +// drop fields the CLI does not model. +// +// PUT replaces the whole resource, so anything missing from the body is nulled +// server-side. Building the body from a typed struct would therefore delete +// every production field we never modeled — and the brand response already +// carries several the published schema omits. So the body starts as a copy of +// what the API just gave us, read-only fields are removed, and only +// explicitly-changed flags are overlaid. Validation stays typed; the payload +// stays lossless. +func BuildBrandUpdateRequest(current map[string]any, o BrandUpdateOptions, changed map[string]bool) (map[string]any, error) { + if current == nil { + return nil, fmt.Errorf("no current resource to update from") + } + + body := deepCopyBrandMap(current) + for _, ro := range brandReadOnlyFields { + delete(body, ro) + } + + for _, flag := range BrandUpdateFieldFlags { + if !changed[flag] { + continue + } + field := brandFlagToField[flag] + if v := o.value(flag); v != "" { + body[field] = v + } else { + // An explicitly empty value clears the field. It must go over the + // wire as JSON null: the API rejects "" on at least one field and + // accepts null. + body[field] = nil + } + } + + if err := ValidateBrandUpdate(body); err != nil { + return nil, err + } + return body, nil +} + +// ValidateBrandUpdate checks the fully overlaid PUT body — the object about to +// go over the wire — not the options struct. Catching a cleared required field +// here makes the failure a local, zero-request FlagError (exit 6) rather than +// a raw 400 from the API. +// +// The universal fields (required on every brand type) are checked first. On +// top of those, the same per-type tier ValidateBrandCreate enforces +// (registeredEntityRequiredFlags for every type except SOLE_PROPRIETOR; +// registeredEntityRequiredFlags + publicProfitRequiredFlags for +// PUBLIC_PROFIT, both from brandoptions.go, reused rather than duplicated) +// applies here too — clearing --vertical on a PRIVATE_PROFIT brand or +// --website on a PUBLIC_PROFIT one is exactly the mistake ValidateBrandCreate +// already catches on create, and update must catch it the same way rather +// than letting it reach the API as a raw 400. brandType is read from the +// completed BODY, not from the options struct: the consequence attaches to +// what the brand IS right now, the same reasoning IdentityFieldsChanged uses. +// SOLE_PROPRIETOR still skips the per-type tier — its rules are +// account-gated and unobservable on any account we have, and inventing them +// would reject requests production accepts. The tier is also skipped +// whenever brandType is itself missing or invalid: which tier would apply +// isn't known. +// +// brandType also gets the same enum check ValidateBrandCreate runs via +// validBrandType: without it, a --brand-type typo exits 6 on create but +// reaches the API and comes back as a raw 400 on update. As in +// ValidateBrandCreate, an invalid brand type does not short-circuit — it is +// aggregated with any cleared required fields (universal AND per-type) into +// one error, the way the API reports every violation in one 400. +func ValidateBrandUpdate(body map[string]any) error { + required := [][2]string{ + {"brand-type", "brandType"}, + {"display-name", "displayName"}, + {"street", "street"}, + {"city", "city"}, + {"state", "state"}, + {"postal-code", "postalCode"}, + {"country-code-a3", "countryCodeA3"}, + {"phone", "phone"}, + {"email", "email"}, + } + var cleared []string + var invalidBrandType bool + brandTypeMissing := false + brandType := "" + for _, p := range required { + s, ok := body[p[1]].(string) + if !ok || s == "" { + cleared = append(cleared, p[0]) + if p[1] == "brandType" { + brandTypeMissing = true + } + continue + } + if p[1] == "brandType" { + brandType = s + if !validBrandType(s) { + invalidBrandType = true + } + } + } + + if !brandTypeMissing && !invalidBrandType { + switch brandType { + case "PRIVATE_PROFIT", "NON_PROFIT", "GOVERNMENT": + cleared = append(cleared, clearedTierFields(body, registeredEntityRequiredFlags)...) + case "PUBLIC_PROFIT": + cleared = append(cleared, clearedTierFields(body, registeredEntityRequiredFlags)...) + cleared = append(cleared, clearedTierFields(body, publicProfitRequiredFlags)...) + // SOLE_PROPRIETOR (and any other valid type): no per-type tier to check. + } + } + + if invalidBrandType { + enumMsg := "--brand-type must be one of: " + strings.Join(BrandTypes, ", ") + if len(cleared) > 0 { + return cmdutil.NewFlagError(enumMsg + "; these fields are required on every brand and cannot be cleared: --" + + strings.Join(cleared, ", --")) + } + return cmdutil.NewFlagError(enumMsg) + } + + if len(cleared) > 0 { + return cmdutil.NewFlagError("these fields are required on every brand and cannot be cleared: --" + + strings.Join(cleared, ", --")) + } + return nil +} + +// clearedTierFields returns the flags in tier (a per-type requirement list +// from brandoptions.go) whose backing field is missing or empty in body — +// i.e. would clear a field production requires for the brand's current type +// on this full-replacement PUT. +func clearedTierFields(body map[string]any, tier []string) []string { + var out []string + for _, flag := range tier { + s, ok := body[brandFlagToField[flag]].(string) + if !ok || s == "" { + out = append(out, flag) + } + } + return out +} + +// identityFlags are the flags whose change can trigger a $4 fee, reset the +// brand's identity status, or revoke Auth+ compliance. +// +// Fee detection is not attempted: whether the API actually charges depends on +// active campaigns and active vettings, neither present in the brand detail +// response. Determining it would need two more reads, a definition of +// "active", and would still race. So the contract is over-asking — --confirm +// is required whenever one of these is explicitly changed, charged or not. +// Over-asking is safe; under-asking bills the customer. +var identityFlags = []string{ + "company-name", "brand-type", "ein", "ein-issuing-country-code-a3", "mobile-phone", +} + +// IdentityFieldsChanged returns the identity-affecting flags the caller +// explicitly set, sorted, or nil when none were. +// +// business-contact-email is conditional: changing it revokes Auth+ compliance +// on PUBLIC_PROFIT brands only, so it counts only when the CURRENT brand is +// PUBLIC_PROFIT. Reading brandType from current, not from the options, is +// deliberate — the consequence attaches to what the brand is right now. +func IdentityFieldsChanged(current map[string]any, changed map[string]bool) []string { + var out []string + for _, f := range identityFlags { + if changed[f] { + out = append(out, f) + } + } + if changed["business-contact-email"] { + if bt, _ := current["brandType"].(string); bt == "PUBLIC_PROFIT" { + out = append(out, "business-contact-email") + } + } + if len(out) == 0 { + return nil + } + sort.Strings(out) + return out +} + +// deepCopyBrandMap copies m so the result shares no mutable structure with it. +// current is read from an api.Envelope the caller may reuse, so a shallow copy +// would leave nested values (brand.accounts is a []any of maps) aliased +// between the outgoing body and the caller's data. +func deepCopyBrandMap(m map[string]any) map[string]any { + out := make(map[string]any, len(m)) + for k, v := range m { + out[k] = deepCopyBrandValue(v) + } + return out +} + +func deepCopyBrandValue(v any) any { + switch vv := v.(type) { + case map[string]any: + return deepCopyBrandMap(vv) + case []any: + out := make([]any, len(vv)) + for i, e := range vv { + out[i] = deepCopyBrandValue(e) + } + return out + default: + return v + } +} diff --git a/internal/tendlc/brandupdate_test.go b/internal/tendlc/brandupdate_test.go new file mode 100644 index 0000000..7d3ed40 --- /dev/null +++ b/internal/tendlc/brandupdate_test.go @@ -0,0 +1,421 @@ +package tendlc + +import ( + "reflect" + "sort" + "strings" + "testing" +) + +// liveBrand mirrors the shape GET /brands/{id} actually returns, including +// read-only and undocumented keys. Tests overlay against this rather than a +// tidy hand-made map, because the whole point of RMW is surviving fields the +// CLI does not model. +func liveBrand() map[string]any { + return map[string]any{ + "accounts": []any{map[string]any{ + "accountId": "9901287", + "customerProfileId": "2H6qSHb8yLCm76Dw7TAA9W", + }}, + "altBusinessId": nil, + "altBusinessIdType": nil, + "authenticationStatus": nil, + "bandwidthId": "WET8JUY8H0", + "brandId": "BGJR2BA", + "brandIdentityStatus": "UNVERIFIED", + "brandRelationship": "MEDIUM_ACCOUNT", + "brandType": "PRIVATE_PROFIT", + "businessContactEmail": "kshah@bandwidth.com", + "businessContactEmailVerifiedDate": nil, + "city": "Raleigh", + "companyName": "Bandwidth Inc", + "country": "US", + "countryCodeA3": "USA", + "createdDate": "2026-05-28T20:34:05.480Z", + "displayName": "Bandwidth Acceptance Test", + "ein": "562242657", + "einIssuingCountry": "US", + "einIssuingCountryCodeA3": "USA", + "email": "npatel@bandwidth.com", + "evpVettingScore": nil, + "imported": false, + "ipAddress": nil, + "modifiedDate": "2026-06-17T19:37:16.929Z", + "phone": "+12025551234", + "postalCode": "27606", + "referenceId": "WET8JUY8H0", + "state": "NC", + "street": "1000 Bandwidth Way", + "universalEin": "US_562242657", + "vertical": "PROFESSIONAL", + "website": "https://bandwidth.com", + // A field the CLI does not model at all. It must survive the round trip. + "someFutureField": "keep me", + // These nested structures represent future API fields that the CLI does not + // model. They are not in brandReadOnlyFields, so they survive the strip step. + // deepCopyBrandValue must recursively copy them, or mutations in body will + // corrupt current — this is why deep copy is critical for this component. + "nestedMetadata": map[string]any{ + "key1": "value1", + "key2": 42, + }, + "nestedEntries": []any{ + map[string]any{ + "id": "entry1", + "count": 10, + }, + map[string]any{ + "id": "entry2", + "count": 20, + }, + }, + } +} + +// livePublicProfitBrand is liveBrand with brandType switched to PUBLIC_PROFIT +// and the two extra fields production requires only for that type +// (stockSymbol, stockExchange) filled in, so a test can clear one PUBLIC_PROFIT +// -specific field at a time without the other three already being "cleared" +// by omission. +func livePublicProfitBrand() map[string]any { + b := liveBrand() + b["brandType"] = "PUBLIC_PROFIT" + b["stockSymbol"] = "BAND" + b["stockExchange"] = "NASDAQ" + return b +} + +// liveSoleProprietorBrand is liveBrand with brandType switched to +// SOLE_PROPRIETOR — the one type ValidateBrandUpdate must never apply the +// registeredEntityRequiredFlags tier to, since those rules are account-gated +// and unobservable on any account available for testing. +func liveSoleProprietorBrand() map[string]any { + b := liveBrand() + b["brandType"] = "SOLE_PROPRIETOR" + return b +} + +// The defining property: changing one field must not disturb any other, +// including fields the CLI has never heard of. +func TestBuildBrandUpdateRequestIsLossless(t *testing.T) { + body, err := BuildBrandUpdateRequest(liveBrand(), + BrandUpdateOptions{DisplayName: "Renamed"}, + map[string]bool{"display-name": true}) + if err != nil { + t.Fatalf("BuildBrandUpdateRequest: %v", err) + } + + if body["displayName"] != "Renamed" { + t.Errorf("displayName = %v, want Renamed", body["displayName"]) + } + for k, want := range map[string]any{ + "companyName": "Bandwidth Inc", + "street": "1000 Bandwidth Way", + "website": "https://bandwidth.com", + "vertical": "PROFESSIONAL", + "someFutureField": "keep me", + } { + if body[k] != want { + t.Errorf("%s = %v, want %v (must survive an unrelated change)", k, body[k], want) + } + } +} + +func TestBuildBrandUpdateRequestStripsReadOnlyFields(t *testing.T) { + body, err := BuildBrandUpdateRequest(liveBrand(), + BrandUpdateOptions{DisplayName: "Renamed"}, + map[string]bool{"display-name": true}) + if err != nil { + t.Fatalf("BuildBrandUpdateRequest: %v", err) + } + for _, k := range []string{ + "accounts", "bandwidthId", "brandId", "brandIdentityStatus", "brandRelationship", + "authenticationStatus", "businessContactEmailVerifiedDate", "createdDate", + "modifiedDate", "evpVettingScore", "imported", "universalEin", "country", + "einIssuingCountry", "referenceId", + } { + if _, present := body[k]; present { + t.Errorf("read-only field %q must be stripped, got %v", k, body[k]) + } + } +} + +// Brands have no version field. A test asserting one would be asserting a +// customer-profile behavior that does not exist here. +func TestBuildBrandUpdateRequestDoesNotInventAVersion(t *testing.T) { + body, err := BuildBrandUpdateRequest(liveBrand(), + BrandUpdateOptions{DisplayName: "Renamed"}, + map[string]bool{"display-name": true}) + if err != nil { + t.Fatalf("BuildBrandUpdateRequest: %v", err) + } + if _, present := body["version"]; present { + t.Error("brands carry no version; the body must not add one") + } +} + +// The documented way to clear a field is passing the flag with an empty value. +// That must reach the wire as JSON null — the API rejects "" on at least one +// field and accepts null. +func TestBuildBrandUpdateRequestClearsWithNull(t *testing.T) { + body, err := BuildBrandUpdateRequest(liveBrand(), + BrandUpdateOptions{Website: ""}, + map[string]bool{"website": true}) + if err != nil { + t.Fatalf("BuildBrandUpdateRequest: %v", err) + } + v, present := body["website"] + if !present { + t.Fatal("website must be present as null, not omitted") + } + if v != nil { + t.Errorf("website = %v, want nil", v) + } +} + +func TestBuildBrandUpdateRequestDeepCopiesNestedValues(t *testing.T) { + current := liveBrand() + body, err := BuildBrandUpdateRequest(current, + BrandUpdateOptions{DisplayName: "Renamed"}, + map[string]bool{"display-name": true}) + if err != nil { + t.Fatalf("BuildBrandUpdateRequest: %v", err) + } + + // Mutate the nested map in body and verify current is unaffected. + // This would fail under a shallow copy because nestedMetadata is not in + // brandReadOnlyFields so it survives the strip step. The type assertions + // are unconditional: a missing or wrongly-typed key must fail the test + // loudly, not silently skip the mutation and the assertion that depends + // on it. + nestedMap, ok := body["nestedMetadata"].(map[string]any) + if !ok { + t.Fatalf("body[%q] = %#v (%T), want map[string]any", "nestedMetadata", body["nestedMetadata"], body["nestedMetadata"]) + } + nestedMap["key1"] = "mutated" + + currentNested, ok := current["nestedMetadata"].(map[string]any) + if !ok { + t.Fatalf("current[%q] = %#v (%T), want map[string]any", "nestedMetadata", current["nestedMetadata"], current["nestedMetadata"]) + } + if currentNested["key1"] != "value1" { + t.Error("mutating nested map in body changed the caller's nested map") + } + + // Mutate a nested entry in the nested array and verify current is unaffected. + // This exercises the []any recursion path. + nestedArray, ok := body["nestedEntries"].([]any) + if !ok || len(nestedArray) == 0 { + t.Fatalf("body[%q] = %#v, want a non-empty []any", "nestedEntries", body["nestedEntries"]) + } + nestedEntry, ok := nestedArray[0].(map[string]any) + if !ok { + t.Fatalf("body[%q][0] = %#v (%T), want map[string]any", "nestedEntries", nestedArray[0], nestedArray[0]) + } + nestedEntry["count"] = 999 + + currentArray, ok := current["nestedEntries"].([]any) + if !ok || len(currentArray) == 0 { + t.Fatalf("current[%q] = %#v, want a non-empty []any", "nestedEntries", current["nestedEntries"]) + } + currentEntry, ok := currentArray[0].(map[string]any) + if !ok { + t.Fatalf("current[%q][0] = %#v (%T), want map[string]any", "nestedEntries", currentArray[0], currentArray[0]) + } + if currentEntry["count"] != 10 { + t.Error("mutating nested array entry in body changed the caller's nested array") + } + + // The read map must also be untouched by the strip step. + if _, present := current["bandwidthId"]; !present { + t.Error("stripping read-only fields mutated the caller's map") + } +} + +func TestBuildBrandUpdateRequestRejectsNilCurrent(t *testing.T) { + if _, err := BuildBrandUpdateRequest(nil, BrandUpdateOptions{}, map[string]bool{}); err == nil { + t.Fatal("want an error for a nil current resource") + } +} + +// Validation runs on the completed body, not the options struct: a struct +// that looks fine alone can still combine with the read brand into something +// the API rejects. Catching it here makes it a zero-request exit 6. +func TestBuildBrandUpdateRequestValidatesCompletedBody(t *testing.T) { + _, err := BuildBrandUpdateRequest(liveBrand(), + BrandUpdateOptions{DisplayName: ""}, + map[string]bool{"display-name": true}) + if err == nil { + t.Fatal("want an error when a required field is cleared") + } + if !strings.Contains(err.Error(), "display-name") { + t.Errorf("error should name the flag, got: %s", err.Error()) + } +} + +// ValidateBrandCreate rejects a brand-type typo against BrandTypes via +// validBrandType, so the same typo must not slip through on update just +// because the string is non-empty — that asymmetry would turn a local +// exit-6 FlagError into a raw 400 from the API. +func TestBuildBrandUpdateRequestRejectsInvalidBrandType(t *testing.T) { + _, err := BuildBrandUpdateRequest(liveBrand(), + BrandUpdateOptions{BrandType: "NOT_A_REAL_TYPE"}, + map[string]bool{"brand-type": true}) + if err == nil { + t.Fatal("want an error for an invalid --brand-type value") + } + if !strings.Contains(err.Error(), "brand-type") { + t.Errorf("error should name the flag, got: %s", err.Error()) + } + for _, bt := range BrandTypes { + if !strings.Contains(err.Error(), bt) { + t.Errorf("error should list valid brand types (missing %q), got: %s", bt, err.Error()) + } + } +} + +// Task 2 of this plan shipped an early return that discarded already-collected +// violations when brand-type was invalid. ValidateBrandUpdate must aggregate: +// an invalid brand-type value and a cleared required field must both surface +// in the same error, not have one hide the other. +func TestBuildBrandUpdateRequestAggregatesInvalidBrandTypeWithClearedFields(t *testing.T) { + _, err := BuildBrandUpdateRequest(liveBrand(), + BrandUpdateOptions{BrandType: "NOT_A_REAL_TYPE", DisplayName: ""}, + map[string]bool{"brand-type": true, "display-name": true}) + if err == nil { + t.Fatal("want an error for an invalid --brand-type value combined with a cleared required field") + } + if !strings.Contains(err.Error(), "brand-type") { + t.Errorf("error should name brand-type, got: %s", err.Error()) + } + if !strings.Contains(err.Error(), "display-name") { + t.Errorf("error should also name display-name (must not short-circuit), got: %s", err.Error()) + } +} + +// ValidateBrandCreate requires --vertical for every brand type except +// SOLE_PROPRIETOR. Update must catch the same mistake: clearing --vertical on +// a PRIVATE_PROFIT brand must fail locally (zero-request FlagError) rather +// than reach the API as a raw 400 on the full-replacement PUT. +func TestBuildBrandUpdateRequestRejectsClearedVerticalOnPrivateProfit(t *testing.T) { + _, err := BuildBrandUpdateRequest(liveBrand(), + BrandUpdateOptions{Vertical: ""}, + map[string]bool{"vertical": true}) + if err == nil { + t.Fatal("want an error when clearing --vertical on a PRIVATE_PROFIT brand") + } + if !strings.Contains(err.Error(), "vertical") { + t.Errorf("error should name vertical, got: %s", err.Error()) + } +} + +// Same asymmetry, the PUBLIC_PROFIT-only tier: ValidateBrandCreate requires +// --website (plus stock-symbol, stock-exchange, business-contact-email) on +// PUBLIC_PROFIT specifically. brandType is read from the CURRENT brand, not +// from the options struct — livePublicProfitBrand supplies it. +func TestBuildBrandUpdateRequestRejectsClearedWebsiteOnPublicProfit(t *testing.T) { + _, err := BuildBrandUpdateRequest(livePublicProfitBrand(), + BrandUpdateOptions{Website: ""}, + map[string]bool{"website": true}) + if err == nil { + t.Fatal("want an error when clearing --website on a PUBLIC_PROFIT brand") + } + if !strings.Contains(err.Error(), "website") { + t.Errorf("error should name website, got: %s", err.Error()) + } +} + +// SOLE_PROPRIETOR is deliberately exempt from the per-type tier: its field +// rules are account-gated and unobservable on any account available for +// testing, and inventing them would reject requests production accepts. +// Clearing --vertical on a SOLE_PROPRIETOR brand must NOT be treated as +// clearing a required field. +func TestBuildBrandUpdateRequestSkipsPerTypeTierForSoleProprietor(t *testing.T) { + _, err := BuildBrandUpdateRequest(liveSoleProprietorBrand(), + BrandUpdateOptions{Vertical: ""}, + map[string]bool{"vertical": true}) + if err != nil { + t.Fatalf("SOLE_PROPRIETOR must skip the per-type tier, got an error: %v", err) + } +} + +// Per-type violations must aggregate with universal-field violations into one +// error, the same way an invalid brand-type aggregates with cleared required +// fields (TestBuildBrandUpdateRequestAggregatesInvalidBrandTypeWithClearedFields +// above) — not have the universal check short-circuit before the per-type +// tier ever runs. +func TestBuildBrandUpdateRequestAggregatesUniversalAndPerTypeClearedFields(t *testing.T) { + _, err := BuildBrandUpdateRequest(liveBrand(), + BrandUpdateOptions{DisplayName: "", Vertical: ""}, + map[string]bool{"display-name": true, "vertical": true}) + if err == nil { + t.Fatal("want an error combining a cleared universal field with a cleared per-type field") + } + if !strings.Contains(err.Error(), "display-name") { + t.Errorf("error should name display-name, got: %s", err.Error()) + } + if !strings.Contains(err.Error(), "vertical") { + t.Errorf("error should also name vertical (must aggregate, not short-circuit), got: %s", err.Error()) + } +} + +func TestIdentityFieldsChanged(t *testing.T) { + tests := []struct { + name string + current map[string]any + changed map[string]bool + want []string + }{ + { + name: "no identity fields touched", + current: liveBrand(), + changed: map[string]bool{"website": true, "street": true}, + want: nil, + }, + { + name: "company name is an identity field", + current: liveBrand(), + changed: map[string]bool{"company-name": true}, + want: []string{"company-name"}, + }, + { + name: "all four identity fields plus mobile phone", + current: liveBrand(), + changed: map[string]bool{ + "company-name": true, "brand-type": true, "ein": true, + "ein-issuing-country-code-a3": true, "mobile-phone": true, + }, + want: []string{"brand-type", "company-name", "ein", "ein-issuing-country-code-a3", "mobile-phone"}, + }, + { + // Changing businessContactEmail revokes Auth+ compliance, but only + // on PUBLIC_PROFIT brands. + name: "business contact email counts only on PUBLIC_PROFIT", + current: liveBrand(), // PRIVATE_PROFIT + changed: map[string]bool{"business-contact-email": true}, + want: nil, + }, + { + name: "business contact email counts on PUBLIC_PROFIT", + current: func() map[string]any { + b := liveBrand() + b["brandType"] = "PUBLIC_PROFIT" + return b + }(), + changed: map[string]bool{"business-contact-email": true}, + want: []string{"business-contact-email"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := IdentityFieldsChanged(tt.current, tt.changed) + sort.Strings(got) + if len(got) == 0 && len(tt.want) == 0 { + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("got %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/tendlc/states.go b/internal/tendlc/states.go new file mode 100644 index 0000000..47326a7 --- /dev/null +++ b/internal/tendlc/states.go @@ -0,0 +1,90 @@ +package tendlc + +// StateClass is how a polled resource's status is interpreted: keep waiting, +// stop and succeed, or stop and report a business failure. +type StateClass int + +const ( + // StatePending means keep polling. It is the DEFAULT for any value not + // explicitly classified, which matters: production returns statuses that + // appear in no published enum. Treating an unknown state as pending costs + // a timeout; treating it as success reports a failed registration as done. + StatePending StateClass = iota + StateSuccess + StateFailure +) + +// ClassifyBrandIdentity maps brandIdentityStatus to a polling outcome. +// +// UNVERIFIED classifies as StatePending, not StateFailure — this looks wrong +// against enumBrandIdentityStatus, which documents REGISTERING as the +// in-progress state. It is not wrong. Measured on production, 2026-08-19: +// production never returns REGISTERING on the read path at all. Two brands +// submitted with byte-identical payloads both read UNVERIFIED within ~3s of +// their 202. WOVNQBAVI2 flipped to VERIFIED at t≈46s. WAR2FRJPVQ was still +// UNVERIFIED at t≈275s, with no TCR response in its history at all. So +// UNVERIFIED means either "TCR hasn't answered yet" or "TCR rejected it", and +// the status alone cannot say which — polling GET .../history for the +// free-text BRAND_IDENTITY_STATUS_UPDATE entry was considered and rejected, +// since that message is undocumented and unversioned and coupling poll +// control flow to its wording would be worse than the latency. Classifying +// UNVERIFIED as a failure made 'brand create --wait' exit 4 within seconds of +// every create, including brands that went on to verify — a false failure on +// the flagship async path, on every single run. Do not "correct" this back +// to StateFailure to match the published enum: the enum is aspirational, +// this is what production actually does. The accepted tradeoff is that a +// brand that really did fail now polls to timeout instead of failing fast; +// awaitTerminal's last-seen-status mechanism exists so that timeout receipt +// still tells the caller what was last observed. +func ClassifyBrandIdentity(status string) StateClass { + switch status { + case "VERIFIED", "VETTED_VERIFIED", "SELF_DECLARED": + return StateSuccess + case "ERROR": + return StateFailure + case "UNVERIFIED": + // Explicit rather than falling through to default: this is the one + // value most likely to be "corrected" back to StateFailure by a + // future reader who only checks it against the enum. See the + // evidence above. + return StatePending + default: + return StatePending + } +} + +// ClassifyVetting maps a brand vetting's vettingStatus to a polling outcome. +// +// The brand-vetting enum is NOT the campaign vettingStatus enum. Live vettings +// report ACTIVE, which is documented in neither. +func ClassifyVetting(status string) StateClass { + switch status { + case "ACTIVE": + return StateSuccess + case "FAILED", "EXPIRED": + return StateFailure + default: + return StatePending + } +} + +// BrandRemediation returns what to do about a brand that settled into a +// business-failure state, or "" for any state that is not one. +// +// UNVERIFIED is deliberately absent: ClassifyBrandIdentity no longer treats +// it as a business failure (see its comment), so awaitTerminal never calls +// this with "UNVERIFIED" on the failure path. It also has no single +// remediation to give — it means either "still registering" or "rejected", +// and advising a paid reverify for a brand that may simply not have finished +// yet would be actively wrong. A brand stuck at UNVERIFIED past --wait's +// timeout is surfaced instead via the timeout receipt's lastSeenStatus and +// note, pointing at 'band tendlc brand get' / 'brand history' to check. +func BrandRemediation(status string) string { + switch status { + case "ERROR": + return "the registry reported an error on this brand. Re-pull its current state from TCR with " + + "'band tendlc brand refresh', and contact your Bandwidth account manager if it persists." + default: + return "" + } +} diff --git a/internal/tendlc/states_test.go b/internal/tendlc/states_test.go new file mode 100644 index 0000000..7d13267 --- /dev/null +++ b/internal/tendlc/states_test.go @@ -0,0 +1,91 @@ +package tendlc + +import ( + "strings" + "testing" +) + +func TestClassifyBrandIdentity(t *testing.T) { + tests := []struct { + status string + want StateClass + }{ + {"VERIFIED", StateSuccess}, + {"VETTED_VERIFIED", StateSuccess}, + {"SELF_DECLARED", StateSuccess}, + // UNVERIFIED is pending, not failure — this contradicts + // enumBrandIdentityStatus, which documents REGISTERING as the + // in-progress state. Measured on production, 2026-08-19: production + // never returns REGISTERING on the read path; every brand reads + // UNVERIFIED from the moment its 202 lands. Two byte-identical + // submissions both read UNVERIFIED at t≈3s — one (WOVNQBAVI2) went on + // to VERIFIED at t≈46s, the other (WAR2FRJPVQ) was still UNVERIFIED + // at t≈275s with no TCR response in its history at all. Do not flip + // this back to StateFailure to match the enum: that would restore the + // bug where 'brand create --wait' exits 4 within seconds for every + // newly created brand, including ones that go on to verify. See + // ClassifyBrandIdentity's comment for the full evidence. + {"UNVERIFIED", StatePending}, + {"ERROR", StateFailure}, + {"REGISTERING", StatePending}, + // An unlisted value keeps polling until timeout. It is never reported + // as success: the vetting enum already surprised us with a live value + // documented nowhere, and guessing "probably fine" on an unknown + // terminal state is how a CLI reports a failed registration as done. + {"SOMETHING_NEW", StatePending}, + {"", StatePending}, + } + for _, tt := range tests { + t.Run(tt.status, func(t *testing.T) { + if got := ClassifyBrandIdentity(tt.status); got != tt.want { + t.Errorf("ClassifyBrandIdentity(%q) = %v, want %v", tt.status, got, tt.want) + } + }) + } +} + +func TestClassifyVetting(t *testing.T) { + tests := []struct { + status string + want StateClass + }{ + // ACTIVE is what live brand vettings report. It appears in neither + // documented enum. + {"ACTIVE", StateSuccess}, + {"FAILED", StateFailure}, + {"EXPIRED", StateFailure}, + {"PENDING", StatePending}, + {"UNSCORE", StatePending}, + {"SOMETHING_NEW", StatePending}, + } + for _, tt := range tests { + t.Run(tt.status, func(t *testing.T) { + if got := ClassifyVetting(tt.status); got != tt.want { + t.Errorf("ClassifyVetting(%q) = %v, want %v", tt.status, got, tt.want) + } + }) + } +} + +// A business failure that says only "it failed" wastes the operator's next +// step. Each failure state names what to do about it. +func TestBrandRemediationIsSpecificPerState(t *testing.T) { + errText := BrandRemediation("ERROR") + if errText == "" { + t.Fatal("ERROR must have remediation text") + } + if !strings.Contains(errText, "refresh") { + t.Errorf("ERROR remediation should mention %q, got: %s", "refresh", errText) + } + // UNVERIFIED is no longer classified as a business failure (see + // ClassifyBrandIdentity) — production uses it for both "still + // registering" and "rejected", and this package can't tell which. It must + // not have failure-remediation text of its own; the ambiguity is + // surfaced instead through the timeout receipt's lastSeenStatus and note. + if got := BrandRemediation("UNVERIFIED"); got != "" { + t.Errorf("UNVERIFIED is no longer a business failure, want no remediation, got: %s", got) + } + if BrandRemediation("VERIFIED") != "" { + t.Error("a success state has no remediation") + } +} diff --git a/internal/tendlc/write.go b/internal/tendlc/write.go new file mode 100644 index 0000000..327854a --- /dev/null +++ b/internal/tendlc/write.go @@ -0,0 +1,126 @@ +package tendlc + +import ( + "fmt" + "net/url" + + "github.com/Bandwidth/cli/internal/api" +) + +// CreateBrand submits a brand for registration (direct customers) or refreshes +// an existing brand from TCR (all customers, body = {"brandId": id}). Both are +// POST /brands; which one happens is decided by the body, not the path. +// Returns 202 with a bandwidthId — the TCR brandId may not exist yet. +func (s *Service) CreateBrand(body map[string]any) (*api.Envelope, error) { + raw, err := s.client.PostRaw(s.base()+"/brands", body) + if err != nil { + return nil, err + } + return api.ParseEnvelope(raw) +} + +// UpdateBrand replaces a brand. +// +// The API treats PUT as a FULL REPLACEMENT — a field omitted from body is set +// to null server-side. Callers must build body with BuildBrandUpdateRequest, +// which starts from the current resource so nothing is dropped. Brands carry +// no version field, so there is no optimistic-locking check: a concurrent +// edit between the GET and this PUT is lost silently. +func (s *Service) UpdateBrand(brandID string, body map[string]any) (*api.Envelope, error) { + if brandID == "" { + return nil, fmt.Errorf("brand ID is required") + } + raw, err := s.client.PutRawJSON(s.brandPath(brandID), body) + if err != nil { + return nil, err + } + return api.ParseEnvelope(raw) +} + +// DeleteBrand permanently deletes a brand. For direct accounts this also +// deletes the brand in TCR, and all campaigns must be deactivated first. It +// does NOT cascade to the associated customer profile — the endpoint docs +// claim it does, but measured against production, both test profiles +// survived with softDeleted: false. Delete the profile separately if it is +// no longer needed. +func (s *Service) DeleteBrand(brandID string) error { + if brandID == "" { + return fmt.Errorf("brand ID is required") + } + return s.client.Delete(s.brandPath(brandID), nil) +} + +// ReverifyBrand resubmits the brand for identity verification. This incurs a +// $4 fee and resets brandIdentityStatus toward re-registration — documented +// as REGISTERING, but production reads it back as UNVERIFIED until TCR +// responds. Returns 204. +func (s *Service) ReverifyBrand(brandID string) error { + if brandID == "" { + return fmt.Errorf("brand ID is required") + } + return s.client.Post(s.brandPath(brandID)+"/identity/reverify", nil, nil) +} + +// Resend2FA re-sends the Business Authentication 2FA email to the brand's +// business contact. Returns 204. +func (s *Service) Resend2FA(brandID string) error { + if brandID == "" { + return fmt.Errorf("brand ID is required") + } + return s.client.Post(s.brandPath(brandID)+"/identity/resend2faEmail", nil, nil) +} + +// BrandHistory returns the brand's activity log: free-text {createdDate, +// message} entries, newest first. Unlike customer profiles there are no +// versioned snapshots and no per-version fetch. +func (s *Service) BrandHistory(brandID string, limit, offset int) (*api.Envelope, error) { + if brandID == "" { + return nil, fmt.Errorf("brand ID is required") + } + return s.get(s.brandPath(brandID) + "/history" + api.EncodeQuery(limit, offset, nil)) +} + +// ListVettings returns the external vettings on a brand. Vettings are +// brand-scoped: there is no campaign vetting endpoint. +func (s *Service) ListVettings(brandID string, limit, offset int) (*api.Envelope, error) { + if brandID == "" { + return nil, fmt.Errorf("brand ID is required") + } + return s.get(s.brandPath(brandID) + "/vettings" + api.EncodeQuery(limit, offset, nil)) +} + +// RequestVetting orders a new external vetting for a brand. Billable. +func (s *Service) RequestVetting(brandID string, body map[string]any) (*api.Envelope, error) { + if brandID == "" { + return nil, fmt.Errorf("brand ID is required") + } + raw, err := s.client.PostRaw(s.brandPath(brandID)+"/vettings", body) + if err != nil { + return nil, err + } + return api.ParseEnvelope(raw) +} + +// ImportVetting records an externally-performed vetting against a brand. +func (s *Service) ImportVetting(brandID, vettingID string, body map[string]any) (*api.Envelope, error) { + if brandID == "" { + return nil, fmt.Errorf("brand ID is required") + } + if vettingID == "" { + return nil, fmt.Errorf("vetting ID is required") + } + raw, err := s.client.PutRawJSON( + s.brandPath(brandID)+"/vettings/"+url.PathEscape(vettingID), body) + if err != nil { + return nil, err + } + return api.ParseEnvelope(raw) +} + +// brandPath builds /brands/{id}. The path parameter is documented as the TCR +// brandId, but production accepts the Bandwidth ID too — which is required, +// not merely convenient: a newly created direct brand has brandId null until +// TCR registers it, and would otherwise be unreachable. +func (s *Service) brandPath(brandID string) string { + return s.base() + "/brands/" + url.PathEscape(brandID) +} diff --git a/internal/tendlc/write_test.go b/internal/tendlc/write_test.go new file mode 100644 index 0000000..b3d0a95 --- /dev/null +++ b/internal/tendlc/write_test.go @@ -0,0 +1,259 @@ +package tendlc + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/Bandwidth/cli/internal/api" +) + +// captured records what the stub server saw, so tests assert on the request +// the service actually sent rather than on a mock's expectations. +type captured struct { + method string + path string + escapedPath string + query string + body map[string]any +} + +func stubService(t *testing.T, status int, respBody string, got *captured) *Service { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got.method = r.Method + got.path = r.URL.Path + got.escapedPath = r.URL.EscapedPath() + got.query = r.URL.RawQuery + if b, _ := io.ReadAll(r.Body); len(b) > 0 { + _ = json.Unmarshal(b, &got.body) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + if respBody != "" { + _, _ = io.WriteString(w, respBody) + } + })) + t.Cleanup(srv.Close) + return NewService(api.NewClientNoAuth(srv.URL), "9901287") +} + +func TestCreateBrandPostsToBrandsPath(t *testing.T) { + var got captured + s := stubService(t, 202, `{"data":{"bandwidthId":"WABC123"}}`, &got) + + env, err := s.CreateBrand(map[string]any{"displayName": "Acme"}) + if err != nil { + t.Fatalf("CreateBrand: %v", err) + } + if got.method != "POST" { + t.Errorf("method = %q, want POST", got.method) + } + if want := "/api/v2/accounts/9901287/tendlc/brands"; got.path != want { + t.Errorf("path = %q, want %q", got.path, want) + } + if got.body["displayName"] != "Acme" { + t.Errorf("body displayName = %v, want Acme", got.body["displayName"]) + } + obj, err := env.Object() + if err != nil { + t.Fatalf("Object: %v", err) + } + if obj["bandwidthId"] != "WABC123" { + t.Errorf("bandwidthId = %v, want WABC123", obj["bandwidthId"]) + } +} + +func TestUpdateBrandPutsToBrandPath(t *testing.T) { + var got captured + s := stubService(t, 202, `{"data":{"bandwidthId":"WABC123"}}`, &got) + + if _, err := s.UpdateBrand("BGJR2BA", map[string]any{"displayName": "Acme"}); err != nil { + t.Fatalf("UpdateBrand: %v", err) + } + if got.method != "PUT" { + t.Errorf("method = %q, want PUT", got.method) + } + if want := "/api/v2/accounts/9901287/tendlc/brands/BGJR2BA"; got.path != want { + t.Errorf("path = %q, want %q", got.path, want) + } +} + +func TestDeleteBrandUsesDelete(t *testing.T) { + var got captured + s := stubService(t, 202, "", &got) + + if err := s.DeleteBrand("WET8JUY8H0"); err != nil { + t.Fatalf("DeleteBrand: %v", err) + } + if got.method != "DELETE" { + t.Errorf("method = %q, want DELETE", got.method) + } + if want := "/api/v2/accounts/9901287/tendlc/brands/WET8JUY8H0"; got.path != want { + t.Errorf("path = %q, want %q", got.path, want) + } +} + +func TestReverifyAndResend2FAPostToIdentityPaths(t *testing.T) { + tests := []struct { + name string + call func(*Service) error + want string + }{ + {"reverify", func(s *Service) error { return s.ReverifyBrand("BGJR2BA") }, + "/api/v2/accounts/9901287/tendlc/brands/BGJR2BA/identity/reverify"}, + {"resend2fa", func(s *Service) error { return s.Resend2FA("BGJR2BA") }, + "/api/v2/accounts/9901287/tendlc/brands/BGJR2BA/identity/resend2faEmail"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got captured + // 204 with an empty body: these endpoints return no content, so the + // service must not try to parse an envelope out of nothing. + s := stubService(t, 204, "", &got) + if err := tt.call(s); err != nil { + t.Fatalf("call: %v", err) + } + if got.method != "POST" { + t.Errorf("method = %q, want POST", got.method) + } + if got.path != tt.want { + t.Errorf("path = %q, want %q", got.path, tt.want) + } + }) + } +} + +func TestBrandHistoryEncodesPagination(t *testing.T) { + var got captured + s := stubService(t, 200, `{"data":[],"page":{"totalElements":0}}`, &got) + + if _, err := s.BrandHistory("BGJR2BA", 10, 20); err != nil { + t.Fatalf("BrandHistory: %v", err) + } + if want := "/api/v2/accounts/9901287/tendlc/brands/BGJR2BA/history"; got.path != want { + t.Errorf("path = %q, want %q", got.path, want) + } + if got.query != "limit=10&offset=20" { + t.Errorf("query = %q, want limit=10&offset=20", got.query) + } +} + +func TestListVettingsEncodesPagination(t *testing.T) { + var got captured + s := stubService(t, 200, `{"data":[],"page":{"totalElements":0}}`, &got) + + if _, err := s.ListVettings("BGJR2BA", 10, 0); err != nil { + t.Fatalf("ListVettings: %v", err) + } + if want := "/api/v2/accounts/9901287/tendlc/brands/BGJR2BA/vettings"; got.path != want { + t.Errorf("path = %q, want %q", got.path, want) + } +} + +func TestRequestVettingPostsBody(t *testing.T) { + var got captured + s := stubService(t, 202, `{"data":{"bandwidthId":"WV123"}}`, &got) + + body := map[string]any{"evpId": "AEGIS", "vettingClass": "STANDARD"} + if _, err := s.RequestVetting("BGJR2BA", body); err != nil { + t.Fatalf("RequestVetting: %v", err) + } + if got.method != "POST" { + t.Errorf("method = %q, want POST", got.method) + } + if got.body["evpId"] != "AEGIS" || got.body["vettingClass"] != "STANDARD" { + t.Errorf("body = %v, want evpId/vettingClass preserved", got.body) + } +} + +func TestImportVettingPutsToVettingPath(t *testing.T) { + var got captured + s := stubService(t, 202, `{"data":{"bandwidthId":"WV123"}}`, &got) + + if _, err := s.ImportVetting("BGJR2BA", "978de74a-7191", map[string]any{"evpId": "AEGIS"}); err != nil { + t.Fatalf("ImportVetting: %v", err) + } + if got.method != "PUT" { + t.Errorf("method = %q, want PUT", got.method) + } + if want := "/api/v2/accounts/9901287/tendlc/brands/BGJR2BA/vettings/978de74a-7191"; got.path != want { + t.Errorf("path = %q, want %q", got.path, want) + } +} + +// Every method that takes an ID must reject an empty one before making a +// request. Without this a caller with an unset variable silently hits the +// collection endpoint — DELETE on /brands rather than /brands/{id}. +func TestEmptyIDsRejectedWithoutRequest(t *testing.T) { + var got captured + s := stubService(t, 200, `{"data":{}}`, &got) + + calls := map[string]func() error{ + "UpdateBrand": func() error { _, err := s.UpdateBrand("", map[string]any{}); return err }, + "DeleteBrand": func() error { return s.DeleteBrand("") }, + "ReverifyBrand": func() error { return s.ReverifyBrand("") }, + "Resend2FA": func() error { return s.Resend2FA("") }, + "BrandHistory": func() error { _, err := s.BrandHistory("", 10, 0); return err }, + "ListVettings": func() error { _, err := s.ListVettings("", 10, 0); return err }, + "RequestVetting": func() error { + _, err := s.RequestVetting("", map[string]any{}) + return err + }, + "ImportVettingNoBrand": func() error { + _, err := s.ImportVetting("", "v1", map[string]any{}) + return err + }, + "ImportVettingNoVetting": func() error { + _, err := s.ImportVetting("B1", "", map[string]any{}) + return err + }, + } + for name, call := range calls { + t.Run(name, func(t *testing.T) { + got = captured{} + if err := call(); err == nil { + t.Fatal("want an error for an empty ID, got nil") + } + if got.method != "" { + t.Errorf("a request was made (%s %s); want none", got.method, got.path) + } + }) + } +} + +// IDs go into the path, so a value containing a slash or a space must be +// escaped rather than silently changing which endpoint is called. +func TestBrandIDIsPathEscaped(t *testing.T) { + var got captured + s := stubService(t, 200, `{"data":[],"page":{"totalElements":0}}`, &got) + + if _, err := s.BrandHistory("a/b c", 10, 0); err != nil { + t.Fatalf("BrandHistory: %v", err) + } + if want := "/api/v2/accounts/9901287/tendlc/brands/a%2Fb%20c/history"; got.escapedPath != want { + t.Errorf("escaped path = %q, want %q", got.escapedPath, want) + } +} + +// A vetting ID is an externally-supplied provider value, not one Bandwidth +// assigns — so it must be escaped in the URL just like a brand ID. +// TestImportVettingPutsToVettingPath above asserts on got.path, which +// net/url DECODES, so it would pass identically whether +// url.PathEscape(vettingID) was called or not (this is the same "regression +// guard that guards nothing" class Task 1's brandPath test hit — see the +// escapedPath field's own history). This test asserts on got.escapedPath +// instead, so it actually catches the escape being dropped. +func TestVettingIDIsPathEscaped(t *testing.T) { + var got captured + s := stubService(t, 202, `{"data":{"bandwidthId":"WV123"}}`, &got) + + if _, err := s.ImportVetting("BGJR2BA", "v/1 2", map[string]any{"evpId": "AEGIS"}); err != nil { + t.Fatalf("ImportVetting: %v", err) + } + if want := "/api/v2/accounts/9901287/tendlc/brands/BGJR2BA/vettings/v%2F1%202"; got.escapedPath != want { + t.Errorf("escaped path = %q, want %q", got.escapedPath, want) + } +}