diff --git a/AGENTS.md b/AGENTS.md index eee6285..1bf2a17 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,7 @@ These principles guide how the CLI is built. If you're contributing changes, maintain them: - **`--plain` output must be stable and parseable.** Agents depend on flat JSON. Don't change the shape of `--plain` output without a migration path. -- **`--if-not-exists` for idempotency.** Any create command should support this flag so agents can retry safely. +- **`--if-not-exists` for idempotency, where a safe natural key exists.** Create commands support this flag when there's a stable identity to retry against. A command that lacks one omits the flag deliberately and documents why (see [Customer Profiles](#customer-profiles)) rather than risk a retry silently reusing the wrong resource. - **`--wait` for async operations.** Agents can't poll — give them a way to block until the operation completes. - **Structured exit codes.** Agents use exit codes for control flow, not string parsing. See [Exit Codes](#exit-codes). - **Update this file.** If you add, remove, or change a command, update this file alongside the README. @@ -880,6 +880,93 @@ band tnoption assign +19195551234 --campaign-id CA3XKE1 --wait **If `band tendlc` returns 403:** Don't retry — escalate. Tell the user: "Your credential may not have the Campaign Management role, or your account may not have the Registration Center feature enabled. Contact your Bandwidth account manager to check your configuration." +## Customer Profiles + +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. + +Requires the **Customer Profiles Access role** — check with `band auth status --plain`. + +### Create, list, and get + +```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} + +band customer-profile list --all --plain # walks every page; cannot combine with --offset +band customer-profile get 622t7KB9oZkl9kQob0b8el --plain +``` + +Keys come back alphabetical because the payload is a Go map — don't expect a +"nicer" ordering; the docs match reality, not a prettified version of it. + +`list` excludes soft-deleted profiles; `get` still returns them, reporting +`softDeleted: true`. Without `--all`, a truncated page warns on stderr. + +**`create` deliberately has no `--if-not-exists`** (see the [Design +Principles](#design-principles) exception). A profile has no safe natural key +to match on, and it's strictly 1:1 with a brand — a retry that silently reused +an existing profile could link an old brand's profile to a new brand's data. +Each brand needs its own freshly created profile; a retry must create, not reuse. + +**`create` is also non-idempotent, which cuts the other way after an +ambiguous failure.** If a `create` call fails ambiguously — e.g. the +connection drops after the POST reached the server but before the response +reached you — do not blindly retry. A blind retry can create a *second*, +duplicate profile if the first one actually succeeded. Instead, run `band +customer-profile list --plain` and reconcile the results against what you +just submitted (name, website, contact) to determine whether the first call +already created a profile. If you cannot establish uniqueness that way, stop +and escalate rather than guessing. + +### Update is read-modify-write + +The API replaces the whole record, so `update` reads the profile first and +re-sends it with your changes applied. Fields you do not pass are preserved. +**Passing a flag with an empty value clears that field** — it sends JSON +`null`, not an empty string, because the API rejects empty strings. A +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 +``` + +### Delete is a soft delete + +`delete` requires `--confirm`. That's a flag, never a prompt, so agents and +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 +``` + +Note these are two different fields on two different resources, not a typo of +each other: `deleted: true` is the delete command's own receipt, confirming the +204 completed synchronously. `softDeleted: true` is a field on the profile +itself, seen when you `get` it afterward — there is no `deleted` field on the +profile, and no `softDeleted` field on the receipt. + +### Version history + +`history list` and `history get` return a `{data, metadata}` envelope, newest +first — the profile snapshot lives under `data`, and `version`, `operation`, +`userName`, `createdDate` live under `metadata`. So the version is at +`metadata.version`, NOT top-level the way it is on `customer-profile get`. +Observed `metadata.operation` values: `CREATED`, `UPDATED`, `DELETED`. + +```bash +band customer-profile history list 622t7KB9oZkl9kQob0b8el --plain +band customer-profile history get 622t7KB9oZkl9kQob0b8el 1 --plain +``` + ## 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. diff --git a/README.md b/README.md index 8c3d7c6..4a46b8d 100644 --- a/README.md +++ b/README.md @@ -257,6 +257,18 @@ The CLI checks three things before every send: | **Callback URL** | App has a real callback URL (not `example.com`, `localhost`, etc.) | Send blocked — tells you to run `band app update --callback-url` | | **Number registration** | 10DLC numbers are on an approved campaign; toll-free numbers have TFV approval | Send blocked — tells you what's missing | +### Customer profiles (10DLC prerequisite) + +Registering a 10DLC brand starts with a customer profile — and a profile backs exactly one brand, so create a fresh one for each brand you register. + +```sh +band customer-profile create --name "Acme Corp" --plain +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. + ### 10DLC campaigns (local numbers) If you're sending from a standard 10-digit local number, it must be assigned to an approved 10DLC campaign. Without this, carriers will block your messages. The CLI detects this and blocks the send with a diagnostic message. diff --git a/cmd/customerprofile/create.go b/cmd/customerprofile/create.go new file mode 100644 index 0000000..d53fecf --- /dev/null +++ b/cmd/customerprofile/create.go @@ -0,0 +1,65 @@ +package customerprofile + +import ( + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/cmdutil" + cpsvc "github.com/Bandwidth/cli/internal/customerprofile" + "github.com/Bandwidth/cli/internal/output" +) + +var createOpts cpsvc.CreateOptions + +func init() { + f := createCmd.Flags() + f.StringVar(&createOpts.Name, "name", "", "Profile name (required)") + f.StringVar(&createOpts.Website, "website", "", "Business website URL") + f.StringVar(&createOpts.ContactName, "contact-name", "", "Contact name (required if any other contact field is set)") + f.StringVar(&createOpts.ContactPhone, "contact-phone", "", "Contact phone in E.164") + f.StringVar(&createOpts.ContactEmail, "contact-email", "", "Contact email") + f.StringVar(&createOpts.AddressID, "address-id", "", "Existing address ID to associate") + Cmd.AddCommand(createCmd) +} + +var createCmd = &cobra.Command{ + Use: "create", + Short: "Create a customer profile", + Long: `Creates a customer profile. + +A profile backs exactly one 10DLC brand, so create a new one for each brand +you intend to register — reusing a profile fails at brand creation. + +This is a non-idempotent write: after an ambiguous failure, do not blindly +retry — list profiles and reconcile against what you submitted first.`, + Example: ` band customer-profile create --name "Acme Corp" --plain + + band customer-profile create --name "Acme Corp" \ + --website https://acme.com \ + --contact-name "Ops Team" --contact-email ops@acme.com`, + // No positional args: this is a non-idempotent 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 profile. + 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 := cpsvc.ValidateCreate(createOpts); err != nil { + return err + } + svc, err := service(cmd) + if err != nil { + return err + } + env, err := svc.Create(cpsvc.BuildCreateRequest(createOpts)) + if err != nil { + return roleGateError(err) + } + obj, err := env.Object() + if err != nil { + return err + } + format, plain := cmdutil.OutputFlags(cmd) + return output.StdoutAuto(format, plain, obj) + }, +} diff --git a/cmd/customerprofile/customerprofile.go b/cmd/customerprofile/customerprofile.go new file mode 100644 index 0000000..b4ad8d1 --- /dev/null +++ b/cmd/customerprofile/customerprofile.go @@ -0,0 +1,37 @@ +// Package customerprofile implements `band customer-profile`. +// +// Customer profiles are a Numbers v2 resource, but they matter here because a +// profile is a hard prerequisite for 10DLC brand registration: a profile backs +// EXACTLY ONE brand, and reusing one fails with "cannot be assigned to another +// brand". Every new brand needs a freshly created profile. +package customerprofile + +import ( + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/cmdutil" + cpsvc "github.com/Bandwidth/cli/internal/customerprofile" +) + +// Cmd is the `band customer-profile` parent command. +var Cmd = &cobra.Command{ + Use: "customer-profile", + Short: "Manage customer profiles", + Long: `Create and manage customer profiles. + +A customer profile is required to register a 10DLC brand, and a profile backs +exactly one brand — create a new profile for each brand you register. + +Requires the Customer Profiles Access role. Check with 'band auth status --plain'.`, +} + +// service builds a customer-profile service for the active account. A package +// var, not a plain func, so tests can substitute a service pointed at a stub — +// the same seam as cmd/sip's service and cmd/tendlc's. +var service = 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 +} diff --git a/cmd/customerprofile/customerprofile_test.go b/cmd/customerprofile/customerprofile_test.go new file mode 100644 index 0000000..3b2d84e --- /dev/null +++ b/cmd/customerprofile/customerprofile_test.go @@ -0,0 +1,296 @@ +package customerprofile + +import ( + "bytes" + "encoding/json" + "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" + cpsvc "github.com/Bandwidth/cli/internal/customerprofile" + "github.com/Bandwidth/cli/internal/testutil" +) + +func exitCodeOf(err error) int { return cmdutil.ExitCodeForError(err) } + +// testRoot is a single fake root, built once via testutil.NewTestRoot and +// reused by every runCmd call in this package, with Cmd as its only child. +// --format/--plain/--account-id/--environment live here, not on Cmd, exactly +// as in production (Cmd carries none of its own — see customerprofile.go). +// +// It is deliberately NOT rebuilt per call. cobra caches each command's merged +// ancestor flags (parentsPflags) the first time it parses and never +// refreshes that cache for a different root object later: constructing a +// fresh testutil.NewTestRoot(Cmd) inside runCmd on every invocation would +// mean Cmd/createCmd/listCmd/getCmd — all package-level, so the same +// instances across every test — get pinned to the FIRST test's root the +// first time any of them parses, and every later --plain/--format/ +// --account-id would silently parse into that stale, discarded root's flag +// object while cmd.Root().Flag(...) reads the current (untouched, always +// default) root. Verified with a minimal cobra repro before writing this: +// a fresh root per call reports --plain as false on every call after the +// first; a single shared root with its own flags reset between calls +// reports it correctly every time. See task-4-report.md, "Fix round 1". +var testRoot = testutil.NewTestRoot(Cmd) + +// resetFlags restores every flag on cmd and all its descendants to its +// default value and clears the Changed bit. +// +// Cmd is a package-level cobra command, and cobra records flag state +// (including Changed) on it permanently. Within one test binary, every +// Cmd.Execute() call in this package shares that state: a flag set by an +// earlier test (e.g. --name from a create test) would otherwise leak into a +// later run, and cmd.Flags().Changed("offset") would stay true for the rest +// of the process once any test passes --offset — silently breaking +// TestListRejectsAllWithExplicitOffset or making it pass for the wrong +// reason. resetFlags is walked recursively (rather than listing flag names +// per command) so it does not need updating as later tasks add more +// commands and flags to this tree. It is called on testRoot, not Cmd, so it +// also resets testRoot's own --format/--plain/--account-id/--environment +// (recursion reaches Cmd and its subcommands via testRoot.Commands()). +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) + } +} + +func TestCommandsRegistered(t *testing.T) { + for _, name := range []string{"create", "list", "get"} { + c, _, err := Cmd.Find([]string{name}) + if err != nil || c.Name() != name { + t.Errorf("Find(%q) = %v, err %v", name, c, err) + } + } +} + +func TestCreateRequiresName(t *testing.T) { + out, err := runCmd(t, nil, "create") + if err == nil { + t.Fatal("expected an error when --name is missing") + } + if !strings.Contains(err.Error(), "missing required flags") || + !strings.Contains(err.Error(), "--name") { + t.Errorf("error = %q, want it to name the missing flag", err.Error()) + } + if out != "" { + t.Errorf("stdout = %q, want nothing written on a flag error", out) + } +} + +func TestCreateEmitsReceipt(t *testing.T) { + out, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":{"id":"abc","name":"Acme","version":0}}`)) + }, "create", "--name", "Acme", "--plain") + if err != nil { + t.Fatalf("create: %v", err) + } + var got map[string]any + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("stdout is not JSON: %q", out) + } + if got["id"] != "abc" { + t.Errorf("stdout id = %v, want abc", got["id"]) + } +} + +// create is a non-idempotent write: a stray positional argument (e.g. a +// typo meant for a flag value) must be rejected outright, not silently +// ignored while the command creates a real profile anyway. Passing a nil +// handler to runCmd means the stub's t.Fatal fires if the command ever +// reaches the wire despite the bad args. +func TestCreateRejectsPositionalArgs(t *testing.T) { + out, err := runCmd(t, nil, "create", "GARBAGE", "--name", "Acme") + if err == nil { + t.Fatal("expected an error: create takes no positional arguments") + } + if out != "" { + t.Errorf("stdout = %q, want nothing written when args are rejected", out) + } +} + +func TestListRejectsPositionalArgs(t *testing.T) { + out, err := runCmd(t, nil, "list", "GARBAGE") + if err == nil { + t.Fatal("expected an error: list takes no positional arguments") + } + if out != "" { + t.Errorf("stdout = %q, want nothing written when args are rejected", out) + } +} + +func TestListReturnsArrayEvenForOneResult(t *testing.T) { + out, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[{"id":"abc"}],"page":{"pageSize":50,"totalElements":1}}`)) + }, "list", "--plain") + if err != nil { + t.Fatalf("list: %v", err) + } + if !strings.HasPrefix(strings.TrimSpace(out), "[") { + t.Errorf("stdout = %q, want a JSON array", out) + } +} + +func TestListAllWalksEveryPage(t *testing.T) { + var offsets []string + out, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + offsets = append(offsets, r.URL.Query().Get("offset")) + if r.URL.Query().Get("offset") == "" || r.URL.Query().Get("offset") == "0" { + _, _ = w.Write([]byte(`{"data":[{"id":"a"}],"page":{"pageSize":1,"totalElements":2}}`)) + return + } + _, _ = w.Write([]byte(`{"data":[{"id":"b"}],"page":{"pageSize":1,"totalElements":2}}`)) + }, "list", "--all", "--limit", "1", "--plain") + if err != nil { + t.Fatalf("list --all: %v", err) + } + if !strings.Contains(out, `"a"`) || !strings.Contains(out, `"b"`) { + t.Errorf("stdout = %q, want items from both pages", out) + } +} + +func TestListRejectsAllWithExplicitOffset(t *testing.T) { + _, err := runCmd(t, nil, "list", "--all", "--offset", "0") + if err == nil { + t.Fatal("expected an error: --all with an explicit --offset is contradictory") + } +} + +func TestGetReturnsObjectNotArray(t *testing.T) { + out, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":{"id":"abc","softDeleted":false}}`)) + }, "get", "abc", "--plain") + if err != nil { + t.Fatalf("get: %v", err) + } + if !strings.HasPrefix(strings.TrimSpace(out), "{") { + t.Errorf("stdout = %q, want a JSON object", out) + } +} + +func TestGetRequiresID(t *testing.T) { + if _, err := runCmd(t, nil, "get"); err == nil { + t.Fatal("expected an error when no ID is given") + } +} + +func TestFilterFlagUsesDeepObjectEncoding(t *testing.T) { + var rawQuery string + _, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + rawQuery = r.URL.RawQuery + _, _ = w.Write([]byte(`{"data":[],"page":{"pageSize":50,"totalElements":0}}`)) + }, "list", "--name-contains", "Acme", "--plain") + if err != nil { + t.Fatalf("list: %v", err) + } + if !strings.Contains(rawQuery, "name%5Bcontains%5D=Acme") { + t.Errorf("query = %q, want deepObject form name[contains]=Acme", rawQuery) + } +} + +// TestRunCmdRootFlagsSurviveAcrossCalls guards runCmd's shared testRoot +// design. 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 (confirmed against the cobra v1.8.1 source: updateParentsPflags +// only allocates parentsPflags once, and pflag.FlagSet.AddFlagSet skips any +// flag already present by name). Cmd/createCmd/listCmd/getCmd are +// package-level, so if runCmd is ever "simplified" back to building a fresh +// testutil.NewTestRoot(Cmd) on every call, only the FIRST call in the whole +// test binary actually gets that root's flags — every --plain/--format/ +// --account-id after that silently parses into the first call's discarded +// root object, while cmd.Root().Flag(...) keeps reading the new, untouched +// root. None of the other tests in this file would catch that: create/list/ +// get's payloads are already flat by the time they reach output.StdoutAuto/ +// StdoutPlainList, so plain=true and plain=false render identical bytes for +// them. --format table is used here instead, specifically because table +// output is visibly not JSON — this is the one assertion in the file that +// actually depends on the second call's root flags having taken effect. +func TestRunCmdRootFlagsSurviveAcrossCalls(t *testing.T) { + stub := func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":{"id":"abc","name":"Acme"}}`)) + } + + // Call 1: ordinary, default (json) output — exercises whatever ancestor- + // flag cache cobra builds the first time getCmd parses. + if _, err := runCmd(t, stub, "get", "abc"); err != nil { + t.Fatalf("get (default json): %v", err) + } + + // Call 2: explicit --format table. If the second call's root flags were + // lost, this silently renders as JSON instead of a table. + out, err := runCmd(t, stub, "get", "abc", "--format", "table") + if err != nil { + t.Fatalf("get --format table: %v", err) + } + trimmed := strings.TrimSpace(out) + if strings.HasPrefix(trimmed, "{") || strings.HasPrefix(trimmed, "[") { + t.Errorf("stdout = %q, want table output — --format table on the second runCmd call was silently dropped", out) + } +} + +// runCmdWithStderr executes one command against a stub server and returns +// both stdout and stderr. It is the single implementation behind runCmd and +// runCmdCapturingStderr — they used to be two near-identical copies of this +// same setup (resetFlags, stub server, service seam, testRoot args/writers), +// differing only in whether stderr was captured or discarded. Every command +// test in this package goes through one of the two wrappers below, so the +// seam is swapped in exactly one place. +func runCmdWithStderr(t *testing.T, h http.HandlerFunc, args ...string) (stdout, stderr string, err error) { + t.Helper() + + resetFlags(testRoot) + + var srvURL string + if h != nil { + srv := httptest.NewServer(h) + t.Cleanup(srv.Close) + srvURL = srv.URL + } + + orig := service + service = func(cmd *cobra.Command) (*cpsvc.Service, error) { + if srvURL == "" { + t.Fatal("command made a request but no stub server was provided") + } + return cpsvc.NewService(api.NewClientNoAuth(srvURL), "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 +} + +// runCmd is runCmdWithStderr for the common case that only cares about +// stdout. Its (string, error) signature is preserved deliberately: it is used +// by every other test in this package, and changing it would ripple across +// the whole suite. +func runCmd(t *testing.T, h http.HandlerFunc, args ...string) (string, error) { + t.Helper() + out, _, err := runCmdWithStderr(t, h, args...) + return out, err +} + +// runCmdCapturingStderr is runCmd's twin for tests that assert on +// cmd.PrintErrf output (e.g. truncation warnings). +func runCmdCapturingStderr(t *testing.T, h http.HandlerFunc, args ...string) (stdout, stderr string, err error) { + t.Helper() + return runCmdWithStderr(t, h, args...) +} diff --git a/cmd/customerprofile/delete.go b/cmd/customerprofile/delete.go new file mode 100644 index 0000000..1fc1dc0 --- /dev/null +++ b/cmd/customerprofile/delete.go @@ -0,0 +1,99 @@ +package customerprofile + +import ( + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/cmdutil" + cpsvc "github.com/Bandwidth/cli/internal/customerprofile" + "github.com/Bandwidth/cli/internal/output" +) + +var deleteConfirm bool + +func init() { + deleteCmd.Flags().BoolVar(&deleteConfirm, "confirm", false, + "Required. Confirms the profile should be deleted.") + Cmd.AddCommand(deleteCmd) + Cmd.AddCommand(restoreCmd) +} + +var deleteCmd = &cobra.Command{ + Use: "delete ", + Short: "Soft-delete a customer profile", + Long: `Soft-deletes a customer profile. + +The record is removed from listings but remains retrievable by ID with +'customer-profile get', reporting softDeleted: true, and can be brought back +with 'customer-profile restore'. + +Requires --confirm. That is a flag rather than a prompt, so scripts, agents, +and humans all get the same contract regardless of whether a terminal is +attached.`, + Example: ` band customer-profile delete 3IIzIFnRRQBE3AMzPpMTNo --confirm --plain`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := requireConfirm(deleteConfirm, + "this soft-deletes customer profile "+args[0]+ + ", removing it from listings; pass --confirm to proceed "+ + "(restore it later with 'band customer-profile restore "+args[0]+"')"); err != nil { + return err + } + svc, err := service(cmd) + if err != nil { + return err + } + if err := svc.Delete(args[0]); err != nil { + return roleGateError(err) + } + format, plain := cmdutil.OutputFlags(cmd) + // A 204 is a completed delete, not an async acceptance, so the receipt + // says "deleted" rather than "accepted". + return output.StdoutAuto(format, plain, map[string]any{ + "id": args[0], + "deleted": true, + "restore": "band customer-profile restore " + args[0], + }) + }, +} + +var restoreCmd = &cobra.Command{ + Use: "restore ", + Short: "Restore a soft-deleted customer profile", + Long: `Restores a soft-deleted customer profile. + +Sends softDeleted: false. Note the published API docs describe restoring with +{"deleted": false} — that form returns 404 "Customer profile not found" even +though the record is retrievable. Reported as MV-23429. + +No --confirm needed: restoring is not destructive.`, + Example: ` band customer-profile restore 3IIzIFnRRQBE3AMzPpMTNo --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.Get(args[0]) + if err != nil { + return roleGateError(err) + } + current, err := env.Object() + if err != nil { + return err + } + body, err := cpsvc.BuildRestoreRequest(current) + if err != nil { + return err + } + restored, err := svc.Update(args[0], body) + if err != nil { + return roleGateError(conflictHint(err)) + } + obj, err := restored.Object() + if err != nil { + return err + } + format, plain := cmdutil.OutputFlags(cmd) + return output.StdoutAuto(format, plain, obj) + }, +} diff --git a/cmd/customerprofile/delete_test.go b/cmd/customerprofile/delete_test.go new file mode 100644 index 0000000..15c6e58 --- /dev/null +++ b/cmd/customerprofile/delete_test.go @@ -0,0 +1,105 @@ +package customerprofile + +import ( + "encoding/json" + "io" + "net/http" + "strings" + "testing" +) + +func TestDeleteRequiresConfirm(t *testing.T) { + _, err := runCmd(t, nil, "delete", "abc") + if err == nil { + t.Fatal("expected an error without --confirm") + } + if !strings.Contains(err.Error(), "--confirm") { + t.Errorf("error = %q, want it to name the flag", err.Error()) + } + if got := exitCodeOf(err); got != 6 { + t.Errorf("exit code = %d, want 6 — this is a usage error with no request made", got) + } +} + +// The gate must not depend on a TTY: an agent and a human get the same contract. +func TestDeleteConfirmGateIsFlagOnlyNotTTY(t *testing.T) { + _, err := runCmd(t, nil, "delete", "abc", "--plain") + if err == nil { + t.Fatal("expected --confirm to be required under --plain too") + } +} + +func TestDeleteWithConfirmEmitsDeletedReceipt(t *testing.T) { + out, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNoContent) + }, "delete", "abc", "--confirm", "--plain") + if err != nil { + t.Fatalf("delete: %v", err) + } + var got map[string]any + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("stdout is not JSON: %q", out) + } + if got["deleted"] != true { + t.Errorf(`receipt = %v, want deleted:true — this is a synchronous 204, not an async accept`, got) + } + if _, present := got["accepted"]; present { + t.Error(`receipt must not say "accepted": the delete completed, it was not queued`) + } + if got["id"] != "abc" { + t.Errorf("receipt id = %v, want the profile ID", got["id"]) + } +} + +func TestRestoreSendsSoftDeletedNotDeleted(t *testing.T) { + var putBody map[string]any + _, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`{"data":{"id":"abc","name":"Acme","version":3,"softDeleted":true}}`)) + return + } + raw, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(raw, &putBody) + _, _ = w.Write([]byte(`{"data":{"id":"abc","version":4,"softDeleted":false}}`)) + }, "restore", "abc", "--plain") + if err != nil { + t.Fatalf("restore: %v", err) + } + if putBody["softDeleted"] != false { + t.Errorf("PUT body softDeleted = %v, want false", putBody["softDeleted"]) + } + if _, present := putBody["deleted"]; present { + t.Error(`must not send "deleted": the documented form returns 404 "Customer profile not found"`) + } +} + +func TestRestoreDoesNotRequireConfirm(t *testing.T) { + _, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`{"data":{"id":"abc","name":"Acme","version":1,"softDeleted":true}}`)) + return + } + _, _ = w.Write([]byte(`{"data":{"id":"abc","name":"Acme","version":2,"softDeleted":false}}`)) + }, "restore", "abc", "--plain") + if err != nil { + t.Fatalf("restore should not need --confirm — it is not destructive: %v", err) + } +} + +// TestDeleteConfirmMakesNoRequest proves the --confirm gate is a client-side +// short-circuit, not merely an error surfaced after a request. A stub that +// records whether it was hit lets this test fail if the gate is ever moved +// after the HTTP call (e.g. reordered to check the response first). +func TestDeleteConfirmMakesNoRequest(t *testing.T) { + hit := false + _, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + hit = true + w.WriteHeader(http.StatusNoContent) + }, "delete", "abc") + if err == nil { + t.Fatal("expected an error without --confirm") + } + if hit { + t.Error("server was hit without --confirm — the gate must short-circuit before any HTTP request") + } +} diff --git a/cmd/customerprofile/get.go b/cmd/customerprofile/get.go new file mode 100644 index 0000000..e75a05f --- /dev/null +++ b/cmd/customerprofile/get.go @@ -0,0 +1,34 @@ +package customerprofile + +import ( + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" +) + +func init() { Cmd.AddCommand(getCmd) } + +var getCmd = &cobra.Command{ + Use: "get ", + Short: "Get a customer profile", + Long: "Shows one customer profile. Soft-deleted profiles are still retrievable here and report softDeleted: true.", + Example: ` band customer-profile get 3IIzIFnRRQBE3AMzPpMTNo --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.Get(args[0]) + if err != nil { + return roleGateError(err) + } + obj, err := env.Object() + if err != nil { + return err + } + format, plain := cmdutil.OutputFlags(cmd) + return output.StdoutAuto(format, plain, obj) + }, +} diff --git a/cmd/customerprofile/helpers.go b/cmd/customerprofile/helpers.go new file mode 100644 index 0000000..a0187e5 --- /dev/null +++ b/cmd/customerprofile/helpers.go @@ -0,0 +1,57 @@ +package customerprofile + +import ( + "errors" + + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// roleGateError maps a 403 from the customer-profile endpoints onto an +// actionable exit 4, instead of letting it fall through to ExitCodeForError's +// default 401/403 -> ExitAuth (2) mapping. +// +// The common cause of a 403 here is that the active credential lacks the +// Customer Profiles Access role (see AGENTS.md's Customer Profiles section) — +// re-authenticating will not fix that, so exit 2's "reauth" signal would send +// an agent down a dead end. Modeled on cmd/tendlc/helpers.go's roleGateError, +// which solved the same problem for 10DLC. +// +// Non-403 errors, and errors that are not (or no longer, once already +// wrapped by something like conflictHint) an *api.APIError, pass through +// unchanged. +func roleGateError(err error) error { + var apiErr *api.APIError + if err == nil || !errors.As(err, &apiErr) || apiErr.StatusCode != 403 { + return err + } + return cmdutil.NewFeatureLimit( + "your credentials don't have the Customer Profiles Access role.\n"+ + "Contact your Bandwidth account manager to have it assigned to your API user — retrying will not help.", + err) +} + +// warnIfTruncated tells the caller on stderr when more records exist than the +// page just returned. stdout stays clean so a pipeline sees only data. Shared +// by 'list' (noun "profiles") and 'history list' (noun "versions") — they +// differ only in which offset they paginate on and what they call a record. +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) + } +} + +// requireConfirm enforces a --confirm gate before any HTTP request is made. +// Centralized so a future destructive command reuses the same +// zero-request-on-refusal guarantee instead of reimplementing the check +// inline next to its own service(cmd) call — and so the gate itself is +// testable independent of any one command's wiring. +func requireConfirm(confirm bool, message string) error { + if confirm { + return nil + } + return cmdutil.NewFlagError(message) +} diff --git a/cmd/customerprofile/helpers_test.go b/cmd/customerprofile/helpers_test.go new file mode 100644 index 0000000..8800188 --- /dev/null +++ b/cmd/customerprofile/helpers_test.go @@ -0,0 +1,131 @@ +package customerprofile + +import ( + "net/http" + "strings" + "testing" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" +) + +func TestRoleGateErrorMapsForbiddenToExitConflict(t *testing.T) { + err := roleGateError(&api.APIError{StatusCode: 403, Body: "does not have access rights"}) + if err == nil { + t.Fatal("expected an error") + } + if got := cmdutil.ExitCodeForError(err); got != cmdutil.ExitConflict { + t.Errorf("exit code = %d, want %d (ExitConflict) — a missing role must not read as a reauth prompt", got, cmdutil.ExitConflict) + } + msg := err.Error() + if !strings.Contains(msg, "Customer Profiles Access") { + t.Errorf("message = %q, want it to name the Customer Profiles Access role", msg) + } + if !strings.Contains(msg, "account manager") { + t.Errorf("message = %q, want it to say an account manager must grant the role", msg) + } + if !strings.Contains(msg, "retrying will not help") { + t.Errorf("message = %q, want it to say retrying will not help", msg) + } +} + +func TestRoleGateErrorPassesThroughNonForbidden(t *testing.T) { + orig := &api.APIError{StatusCode: 500, Body: "boom"} + err := roleGateError(orig) + if err != orig { + t.Errorf("err = %v, want the original 500 error passed through unchanged", err) + } + if got := cmdutil.ExitCodeForError(err); got != cmdutil.ExitGeneral { + t.Errorf("exit code = %d, want %d", got, cmdutil.ExitGeneral) + } +} + +func TestRoleGateErrorPassesThroughNil(t *testing.T) { + if err := roleGateError(nil); err != nil { + t.Errorf("err = %v, want nil", err) + } +} + +func TestRoleGateErrorPassesThroughNonAPIError(t *testing.T) { + orig := cmdutil.NewFlagError("some flag error") + if err := roleGateError(orig); err != orig { + t.Errorf("err = %v, want the non-APIError passed through unchanged", err) + } +} + +// TestServiceForbiddenExitsFour is the end-to-end guard: every command in +// this package must route a 403 from the service through roleGateError, not +// let it fall through to ExitCodeForError's raw 401/403 -> ExitAuth (2) +// mapping. A stray command that forgets this would send an agent chasing +// re-auth for a problem re-auth cannot fix. +func TestServiceForbiddenExitsFour(t *testing.T) { + forbidden := func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"errors":[{"description":"does not have access rights"}]}`)) + } + + cases := []struct { + name string + args []string + }{ + {"create", []string{"create", "--name", "Acme"}}, + {"list", []string{"list"}}, + {"get", []string{"get", "abc"}}, + {"delete", []string{"delete", "abc", "--confirm"}}, + {"history list", []string{"history", "list", "abc"}}, + {"history get", []string{"history", "get", "abc", "2"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := runCmd(t, forbidden, tc.args...) + if err == nil { + t.Fatal("expected an error on a 403 response") + } + if got := cmdutil.ExitCodeForError(err); got != cmdutil.ExitConflict { + t.Errorf("exit code = %d, want %d (ExitConflict)", got, cmdutil.ExitConflict) + } + if !strings.Contains(err.Error(), "Customer Profiles Access") { + t.Errorf("error = %q, want an actionable message naming the role", err.Error()) + } + }) + } +} + +// update and restore both GET before they write, so their 403 case is +// exercised separately: the stub must answer the GET with 403 directly +// (there is nothing to overlay yet). +func TestUpdateAndRestoreForbiddenOnGetExitsFour(t *testing.T) { + forbidden := func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"errors":[{"description":"does not have access rights"}]}`)) + } + + for _, args := range [][]string{ + {"update", "abc", "--name", "New"}, + {"restore", "abc"}, + } { + _, err := runCmd(t, forbidden, args...) + if err == nil { + t.Fatalf("%v: expected an error on a 403 response", args) + } + if got := cmdutil.ExitCodeForError(err); got != cmdutil.ExitConflict { + t.Errorf("%v: exit code = %d, want %d (ExitConflict)", args, got, cmdutil.ExitConflict) + } + } +} + +func TestRequireConfirmRejectsWithoutConfirm(t *testing.T) { + err := requireConfirm(false, "pass --confirm to proceed") + if err == nil { + t.Fatal("expected an error when confirm is false") + } + if got := cmdutil.ExitCodeForError(err); got != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d (ExitFlagError) — no request should have been made", got, cmdutil.ExitFlagError) + } +} + +func TestRequireConfirmAllowsWithConfirm(t *testing.T) { + if err := requireConfirm(true, "pass --confirm to proceed"); err != nil { + t.Errorf("err = %v, want nil when confirm is true", err) + } +} diff --git a/cmd/customerprofile/history.go b/cmd/customerprofile/history.go new file mode 100644 index 0000000..6dd64b4 --- /dev/null +++ b/cmd/customerprofile/history.go @@ -0,0 +1,115 @@ +package customerprofile + +import ( + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" +) + +var ( + historyLimit int + historyOffset int + historyAll bool +) + +func init() { + f := historyListCmd.Flags() + f.IntVar(&historyLimit, "limit", 50, "Page size") + f.IntVar(&historyOffset, "offset", 0, "Pagination offset") + f.BoolVar(&historyAll, "all", false, "Fetch every page (cannot be combined with --offset)") + + historyCmd.AddCommand(historyListCmd) + historyCmd.AddCommand(historyGetCmd) + Cmd.AddCommand(historyCmd) +} + +var historyCmd = &cobra.Command{ + Use: "history", + Short: "Inspect a customer profile's version history", +} + +var historyListCmd = &cobra.Command{ + Use: "list ", + Short: "List a customer profile's versions", + Long: `Lists every recorded version of a customer profile, newest first. Always returns an array. + +Each entry nests the profile snapshot under "data" and audit fields under +"metadata" — the version number is at metadata.version, not top-level the way +it is on 'customer-profile get'. Observed metadata.operation values are +CREATED, UPDATED, and DELETED.`, + Example: ` band customer-profile history list 3IIzIFnRRQBE3AMzPpMTNo --plain + # [{"data":{"id":"...","name":"Acme"},"metadata":{"version":2,"operation":"UPDATED","userName":"...","createdDate":"..."}}, ...]`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if historyAll && 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 !historyAll { + env, err := svc.History(args[0], historyLimit, historyOffset) + if err != nil { + return roleGateError(err) + } + items, err := env.List() + if err != nil { + return err + } + warnIfTruncated(cmd, env, historyOffset, len(items), "versions") + return output.StdoutPlainList(format, plain, items) + } + + var all []any + err = api.ForEachPage(func(limit, offset int) (*api.Envelope, error) { + return svc.History(args[0], limit, offset) + }, historyLimit, func(batch []any) error { + all = append(all, batch...) + return nil + }) + if err != nil { + return roleGateError(err) + } + if all == nil { + all = []any{} + } + return output.StdoutPlainList(format, plain, all) + }, +} + +var historyGetCmd = &cobra.Command{ + Use: "get ", + Short: "Get one version of a customer profile", + Long: `Shows a single historical version of a customer profile. + +The response nests the profile snapshot under "data" and audit fields under +"metadata" — the version number is at metadata.version, not top-level the way +it is on 'customer-profile get'. + +Separate from 'history list' so the --plain shape never depends on argument +count: list always returns an array, get always returns an object.`, + Example: ` band customer-profile history get 3IIzIFnRRQBE3AMzPpMTNo 2 --plain + # {"data":{"id":"...","name":"Acme"},"metadata":{"version":2,"operation":"UPDATED","userName":"...","createdDate":"..."}}`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + svc, err := service(cmd) + if err != nil { + return err + } + env, err := svc.HistoryVersion(args[0], args[1]) + if err != nil { + return roleGateError(err) + } + obj, err := env.Object() + if err != nil { + return err + } + format, plain := cmdutil.OutputFlags(cmd) + return output.StdoutAuto(format, plain, obj) + }, +} diff --git a/cmd/customerprofile/history_test.go b/cmd/customerprofile/history_test.go new file mode 100644 index 0000000..6478899 --- /dev/null +++ b/cmd/customerprofile/history_test.go @@ -0,0 +1,105 @@ +package customerprofile + +import ( + "net/http" + "strings" + "testing" +) + +// Split into list and get so --plain output shape never depends on whether an +// optional argument was supplied: list is always an array, get always an object. +func TestHistoryListReturnsArray(t *testing.T) { + out, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[ + {"data":{"id":"abc","name":"Acme"},"metadata":{"version":1,"operation":"CREATED","userName":"someone","createdDate":"2026-01-01T00:00:00Z"}}, + {"data":{"id":"abc","name":"Acme Renamed"},"metadata":{"version":2,"operation":"UPDATED","userName":"someone","createdDate":"2026-01-02T00:00:00Z"}} + ],"page":{"pageSize":50,"totalElements":2}}`)) + }, "history", "list", "abc", "--plain") + if err != nil { + t.Fatalf("history list: %v", err) + } + if !strings.HasPrefix(strings.TrimSpace(out), "[") { + t.Errorf("stdout = %q, want a JSON array", out) + } +} + +// The real API nests the profile snapshot under "data" and audit fields under +// "metadata" — the version lives at metadata.version, not top-level, unlike +// 'customer-profile get'. +func TestHistoryGetReturnsObject(t *testing.T) { + var gotPath string + out, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + _, _ = w.Write([]byte(`{"data":{"data":{"id":"abc","name":"Acme"},"metadata":{"version":2,"operation":"UPDATED","userName":"someone","createdDate":"2026-01-02T00:00:00Z"}}}`)) + }, "history", "get", "abc", "2", "--plain") + if err != nil { + t.Fatalf("history get: %v", err) + } + if !strings.HasPrefix(strings.TrimSpace(out), "{") { + t.Errorf("stdout = %q, want a JSON object", out) + } + if !strings.Contains(out, `"version":2`) && !strings.Contains(out, `"version": 2`) { + t.Errorf("stdout = %q, want metadata.version at 2", out) + } + if !strings.HasSuffix(gotPath, "/history/2") { + t.Errorf("path = %q, want it to end in /history/2", gotPath) + } +} + +func TestHistoryGetRequiresBothArgs(t *testing.T) { + if _, err := runCmd(t, nil, "history", "get", "abc"); err == nil { + t.Fatal("expected an error when the version argument is missing") + } +} + +func TestHistoryListAllWalksPages(t *testing.T) { + calls := 0 + out, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + calls++ + if calls == 1 { + _, _ = w.Write([]byte(`{"data":[{"data":{"id":"abc"},"metadata":{"version":1,"operation":"CREATED"}}],"page":{"pageSize":1,"totalElements":2}}`)) + return + } + _, _ = w.Write([]byte(`{"data":[{"data":{"id":"abc"},"metadata":{"version":2,"operation":"UPDATED"}}],"page":{"pageSize":1,"totalElements":2}}`)) + }, "history", "list", "abc", "--all", "--limit", "1", "--plain") + if err != nil { + t.Fatalf("history list --all: %v", err) + } + if calls != 2 { + t.Errorf("fetched %d pages, want 2", calls) + } + if !strings.Contains(out, `"version":1`) && !strings.Contains(out, `"version": 1`) { + t.Errorf("stdout = %q, want items from the first page", out) + } + if !strings.Contains(out, `"version":2`) && !strings.Contains(out, `"version": 2`) { + t.Errorf("stdout = %q, want items from the second page too", out) + } +} + +// TestHistoryListWarnsWhenTruncated guards against list.go's warnIfTruncated +// pattern silently not being mirrored here: a paginated, non-`--all` history +// list must warn on stderr when more versions exist than the page returned, +// exactly like 'customer-profile list' does. +func TestHistoryListWarnsWhenTruncated(t *testing.T) { + _, stderr, err := runCmdCapturingStderr(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[{"data":{"id":"abc"},"metadata":{"version":1}}],"page":{"pageSize":1,"totalElements":2}}`)) + }, "history", "list", "abc", "--limit", "1", "--plain") + if err != nil { + t.Fatalf("history list: %v", err) + } + if !strings.Contains(stderr, "pass --all to fetch every page") { + t.Errorf("stderr = %q, want a truncation warning", stderr) + } +} + +func TestHistoryListNoWarningWhenNotTruncated(t *testing.T) { + _, stderr, err := runCmdCapturingStderr(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[{"data":{"id":"abc"},"metadata":{"version":1}},{"data":{"id":"abc"},"metadata":{"version":2}}],"page":{"pageSize":50,"totalElements":2}}`)) + }, "history", "list", "abc", "--plain") + if err != nil { + t.Fatalf("history list: %v", err) + } + if stderr != "" { + t.Errorf("stderr = %q, want no truncation warning", stderr) + } +} diff --git a/cmd/customerprofile/list.go b/cmd/customerprofile/list.go new file mode 100644 index 0000000..c8ad708 --- /dev/null +++ b/cmd/customerprofile/list.go @@ -0,0 +1,81 @@ +package customerprofile + +import ( + "github.com/spf13/cobra" + + "github.com/Bandwidth/cli/internal/api" + "github.com/Bandwidth/cli/internal/cmdutil" + "github.com/Bandwidth/cli/internal/output" +) + +var ( + listLimit int + listOffset int + listAll bool + listNameContains string +) + +func init() { + f := listCmd.Flags() + f.IntVar(&listLimit, "limit", 50, "Page size") + f.IntVar(&listOffset, "offset", 0, "Pagination offset") + f.BoolVar(&listAll, "all", false, "Fetch every page (cannot be combined with --offset)") + f.StringVar(&listNameContains, "name-contains", "", "Filter by profile name substring") + Cmd.AddCommand(listCmd) +} + +var listCmd = &cobra.Command{ + Use: "list", + Short: "List customer profiles", + Long: "Lists customer profiles on the account. Soft-deleted profiles are excluded from this listing but remain retrievable by ID with 'customer-profile get'.", + Example: ` band customer-profile list --plain + band customer-profile list --all --plain + band customer-profile list --name-contains Acme --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 listAll && 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 + } + + var filters []api.Filter + if listNameContains != "" { + filters = append(filters, api.Filter{Field: "name", Op: api.OpContains, Value: listNameContains}) + } + + format, plain := cmdutil.OutputFlags(cmd) + + if !listAll { + env, err := svc.List(listLimit, listOffset, filters) + if err != nil { + return roleGateError(err) + } + items, err := env.List() + if err != nil { + return err + } + warnIfTruncated(cmd, env, listOffset, len(items), "profiles") + return output.StdoutPlainList(format, plain, items) + } + + var all []any + err = api.ForEachPage(func(limit, offset int) (*api.Envelope, error) { + return svc.List(limit, offset, filters) + }, listLimit, func(batch []any) error { + all = append(all, batch...) + return nil + }) + if err != nil { + return roleGateError(err) + } + if all == nil { + all = []any{} + } + return output.StdoutPlainList(format, plain, all) + }, +} diff --git a/cmd/customerprofile/update.go b/cmd/customerprofile/update.go new file mode 100644 index 0000000..774d27c --- /dev/null +++ b/cmd/customerprofile/update.go @@ -0,0 +1,114 @@ +package customerprofile + +import ( + "errors" + + "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" +) + +var updateOpts cpsvc.UpdateOptions + +// updateFieldFlags are the flags that carry a field value, in CLI naming. +// BuildUpdateRequest keys its changed-map on exactly these names. +var updateFieldFlags = []string{"name", "website", "contact-name", "contact-phone", "contact-email", "address-id"} + +func init() { + f := updateCmd.Flags() + f.StringVar(&updateOpts.Name, "name", "", "Profile name") + f.StringVar(&updateOpts.Website, "website", "", "Business website URL") + f.StringVar(&updateOpts.ContactName, "contact-name", "", "Contact name") + f.StringVar(&updateOpts.ContactPhone, "contact-phone", "", "Contact phone in E.164") + f.StringVar(&updateOpts.ContactEmail, "contact-email", "", "Contact email") + f.StringVar(&updateOpts.AddressID, "address-id", "", "Address ID to associate") + Cmd.AddCommand(updateCmd) +} + +var updateCmd = &cobra.Command{ + Use: "update ", + Short: "Update a customer profile", + Long: `Updates a customer profile. + +The API replaces the whole record on update, so this command reads the profile +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. + +Because the read and the write are two requests, a concurrent edit between them +is rejected by the API's version check — the command exits 4 and you can retry.`, + Example: ` band customer-profile update 3IIzIFnRRQBE3AMzPpMTNo --name "Acme Corp" --plain + band customer-profile update 3IIzIFnRRQBE3AMzPpMTNo --website "" --plain # clear the website`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + changed := map[string]bool{} + any := false + for _, name := range updateFieldFlags { + if cmd.Flags().Changed(name) { + changed[name] = true + any = true + } + } + if !any { + return cmdutil.NewFlagError( + "nothing to update — pass at least one of " + flagList(updateFieldFlags)) + } + + svc, err := service(cmd) + if err != nil { + return err + } + + env, err := svc.Get(args[0]) + if err != nil { + return roleGateError(err) + } + current, err := env.Object() + if err != nil { + return err + } + + body, err := cpsvc.BuildUpdateRequest(current, updateOpts, changed) + if err != nil { + return err + } + + updated, err := svc.Update(args[0], body) + if err != nil { + return roleGateError(conflictHint(err)) + } + obj, err := updated.Object() + if err != nil { + return err + } + format, plain := cmdutil.OutputFlags(cmd) + return output.StdoutAuto(format, plain, obj) + }, +} + +// conflictHint turns the API's version conflict into an actionable exit 4. +// The same 409 is returned when version is stale and when it is missing, so +// the message covers the case the caller can actually act on. +func conflictHint(err error) error { + var apiErr *api.APIError + if errors.As(err, &apiErr) && apiErr.StatusCode == 409 { + return &cmdutil.ConflictError{ + Message: "this profile was modified by someone else while the update was in flight; retry the command", + Cause: err, + } + } + return err +} + +func flagList(names []string) string { + out := "" + for i, n := range names { + if i > 0 { + out += ", " + } + out += "--" + n + } + return out +} diff --git a/cmd/customerprofile/update_test.go b/cmd/customerprofile/update_test.go new file mode 100644 index 0000000..fd9910e --- /dev/null +++ b/cmd/customerprofile/update_test.go @@ -0,0 +1,151 @@ +package customerprofile + +import ( + "encoding/json" + "io" + "net/http" + "strings" + "testing" +) + +// The regression lock for this whole PR: an update must not delete a field the +// CLI does not model. PUT is a full replacement, so a dropped field is nulled. +func TestUpdatePreservesUnmodeledFields(t *testing.T) { + var putBody map[string]any + _, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`{"data":{"id":"abc","name":"Acme","website":"https://acme.com", + "version":3,"futureField":"keep me","totalCampaigns":2}}`)) + return + } + raw, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(raw, &putBody) + _, _ = w.Write([]byte(`{"data":{"id":"abc","version":4}}`)) + }, "update", "abc", "--name", "Acme Renamed", "--plain") + if err != nil { + t.Fatalf("update: %v", err) + } + if putBody["futureField"] != "keep me" { + t.Errorf("unmodeled field dropped from PUT — it would be nulled server-side: %#v", putBody) + } + if putBody["website"] != "https://acme.com" { + t.Errorf("unchanged website dropped: %#v", putBody) + } + if putBody["name"] != "Acme Renamed" { + t.Errorf("name = %v, want the new value", putBody["name"]) + } + if putBody["version"] == nil { + t.Error("version missing from PUT — the API rejects updates without it") + } + if _, present := putBody["totalCampaigns"]; present { + t.Error("read-only totalCampaigns should be stripped before PUT") + } +} + +func TestUpdateReadsBeforeWriting(t *testing.T) { + var methods []string + _, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + methods = append(methods, r.Method) + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`{"data":{"id":"abc","name":"Acme","version":1}}`)) + return + } + _, _ = w.Write([]byte(`{"data":{"id":"abc","version":2}}`)) + }, "update", "abc", "--website", "https://new.example", "--plain") + if err != nil { + t.Fatalf("update: %v", err) + } + if len(methods) != 2 || methods[0] != http.MethodGet || methods[1] != http.MethodPut { + t.Errorf("request sequence = %v, want [GET PUT]", methods) + } +} + +// The API rejects an empty string on website ("size must be between 1 and +// 500") but accepts and applies JSON null, measured against production +// (account 9901287). The documented way to clear a field is passing the flag +// with an empty value, so that must reach the server as null, not "". This +// asserts on the JSON the server received in the PUT, not on what was passed +// in — the same standard as TestUpdatePreservesUnmodeledFields. +func TestUpdateClearFieldSendsNull(t *testing.T) { + var putBody map[string]any + _, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`{"data":{"id":"abc","name":"Acme","website":"https://acme.com","version":1}}`)) + return + } + raw, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(raw, &putBody) + _, _ = w.Write([]byte(`{"data":{"id":"abc","version":2}}`)) + }, "update", "abc", "--website", "", "--plain") + if err != nil { + t.Fatalf("update: %v", err) + } + val, present := putBody["website"] + if !present { + t.Fatal("website key missing from PUT body, want it present with a null value") + } + if val != nil { + t.Errorf("website = %v, want explicit null so the API clears the field", val) + } +} + +func TestUpdateWithNoFlagsIsAnError(t *testing.T) { + _, err := runCmd(t, nil, "update", "abc") + if err == nil { + t.Fatal("expected an error: update with no field flags would be a no-op round trip") + } + if !strings.Contains(err.Error(), "nothing to update") { + t.Errorf("error = %q, want it to say nothing was requested", err.Error()) + } +} + +// Confirmed live against production: --name "" overlays a null name onto the +// PUT body, and the API answers with a raw 400 "name must not be null". That +// must be caught locally instead — a FlagError (exit 6) with zero writes. +func TestUpdateEmptyNameExitsSixWithNoWrite(t *testing.T) { + putCalled := false + _, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPut { + putCalled = true + } + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`{"data":{"id":"abc","name":"Acme","version":1}}`)) + return + } + _, _ = w.Write([]byte(`{"data":{"id":"abc","version":2}}`)) + }, "update", "abc", "--name", "", "--plain") + if err == nil { + t.Fatal("expected an error when --name is cleared to empty") + } + if got := exitCodeOf(err); got != 6 { + t.Errorf("exit code = %d, want 6", got) + } + if !strings.Contains(err.Error(), "name") { + t.Errorf("error = %q, want it to name the field", err.Error()) + } + if putCalled { + t.Error("PUT was issued despite an invalid update body — validation must happen before any write") + } +} + +// A 409 means someone else wrote between our GET and PUT. That is a conflict +// the caller can resolve by retrying, so it must exit 4, not 1. +func TestUpdateConflictExitsFour(t *testing.T) { + _, err := runCmd(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`{"data":{"id":"abc","name":"Acme","version":1}}`)) + return + } + w.WriteHeader(http.StatusConflict) + _, _ = w.Write([]byte(`{"errors":[{"description":"entity has been modified by another process or user"}]}`)) + }, "update", "abc", "--name", "X", "--plain") + if err == nil { + t.Fatal("expected a conflict error") + } + if got := exitCodeOf(err); got != 4 { + t.Errorf("exit code = %d, want 4 (conflict)", got) + } + if !strings.Contains(err.Error(), "retry") { + t.Errorf("error = %q, want it to tell the caller to retry", err.Error()) + } +} diff --git a/cmd/root.go b/cmd/root.go index fb51c8f..c2b0d43 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -21,6 +21,7 @@ import ( authcmd "github.com/Bandwidth/cli/cmd/auth" bxmlcmd "github.com/Bandwidth/cli/cmd/bxml" callcmd "github.com/Bandwidth/cli/cmd/call" + customerprofilecmd "github.com/Bandwidth/cli/cmd/customerprofile" locationcmd "github.com/Bandwidth/cli/cmd/location" messagecmd "github.com/Bandwidth/cli/cmd/message" numbercmd "github.com/Bandwidth/cli/cmd/number" @@ -102,6 +103,7 @@ func init() { rootCmd.AddCommand(appcmd.Cmd) rootCmd.AddCommand(numbercmd.Cmd) rootCmd.AddCommand(callcmd.Cmd) + rootCmd.AddCommand(customerprofilecmd.Cmd) rootCmd.AddCommand(messagecmd.Cmd) rootCmd.AddCommand(recordingcmd.Cmd) rootCmd.AddCommand(transcriptioncmd.Cmd) diff --git a/go.mod b/go.mod index edb6485..98f3f50 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/fatih/color v1.19.0 github.com/olekukonko/tablewriter v0.0.5 github.com/spf13/cobra v1.8.1 + github.com/spf13/pflag v1.0.5 github.com/zalando/go-keyring v0.2.5 golang.org/x/term v0.41.0 ) @@ -23,7 +24,6 @@ require ( github.com/mattn/go-runewidth v0.0.9 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect - github.com/spf13/pflag v1.0.5 // indirect github.com/stretchr/testify v1.9.0 // indirect golang.org/x/sys v0.42.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/internal/api/client.go b/internal/api/client.go index 3fd8e99..20a6c8f 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -284,6 +284,36 @@ func (c *Client) PutRaw(path string, data []byte, contentType string) error { return err } +// PostRaw posts a JSON body and returns the raw response bytes, so callers can +// parse an envelope without a typed target. JSON-only: returns an error +// without making a request if c is configured for XML (see NewXMLClient). +func (c *Client) PostRaw(path string, body interface{}) ([]byte, error) { + return c.doRawJSON("POST", path, body) +} + +// PutRawJSON puts a JSON body and returns the raw response bytes. JSON-only: +// returns an error without making a request if c is configured for XML (see +// NewXMLClient). +func (c *Client) PutRawJSON(path string, body interface{}) ([]byte, error) { + return c.doRawJSON("PUT", path, body) +} + +func (c *Client) doRawJSON(method, path string, body interface{}) ([]byte, error) { + if c.contentType == "xml" { + return nil, fmt.Errorf("PostRaw/PutRawJSON send JSON; this client is configured for XML (use the XML methods instead)") + } + data, err := json.Marshal(body) + if err != nil { + return nil, fmt.Errorf("encoding request body: %w", err) + } + req, err := c.newRequest(method, path, bytes.NewReader(data)) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/json") + return c.doRaw(req) +} + // PostXMLReturnLocation performs a POST with an XML body and returns the // Location response header. Useful for endpoints that respond 201 Created // with an empty body and put the new resource's URL in Location (the diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 5dca980..fac1c2d 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -469,6 +469,33 @@ func TestXMLClient_NonXMLBodyReturnsError(t *testing.T) { } } +func TestPostRawAndPutRawJSON_RefuseXMLClient(t *testing.T) { + called := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + client := NewXMLClient(srv.URL, nil) + + if _, err := client.PostRaw("/", map[string]any{"a": "b"}); err == nil { + t.Error("PostRaw on an XML-configured client: want error, got nil") + } else if !strings.Contains(err.Error(), "XML") { + t.Errorf("PostRaw error = %q, want mention of XML", err) + } + + if _, err := client.PutRawJSON("/", map[string]any{"a": "b"}); err == nil { + t.Error("PutRawJSON on an XML-configured client: want error, got nil") + } else if !strings.Contains(err.Error(), "XML") { + t.Errorf("PutRawJSON error = %q, want mention of XML", err) + } + + if called { + t.Error("guard must fire before making an HTTP request, but the server was hit") + } +} + func TestAPIErrorCapturesHeaders(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Retry-After", "7") diff --git a/internal/api/pager.go b/internal/api/pager.go new file mode 100644 index 0000000..7f6b2e2 --- /dev/null +++ b/internal/api/pager.go @@ -0,0 +1,53 @@ +package api + +import "fmt" + +// PageFetcher retrieves one page. Implementations are usually a closure over +// a service method, e.g. func(l, o int) (*Envelope, error) { return svc.List(l, o, filters) }. +type PageFetcher func(limit, offset int) (*Envelope, error) + +// ForEachPage walks every page and hands each batch to fn. +// +// Termination is driven by the page block's totalElements, never by observing +// a short page: a full final page is indistinguishable from "more to come" +// otherwise, and a short page in the middle is legal. Callers therefore do not +// track a cumulative count themselves — getting that wrong is the failure mode +// Page.Truncated's doc comment warns about, and this loop exists so nobody has +// to get it right more than once. +// +// pageSize is passed through to fetch as-is. A pageSize of 0 or less is not +// modified: it is passed to the fetcher, which typically feeds it to EncodeQuery. +// EncodeQuery omits a non-positive limit from the query string, allowing the +// server to apply its own default page size. This matches EncodeQuery's +// convention and simplifies testing. +// +// Fails closed when a response carries no page metadata. Returning the first +// page as if it were the whole result would look like success. +func ForEachPage(fetch PageFetcher, pageSize int, fn func([]any) error) error { + seen := 0 + for { + env, err := fetch(pageSize, seen) + if err != nil { + return err + } + if env.Page == nil { + return fmt.Errorf("response has no page metadata; refusing to report a partial result as complete") + } + batch, err := env.List() + if err != nil { + return err + } + if err := fn(batch); err != nil { + return err + } + seen += len(batch) + if !env.Page.Truncated(seen) { + return nil + } + // A page that returns nothing while claiming more remain would spin forever. + if len(batch) == 0 { + return fmt.Errorf("page at offset %d returned no items, but the server reports %d total elements remaining to be fetched", + seen, env.Page.TotalElements) + } + } +} diff --git a/internal/api/pager_test.go b/internal/api/pager_test.go new file mode 100644 index 0000000..380910c --- /dev/null +++ b/internal/api/pager_test.go @@ -0,0 +1,143 @@ +package api + +import ( + "encoding/json" + "errors" + "fmt" + "testing" +) + +// envFor builds an Envelope like the API returns: a data array plus page metadata. +func envFor(t *testing.T, items int, offset, total int) *Envelope { + t.Helper() + arr := make([]any, 0, items) + for i := 0; i < items; i++ { + arr = append(arr, map[string]any{"id": fmt.Sprintf("p%d", offset+i)}) + } + body, _ := json.Marshal(map[string]any{ + "data": arr, + "page": map[string]any{"pageSize": items, "totalElements": total}, + }) + env, err := ParseEnvelope(body) + if err != nil { + t.Fatalf("ParseEnvelope: %v", err) + } + return env +} + +func TestForEachPageWalksEveryPage(t *testing.T) { + var offsets []int + var seen []string + err := ForEachPage(func(limit, offset int) (*Envelope, error) { + offsets = append(offsets, offset) + switch offset { + case 0: + return envFor(t, 2, 0, 5), nil + case 2: + return envFor(t, 2, 2, 5), nil + default: + return envFor(t, 1, 4, 5), nil + } + }, 2, func(batch []any) error { + for _, it := range batch { + seen = append(seen, it.(map[string]any)["id"].(string)) + } + return nil + }) + if err != nil { + t.Fatalf("ForEachPage: %v", err) + } + if len(seen) != 5 { + t.Errorf("saw %d items, want 5: %v", len(seen), seen) + } + want := []int{0, 2, 4} + if fmt.Sprint(offsets) != fmt.Sprint(want) { + t.Errorf("offsets = %v, want %v", offsets, want) + } +} + +// Termination must come from totalElements, not from observing a short page. +// A full final page is the case a short-page heuristic gets wrong. +func TestForEachPageStopsOnExactMultiple(t *testing.T) { + calls := 0 + err := ForEachPage(func(limit, offset int) (*Envelope, error) { + calls++ + if calls > 3 { + t.Fatal("kept fetching past totalElements") + } + return envFor(t, 2, offset, 4), nil + }, 2, func([]any) error { return nil }) + if err != nil { + t.Fatalf("ForEachPage: %v", err) + } + if calls != 2 { + t.Errorf("fetched %d pages, want 2", calls) + } +} + +// Missing page metadata must fail closed rather than silently returning +// whatever the first page happened to contain. +func TestForEachPageFailsClosedWithoutPageMetadata(t *testing.T) { + env, _ := ParseEnvelope([]byte(`{"data":[{"id":"p0"}]}`)) + err := ForEachPage(func(limit, offset int) (*Envelope, error) { + return env, nil + }, 2, func([]any) error { return nil }) + if err == nil { + t.Fatal("expected an error when page metadata is absent") + } +} + +func TestForEachPagePropagatesFetchError(t *testing.T) { + boom := errors.New("boom") + err := ForEachPage(func(limit, offset int) (*Envelope, error) { + return nil, boom + }, 2, func([]any) error { return nil }) + if !errors.Is(err, boom) { + t.Fatalf("err = %v, want it to wrap boom", err) + } +} + +func TestForEachPagePropagatesCallbackError(t *testing.T) { + boom := errors.New("callback boom") + err := ForEachPage(func(limit, offset int) (*Envelope, error) { + return envFor(t, 2, 0, 10), nil + }, 2, func([]any) error { return boom }) + if !errors.Is(err, boom) { + t.Fatalf("err = %v, want it to wrap boom", err) + } +} + +func TestForEachPageEmptyResultIsNotAnError(t *testing.T) { + body := []byte(`{"data":[],"page":{"pageSize":50,"totalElements":0}}`) + env, _ := ParseEnvelope(body) + calls := 0 + err := ForEachPage(func(limit, offset int) (*Envelope, error) { + calls++ + return env, nil + }, 50, func([]any) error { return nil }) + if err != nil { + t.Fatalf("ForEachPage: %v", err) + } + if calls != 1 { + t.Errorf("fetched %d pages for an empty result, want 1", calls) + } +} + +// pageSize is passed through to fetch unchanged, including zero or negative +// values. This allows the fetcher to defer to the server's default via +// EncodeQuery, which omits non-positive limits from the query string. +func TestForEachPagePassesThroughPageSize(t *testing.T) { + var receivedSize int + body := []byte(`{"data":[],"page":{"pageSize":0,"totalElements":0}}`) + env, _ := ParseEnvelope(body) + err := ForEachPage(func(limit, offset int) (*Envelope, error) { + receivedSize = limit + return env, nil + }, 0, func([]any) error { return nil }) + if err != nil { + t.Fatalf("ForEachPage: %v", err) + } + if receivedSize != 0 { + t.Errorf("fetcher received pageSize %d, want 0", receivedSize) + } +} diff --git a/internal/customerprofile/options.go b/internal/customerprofile/options.go new file mode 100644 index 0000000..c023688 --- /dev/null +++ b/internal/customerprofile/options.go @@ -0,0 +1,210 @@ +package customerprofile + +import ( + "fmt" + + "github.com/Bandwidth/cli/internal/cmdutil" +) + +// readOnlyFields are rejected or ignored by the API on write. version is +// deliberately NOT in this list: the schema marks it readOnly, but production +// requires it on every update and returns 409 without it. +var readOnlyFields = []string{"id", "accountId", "createdDate", "modifiedDate", "totalCampaigns"} + +// CreateOptions is the flag surface of `customer-profile create`. +type CreateOptions struct { + Name string + Website string + ContactName string + ContactPhone string + ContactEmail string + AddressID string +} + +// UpdateOptions is the flag surface of `customer-profile 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 UpdateOptions struct { + Name string + Website string + ContactName string + ContactPhone string + ContactEmail string + AddressID string +} + +// ValidateCreate checks what can be checked locally. Semantic rules the API +// owns are left to the API — duplicating them here guarantees drift. +func ValidateCreate(o CreateOptions) error { + var missing []string + if o.Name == "" { + missing = append(missing, "name") + } + // The API's contact object requires a name whenever a contact is present. + if o.ContactName == "" && (o.ContactPhone != "" || o.ContactEmail != "") { + missing = append(missing, "contact-name") + } + if len(missing) > 0 { + return cmdutil.NewMissingFlagsError(missing) + } + return nil +} + +// BuildCreateRequest builds the POST body, omitting anything unset. An empty +// string is omitted rather than sent, so create never writes a blank over a +// server-side default. +func BuildCreateRequest(o CreateOptions) map[string]any { + body := map[string]any{"name": o.Name} + setIf(body, "website", o.Website) + setIf(body, "addressId", o.AddressID) + + contact := map[string]any{} + setIf(contact, "name", o.ContactName) + setIf(contact, "phoneNumber", o.ContactPhone) + setIf(contact, "email", o.ContactEmail) + if len(contact) > 0 { + body["contact"] = contact + } + return body +} + +// BuildUpdateRequest 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 — universalEin-style fields exist on +// other resources and will exist here eventually. 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 BuildUpdateRequest(current map[string]any, o UpdateOptions, changed map[string]bool) (map[string]any, error) { + if current == nil { + return nil, fmt.Errorf("no current resource to update from") + } + if _, ok := current["version"]; !ok { + return nil, fmt.Errorf("current resource has no version; the API rejects updates without it") + } + + body := deepCopyMap(current) + for _, ro := range readOnlyFields { + delete(body, ro) + } + + overlayIfChanged(body, changed, "name", "name", o.Name) + overlayIfChanged(body, changed, "website", "website", o.Website) + overlayIfChanged(body, changed, "address-id", "addressId", o.AddressID) + + if changed["contact-name"] || changed["contact-phone"] || changed["contact-email"] { + // body is already a deepCopyMap of current, so body["contact"] (if + // present) is already an independent copy — safe to mutate in place + // without recopying it key-by-key into a second new map. + contact, ok := body["contact"].(map[string]any) + if !ok { + contact = map[string]any{} + } + overlayIfChanged(contact, changed, "contact-name", "name", o.ContactName) + overlayIfChanged(contact, changed, "contact-phone", "phoneNumber", o.ContactPhone) + overlayIfChanged(contact, changed, "contact-email", "email", o.ContactEmail) + body["contact"] = contact + } + + if err := ValidateUpdate(body); err != nil { + return nil, err + } + return body, nil +} + +// ValidateUpdate checks the fully overlaid PUT body — the object about to go +// over the wire — not the options struct. A struct that looks fine in +// isolation (e.g. an empty --name with no other flags set) can still combine +// with the read profile into a body the API rejects. Catching that here means +// the failure is a local, zero-request FlagError (exit 6) instead of a raw +// 400 surfaced from the API side, measured against production: --name "" +// sends a null name and the API answers 400 "name must not be null". +// +// Kept as its own exported function, called from BuildUpdateRequest, so the +// check is independently testable and update.go's control flow (surface the +// error before calling svc.Update) falls out for free — BuildUpdateRequest +// already runs, and is already checked, before svc.Update. +func ValidateUpdate(body map[string]any) error { + if name, ok := body["name"].(string); !ok || name == "" { + return cmdutil.NewFlagError("name must not be empty or null") + } + if contact, ok := body["contact"].(map[string]any); ok { + if name, ok := contact["name"].(string); !ok || name == "" { + return cmdutil.NewFlagError("contact-name must not be empty when a contact is present") + } + } + return nil +} + +// BuildRestoreRequest undoes a soft delete. +// +// Sends softDeleted:false. The published docs say to send {"deleted": false}, +// which returns 404 "Customer profile not found" even though GET returns the +// record — measured against production, reported as MV-23429. +func BuildRestoreRequest(current map[string]any) (map[string]any, error) { + body, err := BuildUpdateRequest(current, UpdateOptions{}, map[string]bool{}) + if err != nil { + return nil, err + } + body["softDeleted"] = false + delete(body, "deleted") + return body, nil +} + +func setIf(m map[string]any, key, val string) { + if val != "" { + m[key] = val + } +} + +// overlayIfChanged writes val only when the caller explicitly set that flag. +// flagName is the CLI flag; field is the JSON key. +// +// An explicitly empty value is written as JSON null, not "". The API rejects +// an empty string on at least website ("size must be between 1 and 500"), +// measured against production, but accepts null and clears the field. Since +// the CLI's documented way to clear a field is passing the flag with an empty +// value, that value must become null on the wire. +func overlayIfChanged(m map[string]any, changed map[string]bool, flagName, field, val string) { + if !changed[flagName] { + return + } + if val == "" { + m[field] = nil + return + } + m[field] = val +} + +// deepCopyMap copies m so the result shares no mutable structure with it. +// current is read from an api.Envelope the caller may reuse or cache, so a +// shallow copy would leave nested maps (e.g. "contact") aliased between the +// outgoing body and the caller's data — any in-place mutation of one would +// silently corrupt the other. Nested map[string]any values, and []any slices +// that may themselves contain maps, are copied recursively; other values +// (strings, float64, bool, nil) are immutable in Go and safe to share. +func deepCopyMap(m map[string]any) map[string]any { + out := make(map[string]any, len(m)) + for k, v := range m { + out[k] = deepCopyValue(v) + } + return out +} + +func deepCopyValue(v any) any { + switch vv := v.(type) { + case map[string]any: + return deepCopyMap(vv) + case []any: + out := make([]any, len(vv)) + for i, e := range vv { + out[i] = deepCopyValue(e) + } + return out + default: + return v + } +} diff --git a/internal/customerprofile/options_test.go b/internal/customerprofile/options_test.go new file mode 100644 index 0000000..311d782 --- /dev/null +++ b/internal/customerprofile/options_test.go @@ -0,0 +1,294 @@ +package customerprofile + +import ( + "errors" + "strings" + "testing" + + "github.com/Bandwidth/cli/internal/cmdutil" +) + +func TestValidateCreateRequiresName(t *testing.T) { + err := ValidateCreate(CreateOptions{}) + if err == nil { + t.Fatal("expected an error when --name is missing") + } + var fe *cmdutil.FlagError + if !errors.As(err, &fe) { + t.Fatalf("error type = %T, want *cmdutil.FlagError so it exits 6", err) + } + if got := cmdutil.ExitCodeForError(err); got != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", got, cmdutil.ExitFlagError) + } +} + +// A contact is optional, but the API requires a name inside one if any +// contact field is supplied. Partial contacts must fail locally, not at the API. +func TestValidateCreateRejectsPartialContact(t *testing.T) { + err := ValidateCreate(CreateOptions{Name: "Acme", ContactEmail: "ops@acme.com"}) + if err == nil { + t.Fatal("expected an error when a contact field is set without --contact-name") + } + if got := cmdutil.ExitCodeForError(err); got != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", got, cmdutil.ExitFlagError) + } +} + +func TestValidateCreateAcceptsNameOnly(t *testing.T) { + if err := ValidateCreate(CreateOptions{Name: "Acme"}); err != nil { + t.Fatalf("name alone should be valid: %v", err) + } +} + +func TestBuildCreateRequestOmitsEmptyFields(t *testing.T) { + got := BuildCreateRequest(CreateOptions{Name: "Acme"}) + if got["name"] != "Acme" { + t.Errorf("name = %v", got["name"]) + } + if _, present := got["website"]; present { + t.Error("empty website should be omitted, not sent as an empty string") + } + if _, present := got["contact"]; present { + t.Error("no contact fields set, so contact should be omitted entirely") + } +} + +func TestBuildCreateRequestNestsContact(t *testing.T) { + got := BuildCreateRequest(CreateOptions{ + Name: "Acme", ContactName: "Ops", ContactEmail: "ops@acme.com"}) + c, ok := got["contact"].(map[string]any) + if !ok { + t.Fatalf("contact = %#v, want a nested object", got["contact"]) + } + if c["name"] != "Ops" || c["email"] != "ops@acme.com" { + t.Errorf("contact = %#v", c) + } + if _, present := c["phoneNumber"]; present { + t.Error("unset contact phone should be omitted") + } +} + +// THE CENTRAL TEST OF THIS PR. PUT is a full replacement, so anything the +// outgoing body omits is nulled server-side. Fields the CLI has never heard of +// must survive an update untouched. +func TestBuildUpdateRequestPreservesUnknownFields(t *testing.T) { + current := map[string]any{ + "id": "abc", + "accountId": "9901287", + "name": "Acme", + "website": "https://acme.com", + "version": float64(3), + "createdDate": "2026-01-01T00:00:00Z", + "modifiedDate": "2026-01-02T00:00:00Z", + "totalCampaigns": float64(2), + "softDeleted": false, + "futureField": "the CLI has never heard of this", + } + got, err := BuildUpdateRequest(current, UpdateOptions{Name: "Acme Renamed"}, + map[string]bool{"name": true}) + if err != nil { + t.Fatalf("BuildUpdateRequest: %v", err) + } + + if got["futureField"] != "the CLI has never heard of this" { + t.Errorf("unknown field was dropped — PUT would null it server-side: %#v", got) + } + if got["website"] != "https://acme.com" { + t.Errorf("unchanged website was dropped: %v", got["website"]) + } + if got["name"] != "Acme Renamed" { + t.Errorf("name = %v, want the changed value", got["name"]) + } + if got["version"] != float64(3) { + t.Errorf("version = %v, want it carried through — the API rejects updates without it", got["version"]) + } + for _, ro := range []string{"id", "accountId", "createdDate", "modifiedDate", "totalCampaigns"} { + if _, present := got[ro]; present { + t.Errorf("read-only field %q must be stripped before PUT", ro) + } + } +} + +// An unchanged flag must not overwrite a set value with empty string. +func TestBuildUpdateRequestIgnoresUnchangedFlags(t *testing.T) { + current := map[string]any{"name": "Acme", "website": "https://acme.com", "version": float64(1)} + got, err := BuildUpdateRequest(current, UpdateOptions{}, map[string]bool{}) + if err != nil { + t.Fatalf("BuildUpdateRequest: %v", err) + } + if got["website"] != "https://acme.com" { + t.Errorf("website = %v, want it untouched when --website was not passed", got["website"]) + } +} + +// Explicitly clearing a field is different from not passing it. +// The API rejects an empty string on website ("size must be between 1 and +// 500") but accepts and applies JSON null, measured against production. So an +// explicitly-passed empty flag must overlay as null, not "" — and the key +// must still be PRESENT in the body (not omitted): under a full-replacement +// PUT, an omitted key is nulled server-side too, but for the wrong reason, +// and that behavior is not guaranteed to hold if the API ever changes to a +// patch-style semantic. Presence is asserted with the two-value map lookup so +// this test would fail if a future change switched to `delete(body, field)`. +func TestBuildUpdateRequestAllowsExplicitClear(t *testing.T) { + current := map[string]any{"name": "Acme", "website": "https://acme.com", "version": float64(1)} + got, err := BuildUpdateRequest(current, UpdateOptions{Website: ""}, + map[string]bool{"website": true}) + if err != nil { + t.Fatalf("BuildUpdateRequest: %v", err) + } + val, present := got["website"] + if !present { + t.Fatal("website key missing from body, want it present with a null value") + } + if val != nil { + t.Errorf("website = %v, want explicit null", val) + } +} + +func TestBuildUpdateRequestRequiresVersion(t *testing.T) { + _, err := BuildUpdateRequest(map[string]any{"name": "Acme"}, UpdateOptions{}, map[string]bool{}) + if err == nil { + t.Fatal("expected an error when the current resource has no version") + } +} + +// BuildUpdateRequest must not hand back a body that shares nested mutable +// structure with current — current may be a cached resource the command +// layer reuses, and callers that mutate the returned body in place (rather +// than going through overlayIfChanged) must not corrupt it. +func TestBuildUpdateRequestDoesNotAliasNestedMaps(t *testing.T) { + contact := map[string]any{"name": "Ops", "email": "ops@acme.com"} + current := map[string]any{"name": "Acme", "version": float64(1), "contact": contact} + + got, err := BuildUpdateRequest(current, UpdateOptions{}, map[string]bool{}) + if err != nil { + t.Fatalf("BuildUpdateRequest: %v", err) + } + + gotContact, ok := got["contact"].(map[string]any) + if !ok { + t.Fatalf("contact = %#v, want a nested object", got["contact"]) + } + gotContact["email"] = "mutated@acme.com" + + if contact["email"] != "ops@acme.com" { + t.Errorf("mutating the returned body's contact changed current's contact: %#v", contact) + } +} + +// Partial contact updates must preserve the fields not being changed — this +// is the exact path Finding 1 touches, since the surviving contact fields +// come from a copy of current's nested contact map. +func TestBuildUpdateRequestPreservesUnchangedContactFields(t *testing.T) { + current := map[string]any{ + "name": "Acme", + "version": float64(1), + "contact": map[string]any{"name": "Ops", "phoneNumber": "+15555550100"}, + } + got, err := BuildUpdateRequest(current, UpdateOptions{ContactEmail: "new@acme.com"}, + map[string]bool{"contact-email": true}) + if err != nil { + t.Fatalf("BuildUpdateRequest: %v", err) + } + + c, ok := got["contact"].(map[string]any) + if !ok { + t.Fatalf("contact = %#v, want a nested object", got["contact"]) + } + if c["name"] != "Ops" { + t.Errorf("contact name = %v, want it preserved from current", c["name"]) + } + if c["phoneNumber"] != "+15555550100" { + t.Errorf("contact phoneNumber = %v, want it preserved from current", c["phoneNumber"]) + } + if c["email"] != "new@acme.com" { + t.Errorf("contact email = %v, want the newly set value", c["email"]) + } +} + +// Confirmed live against production (account 9901287): --name "" overlays a +// null name onto the PUT body, and the API answers 400 "name must not be +// null". ValidateUpdate must catch that locally, as a FlagError (exit 6), +// before the request is ever sent. +func TestBuildUpdateRequestRejectsClearedName(t *testing.T) { + current := map[string]any{"name": "Acme", "version": float64(1)} + _, err := BuildUpdateRequest(current, UpdateOptions{Name: ""}, map[string]bool{"name": true}) + if err == nil { + t.Fatal("expected an error when --name is cleared to empty") + } + var fe *cmdutil.FlagError + if !errors.As(err, &fe) { + t.Fatalf("error type = %T, want *cmdutil.FlagError so it exits 6", err) + } + if !strings.Contains(err.Error(), "name") { + t.Errorf("error = %q, want it to name the field", err.Error()) + } +} + +// A contact object with every one of its fields cleared, or a contact-name +// explicitly cleared while another contact field remains, must not reach the +// API without a contact name — the API's contact object requires one. +func TestBuildUpdateRequestRejectsContactWithoutName(t *testing.T) { + current := map[string]any{ + "name": "Acme", + "version": float64(1), + "contact": map[string]any{"name": "Ops", "email": "ops@acme.com"}, + } + _, err := BuildUpdateRequest(current, UpdateOptions{ContactName: ""}, + map[string]bool{"contact-name": true}) + if err == nil { + t.Fatal("expected an error when contact-name is cleared but the contact object remains") + } + if got := cmdutil.ExitCodeForError(err); got != cmdutil.ExitFlagError { + t.Errorf("exit code = %d, want %d", got, cmdutil.ExitFlagError) + } +} + +func TestValidateUpdateRejectsMissingName(t *testing.T) { + err := ValidateUpdate(map[string]any{"name": nil, "version": float64(1)}) + if err == nil { + t.Fatal("expected an error when name is null") + } + var fe *cmdutil.FlagError + if !errors.As(err, &fe) { + t.Fatalf("error type = %T, want *cmdutil.FlagError", err) + } +} + +func TestValidateUpdateRejectsContactWithoutName(t *testing.T) { + err := ValidateUpdate(map[string]any{ + "name": "Acme", + "contact": map[string]any{"email": "ops@acme.com"}, + }) + if err == nil { + t.Fatal("expected an error when contact has no name") + } +} + +func TestValidateUpdateAcceptsValidBody(t *testing.T) { + err := ValidateUpdate(map[string]any{ + "name": "Acme", + "contact": map[string]any{"name": "Ops", "email": "ops@acme.com"}, + }) + if err != nil { + t.Errorf("err = %v, want nil for a valid body", err) + } +} + +func TestBuildRestoreRequestClearsSoftDeleted(t *testing.T) { + current := map[string]any{"name": "Acme", "version": float64(3), "softDeleted": true, "id": "abc"} + got, err := BuildRestoreRequest(current) + if err != nil { + t.Fatalf("BuildRestoreRequest: %v", err) + } + if got["softDeleted"] != false { + t.Errorf("softDeleted = %v, want false", got["softDeleted"]) + } + if _, present := got["deleted"]; present { + t.Error(`must not send "deleted" — the documented form returns 404`) + } + if got["version"] != float64(3) { + t.Errorf("version = %v, want it carried through", got["version"]) + } +} diff --git a/internal/customerprofile/service.go b/internal/customerprofile/service.go index 3d40cec..de211ed 100644 --- a/internal/customerprofile/service.go +++ b/internal/customerprofile/service.go @@ -43,8 +43,8 @@ func (s *Service) List(limit, offset int, filters []api.Filter) (*api.Envelope, } // Get returns one customer profile. Soft-deleted profiles are still -// returned individually, with a "deleted" flag set — check it before -// creating any association. +// returned individually, with softDeleted set to true — check it before +// creating any association. There is no "deleted" field on reads. func (s *Service) Get(profileID string) (*api.Envelope, error) { if profileID == "" { return nil, fmt.Errorf("customer profile ID is required") diff --git a/internal/customerprofile/write.go b/internal/customerprofile/write.go new file mode 100644 index 0000000..2d5507c --- /dev/null +++ b/internal/customerprofile/write.go @@ -0,0 +1,66 @@ +package customerprofile + +import ( + "fmt" + "net/url" + + "github.com/Bandwidth/cli/internal/api" +) + +// Create posts a new customer profile. Callers build body via BuildCreateRequest. +func (s *Service) Create(body map[string]any) (*api.Envelope, error) { + raw, err := s.client.PostRaw(s.base(), body) + if err != nil { + return nil, err + } + return api.ParseEnvelope(raw) +} + +// Update replaces a customer profile. +// +// The API treats PUT as a FULL REPLACEMENT — a field omitted from body is set +// to null server-side — and requires the current version even though the +// schema marks version readOnly. Neither behavior is documented; both were +// measured against production. Callers must build body with +// BuildUpdateRequest, which starts from the current resource so nothing is +// dropped. +func (s *Service) Update(profileID string, body map[string]any) (*api.Envelope, error) { + if profileID == "" { + return nil, fmt.Errorf("customer profile ID is required") + } + raw, err := s.client.PutRawJSON(s.base()+"/"+url.PathEscape(profileID), body) + if err != nil { + return nil, err + } + return api.ParseEnvelope(raw) +} + +// Delete soft-deletes a customer profile. The record remains retrievable by ID +// with softDeleted set to true, and can be restored — see BuildRestoreRequest. +func (s *Service) Delete(profileID string) error { + if profileID == "" { + return fmt.Errorf("customer profile ID is required") + } + return s.client.Delete(s.base()+"/"+url.PathEscape(profileID), nil) +} + +// History returns the version history of a profile. +func (s *Service) History(profileID string, limit, offset int) (*api.Envelope, error) { + if profileID == "" { + return nil, fmt.Errorf("customer profile ID is required") + } + return s.get(s.base() + "/" + url.PathEscape(profileID) + "/history" + + api.EncodeQuery(limit, offset, nil)) +} + +// HistoryVersion returns one historical version of a profile. +func (s *Service) HistoryVersion(profileID, version string) (*api.Envelope, error) { + if profileID == "" { + return nil, fmt.Errorf("customer profile ID is required") + } + if version == "" { + return nil, fmt.Errorf("version is required") + } + return s.get(s.base() + "/" + url.PathEscape(profileID) + + "/history/" + url.PathEscape(version)) +} diff --git a/internal/customerprofile/write_test.go b/internal/customerprofile/write_test.go new file mode 100644 index 0000000..29e012d --- /dev/null +++ b/internal/customerprofile/write_test.go @@ -0,0 +1,179 @@ +package customerprofile + +import ( + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/Bandwidth/cli/internal/api" +) + +type captured struct { + method string + path string + body map[string]any +} + +func newCapturingService(t *testing.T, status int, respBody string) (*Service, *captured, func()) { + t.Helper() + cap := &captured{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + cap.method = r.Method + cap.path = r.URL.EscapedPath() + if raw, _ := io.ReadAll(r.Body); len(raw) > 0 { + _ = json.Unmarshal(raw, &cap.body) + } + w.WriteHeader(status) + _, _ = w.Write([]byte(respBody)) + })) + return NewService(api.NewClientNoAuth(srv.URL), "9901287"), cap, srv.Close +} + +func TestCreatePostsToCollection(t *testing.T) { + svc, cap, done := newCapturingService(t, 200, `{"data":{"id":"abc","version":0}}`) + defer done() + + env, err := svc.Create(map[string]any{"name": "Acme"}) + if err != nil { + t.Fatalf("Create: %v", err) + } + if cap.method != http.MethodPost { + t.Errorf("method = %s, want POST", cap.method) + } + if want := "/api/v2/accounts/9901287/customerProfiles"; cap.path != want { + t.Errorf("path = %q, want %q", cap.path, want) + } + if cap.body["name"] != "Acme" { + t.Errorf("body name = %v, want Acme", cap.body["name"]) + } + obj, err := env.Object() + if err != nil { + t.Fatalf("Object: %v", err) + } + if obj["id"] != "abc" { + t.Errorf("id = %v", obj["id"]) + } +} + +func TestUpdatePutsToResourceAndSendsBodyVerbatim(t *testing.T) { + svc, cap, done := newCapturingService(t, 200, `{"data":{"id":"abc","version":3}}`) + defer done() + + // An unknown field must reach the wire untouched — that is the whole + // point of building the payload from the read map. + body := map[string]any{"name": "Acme", "version": 2, "somethingWeNeverModeled": "keep me"} + if _, err := svc.Update("abc", body); err != nil { + t.Fatalf("Update: %v", err) + } + if cap.method != http.MethodPut { + t.Errorf("method = %s, want PUT", cap.method) + } + if want := "/api/v2/accounts/9901287/customerProfiles/abc"; cap.path != want { + t.Errorf("path = %q, want %q", cap.path, want) + } + if cap.body["somethingWeNeverModeled"] != "keep me" { + t.Errorf("unknown field was dropped: %#v", cap.body) + } +} + +func TestDeleteHitsResource(t *testing.T) { + svc, cap, done := newCapturingService(t, 204, ``) + defer done() + + if err := svc.Delete("abc"); err != nil { + t.Fatalf("Delete: %v", err) + } + if cap.method != http.MethodDelete { + t.Errorf("method = %s, want DELETE", cap.method) + } + if want := "/api/v2/accounts/9901287/customerProfiles/abc"; cap.path != want { + t.Errorf("path = %q, want %q", cap.path, want) + } +} + +func TestHistoryPathsAndPaging(t *testing.T) { + svc, cap, done := newCapturingService(t, 200, `{"data":[],"page":{"totalElements":0}}`) + defer done() + + if _, err := svc.History("abc", 10, 20); err != nil { + t.Fatalf("History: %v", err) + } + if want := "/api/v2/accounts/9901287/customerProfiles/abc/history"; cap.path != want { + t.Errorf("path = %q, want %q", cap.path, want) + } +} + +func TestHistoryVersionPath(t *testing.T) { + svc, cap, done := newCapturingService(t, 200, `{"data":{"version":2}}`) + defer done() + + if _, err := svc.HistoryVersion("abc", "2"); err != nil { + t.Fatalf("HistoryVersion: %v", err) + } + if want := "/api/v2/accounts/9901287/customerProfiles/abc/history/2"; cap.path != want { + t.Errorf("path = %q, want %q", cap.path, want) + } +} + +func TestWriteMethodsRequireAnID(t *testing.T) { + svc, _, done := newCapturingService(t, 200, `{}`) + defer done() + + if _, err := svc.Update("", map[string]any{}); err == nil { + t.Error("Update(\"\") should error before making a request") + } + if err := svc.Delete(""); err == nil { + t.Error("Delete(\"\") should error before making a request") + } + if _, err := svc.HistoryVersion("abc", ""); err == nil { + t.Error("HistoryVersion with empty version should error") + } +} + +func TestWriteMethodsPropagateAPIErrorType(t *testing.T) { + svc, _, done := newCapturingService(t, 409, + `{"errors":[{"description":"entity has been modified by another process or user"}]}`) + defer done() + + _, err := svc.Update("abc", map[string]any{"name": "x"}) + var apiErr *api.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("error type = %T, want *api.APIError to survive", err) + } + if apiErr.StatusCode != 409 { + t.Errorf("StatusCode = %d, want 409", apiErr.StatusCode) + } +} + +func TestUpdateEscapesID(t *testing.T) { + svc, cap, done := newCapturingService(t, 200, `{"data":{}}`) + defer done() + + _, _ = svc.Update("CP/../evil", map[string]any{"name": "x", "version": 1}) + if want := "/api/v2/accounts/9901287/customerProfiles/CP%2F..%2Fevil"; cap.path != want { + t.Errorf("escaped path = %q, want %q", cap.path, want) + } +} + +func TestDeleteEscapesID(t *testing.T) { + svc, cap, done := newCapturingService(t, 204, ``) + defer done() + + _ = svc.Delete("CP/../evil") + if want := "/api/v2/accounts/9901287/customerProfiles/CP%2F..%2Fevil"; cap.path != want { + t.Errorf("escaped path = %q, want %q", cap.path, want) + } +} + +func TestHistoryVersionEscapesIDAndVersion(t *testing.T) { + svc, cap, done := newCapturingService(t, 200, `{"data":{}}`) + defer done() + + _, _ = svc.HistoryVersion("CP/../evil", "v/../1") + if want := "/api/v2/accounts/9901287/customerProfiles/CP%2F..%2Fevil/history/v%2F..%2F1"; cap.path != want { + t.Errorf("escaped path = %q, want %q", cap.path, want) + } +}