diff --git a/.changeset/select-all-override.md b/.changeset/select-all-override.md new file mode 100644 index 00000000..909cbd6e --- /dev/null +++ b/.changeset/select-all-override.md @@ -0,0 +1,5 @@ +--- +"@proofkit/fmodata": minor +--- + +Add select("all") to override defaultSelect on a per-query basis diff --git a/.coderabbit.yaml b/.coderabbit.yaml index c0036945..225357c6 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -4,11 +4,11 @@ reviews: # Enable automated review for pull requests auto_review: - enabled: false + enabled: true base_branches: - ".*" # Matches all branches using regex review_status: false poem: false - high_level_summary: false + high_level_summary: true path_filters: - - "!apps/demo/**" # exclude the demo app from reivews + - "!apps/demo/**" # exclude the demo app from reviews diff --git a/packages/fmodata/src/cli/commands/query.ts b/packages/fmodata/src/cli/commands/query.ts index 7936dd7b..413f3daa 100644 --- a/packages/fmodata/src/cli/commands/query.ts +++ b/packages/fmodata/src/cli/commands/query.ts @@ -111,9 +111,17 @@ export function makeRecordsCommand(): Command { .requiredOption("--table ", "Table name") .requiredOption("--data ", "Update data as JSON object") .option("--where ", "OData filter expression") + .option("--confirm", "Execute without --where (affects all records)") .action(async (opts, cmd) => { const globalOpts = cmd.parent?.parent?.opts() as ConnectionOptions & { pretty: boolean }; try { + if (!(opts.where || opts.confirm)) { + printResult( + { dryRun: true, action: "update", table: opts.table, affectsAllRows: true, hint: "Add --where to filter or --confirm to update all records" }, + { pretty: globalOpts.pretty ?? false }, + ); + return; + } const { db } = buildConnection(globalOpts); let data: Record; try { @@ -145,9 +153,17 @@ export function makeRecordsCommand(): Command { .description("Delete records from a table") .requiredOption("--table ", "Table name") .option("--where ", "OData filter expression") + .option("--confirm", "Execute without --where (affects all records)") .action(async (opts, cmd) => { const globalOpts = cmd.parent?.parent?.opts() as ConnectionOptions & { pretty: boolean }; try { + if (!(opts.where || opts.confirm)) { + printResult( + { dryRun: true, action: "delete", table: opts.table, affectsAllRows: true, hint: "Add --where to filter or --confirm to delete all records" }, + { pretty: globalOpts.pretty ?? false }, + ); + return; + } const { db } = buildConnection(globalOpts); const qs = buildQueryString({ where: opts.where as string | undefined }); const table = encodeURIComponent(opts.table as string); diff --git a/packages/fmodata/src/cli/commands/webhook.ts b/packages/fmodata/src/cli/commands/webhook.ts index a08e13e4..8bf9d31f 100644 --- a/packages/fmodata/src/cli/commands/webhook.ts +++ b/packages/fmodata/src/cli/commands/webhook.ts @@ -1,4 +1,5 @@ import { Command } from "commander"; +import { FMTable } from "../../orm/table"; import type { ConnectionOptions } from "../utils/connection"; import { buildConnection } from "../utils/connection"; import { handleCliError } from "../utils/errors"; @@ -78,7 +79,7 @@ export function makeWebhookCommand(): Command { // Build a minimal FMTable-like proxy for the tableName // webhook.add() only reads the name via Symbol, so this is safe at runtime const tableProxy = { - [Symbol.for("fmodata:FMTableName")]: opts.table, + [FMTable.Symbol.Name]: opts.table, } as unknown as import("../../orm/table").FMTable, string>; const webhookPayload: import("../../client/webhook-builder").Webhook = { diff --git a/packages/fmodata/src/cli/utils/output.ts b/packages/fmodata/src/cli/utils/output.ts index 9ae0441b..07095dd9 100644 --- a/packages/fmodata/src/cli/utils/output.ts +++ b/packages/fmodata/src/cli/utils/output.ts @@ -47,9 +47,3 @@ function printTable(data: unknown): void { // Fallback — just print as JSON console.log(JSON.stringify(data, null, 2)); } - -export function printError(err: unknown): void { - const message = err instanceof Error ? err.message : String(err); - process.stderr.write(`Error: ${message}\n`); - process.exit(1); -} diff --git a/packages/fmodata/src/client/query/query-builder.ts b/packages/fmodata/src/client/query/query-builder.ts index ec9751fb..48baef6b 100644 --- a/packages/fmodata/src/client/query/query-builder.ts +++ b/packages/fmodata/src/client/query/query-builder.ts @@ -166,7 +166,7 @@ export class QueryBuilder< newBuilder.singleMode = (changes.singleMode ?? this.singleMode) as any; // biome-ignore lint/suspicious/noExplicitAny: Type assertion for generic type parameter newBuilder.isCountMode = (changes.isCountMode ?? this.isCountMode) as any; - newBuilder.fieldMapping = changes.fieldMapping ?? this.fieldMapping; + newBuilder.fieldMapping = "fieldMapping" in changes ? changes.fieldMapping : this.fieldMapping; newBuilder.systemColumns = changes.systemColumns !== undefined ? changes.systemColumns : this.systemColumns; // Copy navigation metadata newBuilder.navigation = this.navigation; @@ -175,7 +175,7 @@ export class QueryBuilder< } /** - * Select fields using column references. + * Select fields using column references, or pass "all" to clear any defaultSelect and fetch all fields. * Allows renaming fields by using different keys in the object. * Container fields cannot be selected and will cause a type error. * @@ -192,10 +192,25 @@ export class QueryBuilder< * { ROWID: true, ROWMODID: true } * ) * - * @param fields - Object mapping output keys to column references (container fields excluded) + * @example + * // Override defaultSelect to fetch all fields + * db.from(users).list().select("all") + * + * @param fields - Object mapping output keys to column references (container fields excluded), or "all" to select all fields * @param systemColumns - Optional object to request system columns (ROWID, ROWMODID) * @returns QueryBuilder with updated selected fields */ + select( + fields: "all", + ): QueryBuilder< + Occ, + keyof InferSchemaOutputFromFMTable, + SingleMode, + IsCount, + Expands, + DatabaseIncludeSpecialColumns, + undefined + >; select< // biome-ignore lint/suspicious/noExplicitAny: Generic constraint accepting any Column configuration TSelect extends Record, false>>, @@ -204,7 +219,20 @@ export class QueryBuilder< >( fields: TSelect, systemColumns?: TSystemCols, - ): QueryBuilder { + ): QueryBuilder; + // biome-ignore lint/suspicious/noExplicitAny: Implementation signature hidden from callers + select(fields: any, systemColumns?: any): any { + if (fields === "all") { + return this.cloneWithChanges({ + queryOptions: { + select: undefined, + }, + fieldMapping: undefined, + // biome-ignore lint/suspicious/noExplicitAny: Type assertion for generic type parameter + systemColumns: undefined as any, + }); + } + const tableName = getTableName(this.occurrence); const { selectedFields, fieldMapping } = processSelectWithRenames(fields, tableName, this.logger); diff --git a/packages/fmodata/src/client/record-builder.ts b/packages/fmodata/src/client/record-builder.ts index 5a2e7fb0..b61049cd 100644 --- a/packages/fmodata/src/client/record-builder.ts +++ b/packages/fmodata/src/client/record-builder.ts @@ -207,8 +207,8 @@ export class RecordBuilder< // biome-ignore lint/suspicious/noExplicitAny: Mutation of readonly properties for builder pattern const mutableBuilder = newBuilder as any; - mutableBuilder.selectedFields = changes.selectedFields ?? this.selectedFields; - mutableBuilder.fieldMapping = changes.fieldMapping ?? this.fieldMapping; + mutableBuilder.selectedFields = "selectedFields" in changes ? changes.selectedFields : this.selectedFields; + mutableBuilder.fieldMapping = "fieldMapping" in changes ? changes.fieldMapping : this.fieldMapping; mutableBuilder.systemColumns = changes.systemColumns !== undefined ? changes.systemColumns : this.systemColumns; mutableBuilder.expandConfigs = [...this.expandConfigs]; // Preserve navigation context @@ -266,7 +266,7 @@ export class RecordBuilder< } /** - * Select fields using column references. + * Select fields using column references, or pass "all" to clear any defaultSelect and fetch all fields. * Allows renaming fields by using different keys in the object. * Container fields cannot be selected and will cause a type error. * @@ -283,10 +283,25 @@ export class RecordBuilder< * { ROWID: true, ROWMODID: true } * ) * - * @param fields - Object mapping output keys to column references (container fields excluded) + * @example + * // Override defaultSelect to fetch all fields + * db.from(contacts).get("uuid").select("all") + * + * @param fields - Object mapping output keys to column references (container fields excluded), or "all" to select all fields * @param systemColumns - Optional object to request system columns (ROWID, ROWMODID) * @returns RecordBuilder with updated selected fields */ + select( + fields: "all", + ): RecordBuilder< + Occ, + false, + FieldColumn, + keyof InferSchemaOutputFromFMTable>, + Expands, + DatabaseIncludeSpecialColumns, + undefined + >; select< // biome-ignore lint/suspicious/noExplicitAny: Generic constraint accepting any Column configuration TSelect extends Record, false>>, @@ -294,7 +309,18 @@ export class RecordBuilder< >( fields: TSelect, systemColumns?: TSystemCols, - ): RecordBuilder { + ): RecordBuilder; + // biome-ignore lint/suspicious/noExplicitAny: Implementation signature hidden from callers + select(fields: any, systemColumns?: any): any { + if (fields === "all") { + return this.cloneWithChanges({ + selectedFields: undefined, + fieldMapping: undefined, + // biome-ignore lint/suspicious/noExplicitAny: Type assertion for generic type parameter + systemColumns: undefined as any, + }); + } + const tableName = getTableName(this.table); const { selectedFields, fieldMapping } = processSelectWithRenames(fields, tableName, this.logger); diff --git a/packages/fmodata/tests/cli/unit/output.test.ts b/packages/fmodata/tests/cli/unit/output.test.ts index 01d245e4..04c1d240 100644 --- a/packages/fmodata/tests/cli/unit/output.test.ts +++ b/packages/fmodata/tests/cli/unit/output.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { printError, printResult } from "../../../src/cli/utils/output"; +import { printResult } from "../../../src/cli/utils/output"; describe("printResult", () => { let stdoutSpy: ReturnType; @@ -50,19 +50,3 @@ describe("printResult", () => { expect(output).toContain("c"); }); }); - -describe("printError", () => { - it("writes to stderr and exits", () => { - const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); - const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => { - throw new Error("process.exit called"); - }); - - expect(() => printError(new Error("something went wrong"))).toThrow("process.exit called"); - expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("something went wrong")); - expect(exitSpy).toHaveBeenCalledWith(1); - - stderrSpy.mockRestore(); - exitSpy.mockRestore(); - }); -}); diff --git a/packages/fmodata/tests/navigate.test.ts b/packages/fmodata/tests/navigate.test.ts index 50f69eaf..4e9447be 100644 --- a/packages/fmodata/tests/navigate.test.ts +++ b/packages/fmodata/tests/navigate.test.ts @@ -117,12 +117,13 @@ describe("navigate", () => { // contacts -> invoices navigation const invoiceQuery = db.from(contacts).navigate(invoices).list(); expectTypeOf(invoiceQuery.select).parameter(0).not.toEqualTypeOf(); + // valid fields from navigated table should work invoiceQuery.select({ invoiceNumber: invoices.invoiceNumber, total: invoices.total, - // @ts-expect-error - not valid since we navigated to invoices, not contacts - other: contacts.name, }); + // @ts-expect-error - contacts.name not valid for invoices table + invoiceQuery.select({ other: contacts.name }); // invoices -> lineItems navigation const lineItemsQuery = db.from(invoices).navigate(lineItems).list(); diff --git a/packages/fmodata/tests/record-builder-select-expand.test.ts b/packages/fmodata/tests/record-builder-select-expand.test.ts index 8469e75d..69a70c8e 100644 --- a/packages/fmodata/tests/record-builder-select-expand.test.ts +++ b/packages/fmodata/tests/record-builder-select-expand.test.ts @@ -177,6 +177,28 @@ describe("RecordBuilder Select/Expand", () => { expect(queryString).not.toContain("PrimaryKey"); expect(queryString).not.toContain("hobby"); }); + + it('should clear defaultSelect when select("all") is called on get()', () => { + const queryString = db.from(contactsWithSchemaSelect).get("test-uuid").select("all").getQueryString(); + + // select("all") should remove $select entirely + expect(queryString).toBe("/contacts('test-uuid')"); + expect(queryString).not.toContain("$select="); + }); + + it('should clear defaultSelect when select("all") is called on list()', () => { + const queryString = db.from(contactsWithSchemaSelect).list().select("all").getQueryString(); + + // select("all") should remove $select entirely + expect(queryString).not.toContain("$select="); + }); + + it('should clear custom defaultSelect when select("all") is called', () => { + const queryString = db.from(contactsWithArraySelect).get("test-uuid").select("all").getQueryString(); + + expect(queryString).toBe("/contacts('test-uuid')"); + expect(queryString).not.toContain("$select="); + }); }); describe("defaultSelect within expand()", () => { diff --git a/packages/fmodata/tests/typescript.test.ts b/packages/fmodata/tests/typescript.test.ts index 1d02a3a4..4df4392d 100644 --- a/packages/fmodata/tests/typescript.test.ts +++ b/packages/fmodata/tests/typescript.test.ts @@ -112,6 +112,12 @@ describe("fmodata", () => { expect(query2).toBeDefined(); expect(query3).toBeDefined(); + // select("all") should be accepted and return full schema type + const allQuery = entitySet.list().select("all"); + expectTypeOf(allQuery.execute).returns.resolves.toMatchTypeOf<{ + data: { id: string; name: string | null; email: string | null; age: number | null }[] | undefined; + }>(); + // These should be TypeScript errors - fields not in schema const _typeChecks = () => { // @ts-expect-error - should pass an object @@ -120,9 +126,9 @@ describe("fmodata", () => { entitySet.list().select(""); // @ts-expect-error - should pass an object with column references entitySet.list().select({ invalidField: true }); + // @ts-expect-error - column must be from the correct table entitySet.list().select({ age: users.age, - // @ts-expect-error - column must be from the correct table name: contacts.name, }); }; @@ -188,9 +194,9 @@ describe("fmodata", () => { entitySet.list().select(""); // @ts-expect-error - should pass an object with column references entitySet.list().select({ invalidField: true }); + // @ts-expect-error - column must be from the correct table entitySet.list().select({ anyName: products.productName, - // @ts-expect-error - column must be from the correct table name: contacts.name, }); }; diff --git a/packages/fmodata/tsdown.config.ts b/packages/fmodata/tsdown.config.ts index d8afda25..b8f5f549 100644 --- a/packages/fmodata/tsdown.config.ts +++ b/packages/fmodata/tsdown.config.ts @@ -6,6 +6,7 @@ export default defineConfig({ target: "esnext", outDir: "dist/cli", clean: false, + splitting: false, banner: { js: "#!/usr/bin/env node", },