Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/28168-record-currency-code-before-boot.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@objectstack/cli': patch
---

fix(cli): `os migrate record-currency-codes` refuses a `--code` outside ISO 4217 before it probes the database, prompts or boots

Clause-②: no
Metadata-protocol: none

A code that is not an uppercase ISO 4217 code (`usd`, `EURO`) used to be refused only by the backfill, after the app had booted, and an `--apply` boot creates missing tables and columns before that refusal. The command now judges `--code` first, with the same predicate the backfill uses, and answers with the same `INVALID_FIELD` / 400 envelope (`code` and `httpStatus` under `--json`) and exit 1. Nothing is probed, prompted or booted for a bad code. The backfill in `@objectstack/objectql` keeps its own check unchanged.
22 changes: 22 additions & 0 deletions content/docs/data-modeling/currency.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,28 @@ as an explicit value. A `defaultValue` you declare on the code field yourself is
ordinary field default: the engine applies it before the check, so it satisfies the
rule, because you wrote it.

### Switching a field that already holds rows

An existing `dynamic` or `fixed` amount can be switched to `currencyMode: 'record'`,
but every row already stored then holds an amount beside a blank code. Such a row is
not refused for resting, but it shows as a plain number, falls into the `null` group of
a grouped aggregate, and sits outside every filter on the pair until it holds a code.

1. **The save answers 409 `destructive_change`.** Saving the switched object reports
the finding `currency_mode_to_record` on the amount field, and the message names the
command to run. Re-submit the save with `force` (`?force=true` on
`PUT /api/v1/meta/:type/:name`) to make the switch.
2. **Dry run.** `os migrate record-currency-codes --object opportunity --field amount --code USD`
counts the rows holding an amount and no code, and writes nothing.
3. **`--apply`.** The same command with `--apply` writes the code into those rows. It
is idempotent: re-run it until the report says zero.

