From 2c97be57beec4cf40abc6ffd08594ef060dd6020 Mon Sep 17 00:00:00 2001 From: Yasser's studio Date: Sat, 13 Jun 2026 12:58:35 +0100 Subject: [PATCH] refactor(cli): consolidate the duplicated pick helpers into one shared util MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The accounts (generic pick), regions (pickWritable), and datasources (pickWritable) commands each had a copy of the same allowlist-pick that strips output-only fields from a parsed --file body before a PATCH. Move the generic pick to _shared.ts and have all three use it; regions/datasources keep their own field-list constants (now satisfies-guarded) and pass them in. Behavior-preserving — no command output or API payload changes. --- packages/cli/src/commands/_shared.ts | 16 +++++++++++++++ packages/cli/src/commands/accounts.ts | 16 +-------------- packages/cli/src/commands/datasources.ts | 20 +++++-------------- packages/cli/src/commands/regions.ts | 25 ++++++++---------------- 4 files changed, 30 insertions(+), 47 deletions(-) diff --git a/packages/cli/src/commands/_shared.ts b/packages/cli/src/commands/_shared.ts index 272de8f..994e453 100644 --- a/packages/cli/src/commands/_shared.ts +++ b/packages/cli/src/commands/_shared.ts @@ -169,6 +169,22 @@ export async function readJsonObject( return parsed as Record; } +/** + * Keep only the writable keys of a parsed `--file` body, dropping the output-only + * fields (`name`, `*Eligible`, `dataSourceId`, …) the API rejects in a PATCH + * `updateMask` — so a body saved from a `get` can be re-applied as-is. `fields` is + * constrained to keys of `T`, so the allowlist can't drift from the return type. + * Shared by the accounts / regions / datasources write commands; mirrors how + * `toProductInput` strips output-only product data. + */ +export function pick(obj: Record, fields: readonly (keyof T & string)[]): T { + const out: Record = {}; + for (const key of fields) { + if (key in obj) out[key] = obj[key]; + } + return out as T; +} + /** A feed file that couldn't be read or parsed as a product input. */ export interface FileLoadFailure { file: string; diff --git a/packages/cli/src/commands/accounts.ts b/packages/cli/src/commands/accounts.ts index 4ffb292..e397b3f 100644 --- a/packages/cli/src/commands/accounts.ts +++ b/packages/cli/src/commands/accounts.ts @@ -25,7 +25,7 @@ import { type CustomerService, } from "@gmc-cli/api"; import { contextFrom, wantsJson } from "../context.js"; -import { clientFor, resolveAccount, line, readJsonObject } from "./_shared.js"; +import { clientFor, resolveAccount, line, readJsonObject, pick } from "./_shared.js"; function accountIdOf(account: Account): string { return account.accountId ?? account.name.replace(/^accounts\//, ""); @@ -128,20 +128,6 @@ const AUTOFEED_FIELDS = [ "enableProducts", ] as const satisfies readonly (keyof AutofeedSettingsInput)[]; -/** - * Keep only the writable keys of a parsed `--file` body, dropping output-only fields - * the API rejects in a PATCH `updateMask`. Mirrors `pickWritable` in `regions.ts`, so a - * body saved from `accounts get`/`info` can be re-applied as-is. `fields` is constrained - * to keys of `T`, so the field list can't drift from the return type. - */ -function pick(obj: Record, fields: readonly (keyof T & string)[]): T { - const out: Record = {}; - for (const key of fields) { - if (key in obj) out[key] = obj[key]; - } - return out as T; -} - function renderHomepage(homepage: Homepage): void { line("URI", homepage.uri ?? "—"); line("Claimed", homepage.claimed ? "yes" : "no"); diff --git a/packages/cli/src/commands/datasources.ts b/packages/cli/src/commands/datasources.ts index b382fcd..75678c8 100644 --- a/packages/cli/src/commands/datasources.ts +++ b/packages/cli/src/commands/datasources.ts @@ -7,7 +7,7 @@ import { type PrimaryProductDataSource, } from "@gmc-cli/api"; import { contextFrom, wantsJson } from "../context.js"; -import { clientFor, resolveAccount, readJsonObject, line } from "./_shared.js"; +import { clientFor, resolveAccount, readJsonObject, line, pick } from "./_shared.js"; interface CreateFlags { name?: string; @@ -175,19 +175,6 @@ const DATASOURCE_WRITABLE_FIELDS = [ "fileInput", ] as const satisfies readonly (keyof DataSource)[]; -/** - * Keep only the writable keys of a parsed `--file` body, dropping the output-only - * `name`/`dataSourceId`/`input` the API rejects in a PATCH `updateMask`. Mirrors - * `pickWritable` in `regions.ts`, so a body saved from `datasources get` re-applies cleanly. - */ -function pickWritable(obj: Record): DataSource { - const out: Record = {}; - for (const key of DATASOURCE_WRITABLE_FIELDS) { - if (key in obj) out[key] = obj[key]; - } - return out as DataSource; -} - function dataSourceType(ds: DataSource): string { if (ds.primaryProductDataSource) return "primary product"; if (ds.supplementalProductDataSource) return "supplemental"; @@ -347,7 +334,10 @@ export function registerDataSourcesCommands(program: Command): void { // `update --name X` doesn't block on stdin in a non-TTY/CI context). let body: DataSource = {}; if (opts.file || (opts.name === undefined && !process.stdin.isTTY)) { - body = pickWritable(await readJsonObject(opts.file, "data source")); + body = pick( + await readJsonObject(opts.file, "data source"), + DATASOURCE_WRITABLE_FIELDS, + ); } if (opts.name !== undefined) body.displayName = opts.name; if (Object.keys(body).length === 0) { diff --git a/packages/cli/src/commands/regions.ts b/packages/cli/src/commands/regions.ts index 882cbd1..0fa305c 100644 --- a/packages/cli/src/commands/regions.ts +++ b/packages/cli/src/commands/regions.ts @@ -9,7 +9,7 @@ import { type GeoTargetArea, } from "@gmc-cli/api"; import { contextFrom, wantsJson } from "../context.js"; -import { clientFor, resolveAccount, line, readJsonObject, parsePageSize } from "./_shared.js"; +import { clientFor, resolveAccount, line, readJsonObject, parsePageSize, pick } from "./_shared.js"; interface RegionWriteOpts { displayName?: string; @@ -69,21 +69,12 @@ function parseGeotargetIds(raw: string): GeoTargetArea { } /** The writable Region fields — everything else (`name`, the `*Eligible` flags) is output-only. */ -const WRITABLE_FIELDS = ["displayName", "postalCodeArea", "geotargetArea", "radiusArea"] as const; - -/** - * Keep only the writable fields of a parsed `--file` body. A body saved from - * `regions get` carries output-only `name` / `*Eligible` flags, which the API rejects - * in a PATCH `updateMask` (and shouldn't be sent on a write) — allowlisting drops them, - * mirroring how `toProductInput` strips output-only product data. - */ -function pickWritable(obj: Record): RegionInput { - const out: Record = {}; - for (const key of WRITABLE_FIELDS) { - if (key in obj) out[key] = obj[key]; - } - return out as RegionInput; -} +const WRITABLE_FIELDS = [ + "displayName", + "postalCodeArea", + "geotargetArea", + "radiusArea", +] as const satisfies readonly (keyof RegionInput)[]; /** * Build a RegionInput from `--file` (JSON base) overlaid with the convenience flags. @@ -92,7 +83,7 @@ function pickWritable(obj: Record): RegionInput { */ async function buildRegionInput(opts: RegionWriteOpts, requireArea: boolean): Promise { const input: RegionInput = opts.file - ? pickWritable(await readJsonObject(opts.file, "region")) + ? pick(await readJsonObject(opts.file, "region"), WRITABLE_FIELDS) : {}; if (opts.regionCode && !opts.postalCodes) { throw new UsageError(