The code is the one you give with `--code`, and it is never derived: not from the
`defaultCurrency` the field had under `fixed`, not from the organization's currency.
One run writes one code, so if the stored amounts are in more than one currency, split
those rows with your own writes first. See
[`os migrate record-currency-codes`](/docs/deployment/cli#os-migrate-record-currency-codes).

### Aggregating a record-mode amount

Amounts in different currencies do not add. A `sum`, `avg`, `min` or `max` over a
Expand Down
38 changes: 38 additions & 0 deletions content/docs/deployment/cli.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -1302,6 +1302,7 @@ where the data lives.
| `os migrate files-to-references` | Convert legacy file-field values to `sys_file` references, verify the ownership ledger, and record the deployment's migration flag |
| `os migrate value-shapes` | Scan stored reference and structured-JSON field values against the platform's value contract, and record the deployment's migration flag when clean |
| `os migrate summary-nulls` | Backfill roll-up `count` / `sum` columns still stored as `NULL` on parent rows created before the insert-time seed. Repairs values; no flag, nothing depends on it having run |
| `os migrate record-currency-codes` | Write the currency code you give into the rows of a record-mode currency field that hold an amount and no code, the rows stored before the field was switched to `currencyMode: 'record'`. The code is never derived. No flag; idempotent. See [below](#os-migrate-record-currency-codes) |
| `os migrate comment-reactions` | Carry the emoji reactions a 17.x deployment stored in the retired `sys_comment.reactions` column into `sys_comment_reaction` records, list every stored value that does not read, and record the verified run that lets `os migrate apply --allow-destructive` drop the column |
| `os migrate organization-ownership` | The v18 ownership ceremony (ADR-0131 D10): write a plan of what each table's rows will become, then `--apply` that reviewed plan. It upgrades a database 17.x or an 18.0 pre-release (`18.0.0-next.*`) created; a v18 server refuses to start on one until it has run. See [below](#os-migrate-organization-ownership) |
| `os migrate activity-changed-fields` | Stamp the two read-rule columns (`changed_fields`, `milestone_key`) on the `sys_activity` rows written before they existed. Until it runs, those rows are withheld from every reader but the system, so run it at once after upgrading. No flag; idempotent |
Expand Down Expand Up @@ -1504,6 +1505,43 @@ A deployment whose database was seeded fresh on this version has nothing to do
here: its parents were created with the value already in place, and the run
reports zero.

#### `os migrate record-currency-codes`

Switching an existing currency field to `currencyMode: 'record'` leaves every row
already stored holding an amount beside a blank code. Nothing refuses such a row for
resting, but until it holds a code it shows as a plain number, falls into the `null`
group of a grouped aggregate, and sits outside every filter on the amount and its
code. The save that makes the switch answers 409 `destructive_change` naming this
command ([Currency fields](/docs/data-modeling/currency#switching-a-field-that-already-holds-rows)
covers the switch). This command writes the code into those rows.

```bash
os migrate record-currency-codes --object opportunity --field amount --code USD # Dry run: counts the rows, writes none
os migrate record-currency-codes --object opportunity --field amount --code USD --apply # Write the code (prompts)
os migrate record-currency-codes --object opportunity --field amount --code USD --apply --yes --json # CI / scripts
```

**The code is yours, never derived.** `--object`, `--field` and `--code` are all
required. One run writes the one code you give into every row of the field whose
amount is set and whose code is blank. It never reads a code from the `defaultCurrency`
the field had under `fixed`, from the organization's `localization.currency`, or from
anything else. A row that holds a code is never touched, whatever the code, and a row
with no amount owes none. If the stored amounts are in more than one currency, split
them with your own writes first: nothing here can tell those rows apart.

**A bad `--code` is refused before anything is opened.** A code that is not an
uppercase ISO 4217 code (`usd`, `EURO`) is refused with `INVALID_FIELD` / 400 and
exit 1 before the database is probed, the `--apply` prompt is shown or the app is
booted. An object your deployment does not declare (`OBJECT_NOT_FOUND`), or a field
that is not a record-mode currency field (`INVALID_FIELD`), is refused after the boot
and before any row is read.

Each row is written through the engine like any other update, so the record-mode
write rule and the code field's `valueDomain` judge it, and it is audited. Idempotent:
every write turns a blank code into a code, so a second run finds nothing to write.
Re-running until the report says zero *is* the verification, which is why this
command records no flag.

#### `os migrate comment-reactions`

v18 retired `sys_comment.reactions`, the JSON column that held every emoji
Expand Down
106 changes: 90 additions & 16 deletions packages/cli/src/commands/migrate/record-currency-codes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
* `record-currency-backfill.test.ts`. What is pinned here is what a unit test
* of the backfill cannot see: the command is dry-run-by-default, its three
* naming flags are required, they reach `backfillRecordCurrencyCodes` as given,
* a refusal reaches the `--json` envelope with its code, and the human face's
* dry-run line is followed by the journal-mode note on a SQLite target only.
* a refusal reaches the `--json` envelope with its code, a code outside ISO
* 4217 is refused before the occupancy probe, the `--apply` prompt and the boot
* (card #28168), and the human face's dry-run line is followed by the
* journal-mode note on a SQLite target only.
* The seams that would boot a database or walk a real engine are replaced; the
* command's own parse and control flow run for real, and the report's formatter
* and the refusal are the real ones.
Expand All @@ -31,6 +33,19 @@ vi.mock('../../utils/migrate-occupancy-gate.js', () => ({
probeMigrationTarget: vi.fn(),
}));
vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: vi.fn(async () => []) }));
// The `--apply` confirmation: a prompt opened at all is recorded, and answered
// no, so a case that should never reach it can say so instead of hanging.
const prompt = vi.hoisted(() => ({ opened: 0 }));
vi.mock('node:readline', async (importOriginal) => {
const real = await importOriginal<typeof import('node:readline')>();
return {
...real,
createInterface: (() => {
prompt.opened++;
return { question: (_q: string, answer: (a: string) => void) => answer('n'), close: () => {} };
}) as unknown as typeof real.createInterface,
};
});
vi.mock('@objectstack/objectql', async (importOriginal) => {
const real = await importOriginal<typeof import('@objectstack/objectql')>();
return {
Expand Down Expand Up @@ -134,29 +149,88 @@ describe('os migrate record-currency-codes', () => {
expect(payload).toMatchObject({ code: 'OBJECT_NOT_FOUND' });
}, RUN_TIMEOUT);

it('a refused code reaches the --json envelope with its code and httpStatus, and exits 1', async () => {
it('a refusal from the backfill reaches the --json envelope with its code and httpStatus, and exits 1', async () => {
const real = await vi.importActual<typeof import('@objectstack/objectql')>('@objectstack/objectql');
const recordEngine = {
...engine,
getObject: () => ({
fields: { amount: { type: 'currency', currencyConfig: { currencyMode: 'record', currencyField: 'currency_code' } } },
}),
};
const refusal = await real.backfillRecordCurrencyCodes(recordEngine, { info() {}, warn() {} }, {
object: 'opportunity', field: 'amount', code: 'usd',
}).then(() => { throw new Error('expected the code to be refused'); }, (e: unknown) => e as Error);
// A member code on a field that is not record-mode: the refusal only the
// booted engine can make, since it reads the object's declaration.
const plainEngine = { ...engine, getObject: () => ({ fields: { amount: { type: 'currency' } } }) };
const refusal = await real.backfillRecordCurrencyCodes(plainEngine, { info() {}, warn() {} }, {
object: 'opportunity', field: 'amount', code: 'USD',
}).then(() => { throw new Error('expected the field to be refused'); }, (e: unknown) => e as Error);
vi.mocked(backfillRecordCurrencyCodes).mockRejectedValue(refusal);

const err = await MigrateRecordCurrencyCodes.run(
['--json', '--object', 'opportunity', '--field', 'amount', '--code', 'usd'],
{ root: CLI_ROOT },
).catch((e: unknown) => e);
const err = await MigrateRecordCurrencyCodes.run(['--json', ...NAMING], { root: CLI_ROOT }).catch((e: unknown) => e);
expect(isExitSignal(err)).toBe(true);
expect((err as { oclif?: { exit?: number } }).oclif?.exit).toBe(1);
const payload = JSON.parse(stdout.mock.calls.map((c: unknown[]) => String(c[0])).join(''));
expect(payload).toMatchObject({ code: 'INVALID_FIELD', httpStatus: 400, error: refusal.message });
}, RUN_TIMEOUT);

describe('a code outside ISO 4217 is refused before the probe, the prompt and the boot', () => {
/** The engine's own refusal of the same code, for the envelope and wording it is held to. */
const engineRefusal = async (code: string): Promise<Error & { code?: string; httpStatus?: number }> => {
const real = await vi.importActual<typeof import('@objectstack/objectql')>('@objectstack/objectql');
const recordEngine = {
...engine,
getObject: () => ({
fields: { amount: { type: 'currency', currencyConfig: { currencyMode: 'record', currencyField: 'currency_code' } } },
}),
};
return real.backfillRecordCurrencyCodes(recordEngine, { info() {}, warn() {} }, {
object: 'opportunity', field: 'amount', code,
}).then(() => { throw new Error('expected the code to be refused'); }, (e: unknown) => e as Error);
};
/** Up to the clause naming the field, which only the booted engine can name. */
const leadingClause = (message: string): string => message.slice(0, message.indexOf(', so '));

beforeEach(() => {
vi.mocked(bootSchemaStack).mockClear();
vi.mocked(probeMigrationTarget).mockClear();
prompt.opened = 0;
});

it('--json --apply: the engine\'s INVALID_FIELD / 400 envelope, never confirmation_required, and nothing opened', async () => {
const err = await MigrateRecordCurrencyCodes.run(
['--json', '--apply', '--object', 'opportunity', '--field', 'amount', '--code', 'usd'],
{ root: CLI_ROOT },
).catch((e: unknown) => e);
expect(isExitSignal(err)).toBe(true);
expect((err as { oclif?: { exit?: number } }).oclif?.exit).toBe(1);
const payload = JSON.parse(stdout.mock.calls.map((c: unknown[]) => String(c[0])).join(''));
const refusal = await engineRefusal('usd');
expect(payload).toMatchObject({ code: refusal.code, httpStatus: refusal.httpStatus });
expect(payload).toMatchObject({ code: 'INVALID_FIELD', httpStatus: 400 });
expect(leadingClause(payload.error)).toBe(leadingClause(refusal.message));
expect(vi.mocked(bootSchemaStack)).not.toHaveBeenCalled();
expect(vi.mocked(probeMigrationTarget)).not.toHaveBeenCalled();
expect(vi.mocked(backfillRecordCurrencyCodes)).not.toHaveBeenCalled();
}, RUN_TIMEOUT);

it('human face, --apply on a terminal: the refusal is printed and no prompt opens', async () => {
const wasTTY = process.stdin.isTTY;
Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true });
try {
const err = await MigrateRecordCurrencyCodes.run(
['--apply', '--object', 'opportunity', '--field', 'amount', '--code', 'EURO'],
{ root: CLI_ROOT },
).catch((e: unknown) => e);
expect((err as { oclif?: { exit?: number } }).oclif?.exit).toBe(1);
} finally {
Object.defineProperty(process.stdin, 'isTTY', { value: wasTTY, configurable: true });
}
expect(printedLines().join('\n')).toContain(leadingClause((await engineRefusal('EURO')).message));
expect(prompt.opened).toBe(0);
expect(vi.mocked(bootSchemaStack)).not.toHaveBeenCalled();
expect(vi.mocked(probeMigrationTarget)).not.toHaveBeenCalled();
}, RUN_TIMEOUT);

it('a member code still reaches the boot (the control)', async () => {
await MigrateRecordCurrencyCodes.run(['--json', ...NAMING], { root: CLI_ROOT });
expect(vi.mocked(bootSchemaStack)).toHaveBeenCalledTimes(1);
expect(vi.mocked(backfillRecordCurrencyCodes)).toHaveBeenCalledTimes(1);
}, RUN_TIMEOUT);
});

it('on a SQLite file the dry-run line claims no row or table and names the journal-mode switch', async () => {
boot(SQLITE_CONFIG);
await MigrateRecordCurrencyCodes.run(NAMING, { root: CLI_ROOT });
Expand Down
49 changes: 46 additions & 3 deletions packages/cli/src/commands/migrate/record-currency-codes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { Command, Flags } from '@oclif/core';
import chalk from 'chalk';
import { createInterface } from 'node:readline';
import { isValueDomainMember } from '@objectstack/spec/shared';
import {
printHeader,
printSuccess,
Expand All @@ -24,6 +25,33 @@ import { refuseUndeclaredObjects } from '../../utils/migrate-object-scope.js';
// Type-only, so the heavy engine package is still loaded lazily below.
import type { RecordCurrencyBackfillEngine } from '@objectstack/objectql';

/**
* The `--code` refusal, judged before the occupancy probe, the `--apply`
* prompt and the boot: a code the code field's `valueDomain` would refuse
* fails the same way on every database, so nothing about the database needs
* opening to say so, and an `--apply` boot may create tables and columns.
*
* The predicate is the one `backfillRecordCurrencyCodes` applies, and the
* error is its envelope (`INVALID_FIELD` / 400, `httpStatus` for the `--json`
* face) and its wording, so the answer is the one the engine gave when this
* was judged after the boot. The code field's name is in the object's
* declaration, which is not loaded yet, so the message names the amount field
* the code is owed by. The engine keeps its own check for every other caller.
*/
function refuseNonMemberCurrencyCode(object: string, field: string, code: string): void {
if (isValueDomainMember('iso_4217_currency', code)) return;
const err = new Error(
`[record-currency-backfill] '${code}' is not an ISO 4217 currency code, so the currency code field of ` +
`'${object}.${field}' would refuse it: give the uppercase alphabetic code of the currency these amounts ` +
'are in (for example USD). Refused before the database was opened or any row was read.',
) as Error & { code: string; status: number; httpStatus: number; field: string };
err.code = 'INVALID_FIELD';
err.status = 400;
err.httpStatus = 400;
err.field = field;
throw err;
}

async function confirm(question: string): Promise<boolean> {
if (!process.stdin.isTTY) return false; // non-interactive → require --yes
const rl = createInterface({ input: process.stdin, output: process.stdout });
Expand Down Expand Up @@ -78,7 +106,7 @@ export default class MigrateRecordCurrencyCodes extends Command {
code: Flags.string({
description:
'The ISO 4217 code to write, uppercase (e.g. USD). Written into every row with an amount and a blank code; ' +
'a row that holds a code is never touched. A code that is not ISO 4217 is refused before any row is read',
'a row that holds a code is never touched. A code that is not ISO 4217 is refused before the database is opened',
required: true,
}),
apply: Flags.boolean({
Expand All @@ -103,6 +131,20 @@ export default class MigrateRecordCurrencyCodes extends Command {

if (!flags.json) printHeader('Migrate · record-currency-codes');

// `--code` first: before the occupancy probe, the `--apply` prompt and the boot.
try {
refuseNonMemberCurrencyCode(flags.object, flags.field, flags.code);
} catch (error: any) {
if (flags.json) {
await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true });
this.exit(1);
return;
}
printError(error.message);
this.exit(1);
return;
}

// Occupancy gate, as `summary-nulls`: probed before boot and before the prompt.
const occupancy = await probeMigrationTarget(flags['database-url']);
if (occupancy.status === 'busy' && apply && !flags.force) {
Expand Down Expand Up @@ -244,8 +286,9 @@ export default class MigrateRecordCurrencyCodes extends Command {
} catch (error: any) {
if (isExitSignal(error)) throw error;
if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); }
// `INVALID_FIELD` is the backfill refusing the field or the code before
// its walk ("Refused before the walk read or wrote any row."). The boot
// `INVALID_FIELD` is the backfill refusing the field before its walk
// ("Refused before the walk read or wrote any row."; the code was judged
// before the boot, so it is refused there only if that judge drifts). The boot
// above ran first, so on a SQLite file the note naming a journal-mode
// switch follows, as on `summary-nulls`.
const journalNote = error?.code === 'INVALID_FIELD'
Expand Down
Loading