From 5761bac09ee330c71c72f65bd24277373a4b4e38 Mon Sep 17 00:00:00 2001 From: Michiel Degezelle Date: Tue, 24 Mar 2026 13:41:47 +0100 Subject: [PATCH 1/6] Add markdown files --- .cursor/rules/silverfin-cli-context.mdc | 13 ++++ AGENTS.md | 49 +++++++++++++++ docs/ARCHITECTURE.md | 84 +++++++++++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 .cursor/rules/silverfin-cli-context.mdc create mode 100644 AGENTS.md create mode 100644 docs/ARCHITECTURE.md diff --git a/.cursor/rules/silverfin-cli-context.mdc b/.cursor/rules/silverfin-cli-context.mdc new file mode 100644 index 0000000..fb1a6d1 --- /dev/null +++ b/.cursor/rules/silverfin-cli-context.mdc @@ -0,0 +1,13 @@ +--- +description: +alwaysApply: true +--- + +# silverfin-cli — agent context + +Before non-trivial changes in this package: + +1. Follow **AGENTS.md** (repo root) — commands (`npm test`, `npm run lint`), entry points (`bin/cli.js`, `index.js`), public API stability, auth/error pointers. +2. Use **docs/ARCHITECTURE.md** for the module map and how CLI vs `index.js` vs `lib/` connect. + +Coding style: match existing code; run **ESLint** and **Prettier** settings from the repo (`npm run lint`, `package.json` `prettier` field). Do not duplicate long style lists here. diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..2a451e3 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,49 @@ +# Agent / contributor guide (silverfin-cli) + +Short orientation for working in this repository. End-user setup, credentials, and template **repository** layout are documented in [README.md](README.md). + +## Commands + +- **Full test suite:** `npm test` +- **Lint:** `npm run lint` +- **Focused Jest run:** `npx jest path/to/test.js` (or a directory under `tests/`) + +Before finishing a change that touches library or CLI behavior, run **lint and the full test suite**. + +## Entry points + +| Layer | Path | Role | +|--------|------|------| +| CLI executable | [bin/cli.js](bin/cli.js) | [Commander](https://github.com/tj/commander.js) program: defines subcommands, parses options, calls `index` exports and helpers (`liquidTestRunner`, `liquidTestGenerator`, etc.). | +| Programmatic API | [index.js](index.js) | `require('silverfin-cli')` surface: template fetch/publish/create helpers and related utilities. See **Public API** below. | +| Core libraries | [lib/](lib/) | `api/` (HTTP + auth), `templates/` (reconciliation, shared parts, export files, account templates), `cli/` (cwd checks, completions, updater, stats), `utils/` (fs, API helpers, errors, Liquid test helpers). | + +## Public API (`index.js`) + +The package `main` is [index.js](index.js). Downstream tools (e.g. the [Silverfin VS Code extension](https://github.com/silverfin/silverfin-vscode)) may depend on **named exports** on `module.exports`. Treat additions as safe; **removals or signature changes** are semver-sensitive—prefer deprecation when possible. + +## Invariants / do not break + +- **Global vs local install:** README recommends a **global** install; some flows (e.g. `update`) assume global usage. Local `node_modules` installs can behave differently. +- **Backward compatibility:** Keep the `index.js` export object stable for extension and automation consumers unless you are intentionally shipping a major version. +- **User template repos** follow the folder layout in README (`reconciliation_texts/`, `shared_parts/`, etc.). That is **not** the same as this package’s `lib/` tree—do not confuse the two. + +## Auth and networking + +- **OAuth / tokens / firm storage:** [lib/api/silverfinAuthorizer.js](lib/api/silverfinAuthorizer.js) (browser flow, refresh, partner keys). +- **HTTP client and API calls:** [lib/api/sfApi.js](lib/api/sfApi.js) (uses [lib/api/axiosFactory.js](lib/api/axiosFactory.js) for authenticated instances). +- **Stored credentials / host:** [lib/api/firmCredentials.js](lib/api/firmCredentials.js). + +Environment variables and scopes: see [README.md](README.md) (e.g. `SF_API_CLIENT_ID`, `SF_API_SECRET`; host via `silverfin config --set-host`). + +## Errors and logging + +- **Shared helpers:** [lib/utils/errorUtils.js](lib/utils/errorUtils.js) — missing IDs/config, batch reconciliation summaries, `uncaughtErrors` / `errorHandler`. +- **CLI process handlers:** [lib/cli/utils.js](lib/cli/utils.js) (`handleUncaughtErrors`) wires `uncaughtRejection` / `uncaughtException` to `errorUtils.uncaughtErrors`. +- **User-facing messages:** [consola](https://github.com/unjs/consola) and [chalk](https://github.com/chalk/chalk) (e.g. suggested fix-it commands). Prefer extending `errorUtils` for consistent messaging rather than ad hoc `console.log` in many places. + +## When in doubt + +1. Run `npm run lint` and `npm test`. +2. Mirror patterns in the nearest existing command or module ([bin/cli.js](bin/cli.js) + matching code in [index.js](index.js) / [lib/](lib/)). +3. For structure and data flow, see [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md). diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md new file mode 100644 index 0000000..40358e1 --- /dev/null +++ b/docs/ARCHITECTURE.md @@ -0,0 +1,84 @@ +# silverfin-cli architecture + +High-level map of the package. For commands to run and contributor rules, see [AGENTS.md](../AGENTS.md). For how **users** lay out a Liquid template repository on disk, see [README.md](../README.md) — that layout is **not** the same as this repo’s `lib/` tree. + +## Flow overview + +```mermaid +flowchart LR + subgraph cli_layer [CLI] + Bin[bin/cli.js] + end + subgraph prog [Programmatic API] + Index[index.js] + end + subgraph lib_pkg [lib] + API[api] + TPL[templates] + CLIUTIL[cli] + UTILS[utils] + LIQ[liquidTestRunner / liquidTestGenerator] + EXP[exportFileInstanceGenerator] + end + Bin --> Index + Bin --> LIQ + Bin --> EXP + Index --> API + Index --> TPL + Index --> UTILS + API --> UTILS + TPL --> UTILS + CLIUTIL --> UTILS + LIQ --> UTILS +``` + +- **bin/cli.js** is the Commander entry: it calls **`index.js`** for template workflows and pulls in **Liquid test** and **export-instance** modules directly where the CLI needs them. +- **index.js** orchestrates template types (reconciliations, shared parts, export files, account templates) using **`lib/api`** and **`lib/templates`**, plus filesystem helpers. + +## `lib/` responsibilities + +### `lib/api/` + +| Module | Responsibility | +|--------|------------------| +| [sfApi.js](../lib/api/sfApi.js) | Silverfin REST calls (reconciliations, shared parts, export files, account templates, etc.). Builds axios instances via `AxiosFactory`, handles responses with `apiUtils`. | +| [silverfinAuthorizer.js](../lib/api/silverfinAuthorizer.js) | OAuth-style firm authorization (browser + code), token refresh, partner token refresh. | +| [axiosFactory.js](../lib/api/axiosFactory.js) | Configured axios instances for firm/partner contexts (base URL, auth headers, optional basic auth). | +| [firmCredentials.js](../lib/api/firmCredentials.js) | Persisted tokens, host, firm name; read/write credential store used by authorizer and API layer. | + +### `lib/templates/` + +Object-oriented wrappers around on-disk template folders and sync with the API: + +- **reconciliationText.js** — reconciliation texts under `reconciliation_texts/` +- **sharedPart.js** — `shared_parts/` +- **exportFile.js** — `export_files/` +- **accountTemplate.js** — `account_templates/` + +They work with [fsUtils.js](../lib/utils/fsUtils.js) and [templateUtils.js](../lib/utils/templateUtils.js) for paths, configs, and IDs. + +### `lib/cli/` + +Command-line–specific behavior (not re-exported as the main programmatic API): + +- **utils.js** — option checks, default firm id, uncaught error wiring to `errorUtils` +- **cwdValidator.js** — working-directory validation for repo conventions +- **autoCompletions.js** — shell completions +- **cliUpdater.js** — CLI self-update +- **stats.js**, **changelogReader.js**, **spinner.js**, **devMode.js** — UX and maintenance helpers + +### `lib/utils/` + +Shared cross-cutting helpers: **errorUtils** (messages, exits, batch summaries), **fsUtils**, **apiUtils** (env checks, response handlers), **urlHandler**, **wslHandler**, **runTestUtils**, **liquidTestUtils**, etc. + +### Root-level `lib/*.js` (not under a subfolder) + +| Module | Responsibility | +|--------|------------------| +| [liquidTestRunner.js](../lib/liquidTestRunner.js) | Run Liquid tests from the CLI against the API. | +| [liquidTestGenerator.js](../lib/liquidTestGenerator.js) | Generate test YAML from company data. | +| [exportFileInstanceGenerator.js](../lib/exportFileInstanceGenerator.js) | Export file instance generation workflow used by CLI commands. | + +## Tests + +Jest tests live under [tests/](../tests/) and generally mirror `lib/`. See [AGENTS.md](../AGENTS.md) for commands; API tests often use `axios-mock-adapter`. From 4f523123c312bcaf726cbfb319ac2b1f8d6b3922 Mon Sep 17 00:00:00 2001 From: Michiel Degezelle Date: Tue, 24 Mar 2026 14:26:37 +0100 Subject: [PATCH 2/6] Create function to summarize errors + implement error summary for update-all reconciliations --- index.js | 51 +++++++++++++++++++++++++++++++++++++---- lib/utils/errorUtils.js | 34 +++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index f53bd70..11326e8 100644 --- a/index.js +++ b/index.js @@ -118,12 +118,24 @@ async function fetchExistingReconciliations(type, envId) { } } -async function publishReconciliationByHandle(type, envId, handle, message = "Updated with the Silverfin CLI") { +async function publishReconciliationByHandle( + type, + envId, + handle, + message = "Updated with the Silverfin CLI", + deferredErrors = null +) { + const defer = Array.isArray(deferredErrors); + try { const configPresent = fsUtils.configExists("reconciliationText", handle); if (!configPresent) { - errorUtils.missingReconciliationId(handle); + if (defer) { + deferredErrors.push({ kind: "missing_id", handle }); + } else { + errorUtils.missingReconciliationId(handle); + } return false; } @@ -131,7 +143,11 @@ async function publishReconciliationByHandle(type, envId, handle, message = "Upd const templateId = fsUtils.getTemplateId(type, envId, templateConfig); if (!templateId) { - errorUtils.missingReconciliationId(handle); + if (defer) { + deferredErrors.push({ kind: "missing_id", handle }); + } else { + errorUtils.missingReconciliationId(handle); + } return false; } @@ -154,10 +170,30 @@ async function publishReconciliationByHandle(type, envId, handle, message = "Upd consola.success(`Reconciliation updated: ${response.data.handle}`); return true; } else { - consola.error(`Reconciliation update failed: ${handle}`); + if (defer) { + deferredErrors.push({ kind: "update_failed", handle }); + } else { + consola.error(`Reconciliation update failed: ${handle}`); + } return false; } } catch (error) { + if (defer) { + if (error.code === "ENOENT") { + deferredErrors.push({ + kind: "exception", + handle, + message: `The path ${error.path} was not found, please ensure you've imported or created all required files`, + }); + } else { + deferredErrors.push({ + kind: "exception", + handle, + message: error.message || String(error), + }); + } + return false; + } errorUtils.errorHandler(error); } } @@ -199,10 +235,15 @@ async function publishReconciliationById(type, envId, reconciliationId, message } async function publishAllReconciliations(type, envId, message = "updated through the Silverfin CLI") { + const deferredErrors = []; const templates = fsUtils.getAllTemplatesOfAType("reconciliationText"); for (const handle of templates) { if (!handle) continue; - await publishReconciliationByHandle(type, envId, handle, message); + await publishReconciliationByHandle(type, envId, handle, message, deferredErrors); + } + errorUtils.printReconciliationBatchErrorSummary(deferredErrors); + if (deferredErrors.length > 0) { + process.exitCode = 1; } } diff --git a/lib/utils/errorUtils.js b/lib/utils/errorUtils.js index 8869eef..d6b8b40 100644 --- a/lib/utils/errorUtils.js +++ b/lib/utils/errorUtils.js @@ -53,6 +53,39 @@ function missingAccountTemplateId(name) { return false; } +/** + * Print errors collected during publishAllReconciliations (after the loop). + * @param {Array<{ kind: string, handle?: string, message?: string }>} errors + */ +function printReconciliationBatchErrorSummary(errors) { + if (!errors || errors.length === 0) { + return; + } + + consola.log(""); + consola.error(`Reconciliation update finished with ${errors.length} error(s):`); + + const hadMissingId = errors.some((e) => e.kind === "missing_id"); + + for (const e of errors) { + if (e.kind === "missing_id") { + consola.error( + `Reconciliation ${e.handle}: ID is missing. Please check your command for typos and check if the folder name matches the name_nl` + ); + } else if (e.kind === "update_failed") { + consola.error(`Reconciliation update failed: ${e.handle}`); + } else if (e.kind === "exception") { + consola.error(e.handle ? `Reconciliation ${e.handle}: ${e.message}` : e.message); + } + } + + if (hadMissingId) { + consola.log( + `Try running: ${chalk.bold("silverfin get-reconciliation-id --all")} (or ${chalk.bold("silverfin get-reconciliation-id --handle ")} for one template)` + ); + } +} + module.exports = { uncaughtErrors, errorHandler, @@ -61,4 +94,5 @@ module.exports = { missingSharedPartId, missingExportFileId, missingAccountTemplateId, + printReconciliationBatchErrorSummary, }; From 7a52f740ded1d1b01097fa0871864f7490ada940 Mon Sep 17 00:00:00 2001 From: Michiel Degezelle Date: Tue, 24 Mar 2026 14:55:41 +0100 Subject: [PATCH 3/6] Create error summary for update-all export files --- index.js | 59 +++++++++++++++++++++++++++++++++++++---- lib/utils/errorUtils.js | 36 ++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index 11326e8..fc79b7f 100644 --- a/index.js +++ b/index.js @@ -184,12 +184,16 @@ async function publishReconciliationByHandle( kind: "exception", handle, message: `The path ${error.path} was not found, please ensure you've imported or created all required files`, + stack: error.stack, + rawError: error, }); } else { deferredErrors.push({ kind: "exception", handle, message: error.message || String(error), + stack: error.stack, + rawError: error, }); } return false; @@ -360,12 +364,24 @@ async function fetchExistingExportFiles(type, envId) { }); } -async function publishExportFileByName(type, envId, name, message = "updated through the Silverfin CLI") { +async function publishExportFileByName( + type, + envId, + name, + message = "updated through the Silverfin CLI", + deferredErrors = null +) { + const defer = Array.isArray(deferredErrors); + try { const configPresent = fsUtils.configExists("exportFile", name); if (!configPresent) { - errorUtils.missingExportFileId(name); + if (defer) { + deferredErrors.push({ kind: "missing_id", name }); + } else { + errorUtils.missingExportFileId(name); + } return false; } @@ -373,7 +389,11 @@ async function publishExportFileByName(type, envId, name, message = "updated thr const templateId = fsUtils.getTemplateId(type, envId, templateConfig); if (!templateConfig || !templateId) { - errorUtils.missingExportFileId(name); + if (defer) { + deferredErrors.push({ kind: "missing_id", name }); + } else { + errorUtils.missingExportFileId(name); + } return false; } @@ -395,10 +415,34 @@ async function publishExportFileByName(type, envId, name, message = "updated thr consola.success(`Export file updated: ${response.data.name_nl}`); return true; } else { - consola.error(`Export file update failed: ${name}`); + if (defer) { + deferredErrors.push({ kind: "update_failed", name }); + } else { + consola.error(`Export file update failed: ${name}`); + } return false; } } catch (error) { + if (defer) { + if (error.code === "ENOENT") { + deferredErrors.push({ + kind: "exception", + name, + message: `The path ${error.path} was not found, please ensure you've imported or created all required files`, + stack: error.stack, + rawError: error, + }); + } else { + deferredErrors.push({ + kind: "exception", + name, + message: error.message || String(error), + stack: error.stack, + rawError: error, + }); + } + return false; + } errorUtils.errorHandler(error); } } @@ -439,10 +483,15 @@ async function publishExportFileById(type, envId, exportFileId, message = "Updat } async function publishAllExportFiles(type, envId, message = "updated through the Silverfin CLI") { + const deferredErrors = []; const templates = fsUtils.getAllTemplatesOfAType("exportFile"); for (const name of templates) { if (!name) continue; - await publishExportFileByName(type, envId, name, message); + await publishExportFileByName(type, envId, name, message, deferredErrors); + } + errorUtils.printExportFileBatchErrorSummary(deferredErrors); + if (deferredErrors.length > 0) { + process.exitCode = 1; } } diff --git a/lib/utils/errorUtils.js b/lib/utils/errorUtils.js index d6b8b40..5c4e89a 100644 --- a/lib/utils/errorUtils.js +++ b/lib/utils/errorUtils.js @@ -55,7 +55,7 @@ function missingAccountTemplateId(name) { /** * Print errors collected during publishAllReconciliations (after the loop). - * @param {Array<{ kind: string, handle?: string, message?: string }>} errors + * @param {Array<{ kind: string, handle?: string, message?: string, stack?: string, rawError?: unknown }>} errors */ function printReconciliationBatchErrorSummary(errors) { if (!errors || errors.length === 0) { @@ -86,6 +86,39 @@ function printReconciliationBatchErrorSummary(errors) { } } +/** + * Print errors collected during publishAllExportFiles (after the loop). + * @param {Array<{ kind: string, name?: string, message?: string, stack?: string, rawError?: unknown }>} errors + */ +function printExportFileBatchErrorSummary(errors) { + if (!errors || errors.length === 0) { + return; + } + + consola.log(""); + consola.error(`Export file update finished with ${errors.length} error(s):`); + + const hadMissingId = errors.some((e) => e.kind === "missing_id"); + + for (const e of errors) { + if (e.kind === "missing_id") { + consola.error( + `Export file ${e.name}: ID is missing. Aborted. Please check your command for typos and check if the folder name matches the name_nl` + ); + } else if (e.kind === "update_failed") { + consola.error(`Export file update failed: ${e.name}`); + } else if (e.kind === "exception") { + consola.error(e.name ? `Export file ${e.name}: ${e.message}` : e.message); + } + } + + if (hadMissingId) { + consola.log( + `Try running: ${chalk.bold("silverfin get-export-file-id --all")} (or ${chalk.bold('silverfin get-export-file-id --name ""')} for one template)` + ); + } +} + module.exports = { uncaughtErrors, errorHandler, @@ -95,4 +128,5 @@ module.exports = { missingExportFileId, missingAccountTemplateId, printReconciliationBatchErrorSummary, + printExportFileBatchErrorSummary, }; From 8d6ebb033e42788b8ccd22f4279cc1f6dbc759d7 Mon Sep 17 00:00:00 2001 From: Michiel Degezelle Date: Tue, 24 Mar 2026 14:59:40 +0100 Subject: [PATCH 4/6] Create error summary for update-all shared parts --- index.js | 55 +++++++++++++++++++++++++++++++++++++---- lib/utils/errorUtils.js | 34 +++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index fc79b7f..c1a7231 100644 --- a/index.js +++ b/index.js @@ -814,18 +814,34 @@ async function fetchExistingSharedParts(type, envId) { } } -async function publishSharedPartByName(type, envId, name, message = "Updated through the Silverfin CLI") { +async function publishSharedPartByName( + type, + envId, + name, + message = "Updated through the Silverfin CLI", + deferredErrors = null +) { + const defer = Array.isArray(deferredErrors); + try { const configPresent = fsUtils.configExists("sharedPart", name); if (!configPresent) { - errorUtils.missingSharedPartId(name); + if (defer) { + deferredErrors.push({ kind: "missing_id", name }); + } else { + errorUtils.missingSharedPartId(name); + } return false; } const templateConfig = fsUtils.readConfig("sharedPart", name); const templateId = fsUtils.getTemplateId(type, envId, templateConfig); if (!templateConfig || !templateId) { - errorUtils.missingSharedPartId(name); + if (defer) { + deferredErrors.push({ kind: "missing_id", name }); + } else { + errorUtils.missingSharedPartId(name); + } return false; } consola.debug(`Updating shared part ${name}...`); @@ -846,10 +862,34 @@ async function publishSharedPartByName(type, envId, name, message = "Updated thr consola.success(`Shared part updated: ${response.data.name}`); return true; } else { - consola.error(`Shared part update failed: ${name}`); + if (defer) { + deferredErrors.push({ kind: "update_failed", name }); + } else { + consola.error(`Shared part update failed: ${name}`); + } return false; } } catch (error) { + if (defer) { + if (error.code === "ENOENT") { + deferredErrors.push({ + kind: "exception", + name, + message: `The path ${error.path} was not found, please ensure you've imported or created all required files`, + stack: error.stack, + rawError: error, + }); + } else { + deferredErrors.push({ + kind: "exception", + name, + message: error.message || String(error), + stack: error.stack, + rawError: error, + }); + } + return false; + } errorUtils.errorHandler(error); } } @@ -890,10 +930,15 @@ async function publishSharedPartById(type, envId, sharedPartId, message = "Updat } async function publishAllSharedParts(type, envId, message = "updated through the Silverfin CLI") { + const deferredErrors = []; const templates = fsUtils.getAllTemplatesOfAType("sharedPart"); for (const name of templates) { if (!name) continue; - await publishSharedPartByName(type, envId, name, message); + await publishSharedPartByName(type, envId, name, message, deferredErrors); + } + errorUtils.printSharedPartBatchErrorSummary(deferredErrors); + if (deferredErrors.length > 0) { + process.exitCode = 1; } } diff --git a/lib/utils/errorUtils.js b/lib/utils/errorUtils.js index 5c4e89a..203569e 100644 --- a/lib/utils/errorUtils.js +++ b/lib/utils/errorUtils.js @@ -119,6 +119,39 @@ function printExportFileBatchErrorSummary(errors) { } } +/** + * Print errors collected during publishAllSharedParts (after the loop). + * @param {Array<{ kind: string, name?: string, message?: string, stack?: string, rawError?: unknown }>} errors + */ +function printSharedPartBatchErrorSummary(errors) { + if (!errors || errors.length === 0) { + return; + } + + consola.log(""); + consola.error(`Shared part update finished with ${errors.length} error(s):`); + + const hadMissingId = errors.some((e) => e.kind === "missing_id"); + + for (const e of errors) { + if (e.kind === "missing_id") { + consola.error( + `Shared part ${e.name}: ID is missing. Aborted. Please check your command for typos and check if the folder name matches the name_nl` + ); + } else if (e.kind === "update_failed") { + consola.error(`Shared part update failed: ${e.name}`); + } else if (e.kind === "exception") { + consola.error(e.name ? `Shared part ${e.name}: ${e.message}` : e.message); + } + } + + if (hadMissingId) { + consola.log( + `Try running: ${chalk.bold("silverfin get-shared-part-id --all")} (or ${chalk.bold("silverfin get-shared-part-id --shared-part ")} for one template)` + ); + } +} + module.exports = { uncaughtErrors, errorHandler, @@ -129,4 +162,5 @@ module.exports = { missingAccountTemplateId, printReconciliationBatchErrorSummary, printExportFileBatchErrorSummary, + printSharedPartBatchErrorSummary, }; From b2ed537737e39b9ed3f2db9b04f48472ba621087 Mon Sep 17 00:00:00 2001 From: Michiel Degezelle Date: Tue, 24 Mar 2026 15:00:04 +0100 Subject: [PATCH 5/6] Create error summary for update-all account templates --- index.js | 55 +++++++++++++++++++++++++++++++++++++---- lib/utils/errorUtils.js | 34 +++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index c1a7231..2a31043 100644 --- a/index.js +++ b/index.js @@ -600,12 +600,24 @@ async function fetchExistingAccountTemplates(type, envId) { }); } -async function publishAccountTemplateByName(type, envId, name, message = "updated through the Silverfin CLI") { +async function publishAccountTemplateByName( + type, + envId, + name, + message = "updated through the Silverfin CLI", + deferredErrors = null +) { + const defer = Array.isArray(deferredErrors); + try { const configPresent = fsUtils.configExists("accountTemplate", name); if (!configPresent) { - errorUtils.missingAccountTemplateId(name); + if (defer) { + deferredErrors.push({ kind: "missing_id", name }); + } else { + errorUtils.missingAccountTemplateId(name); + } return false; } @@ -613,7 +625,11 @@ async function publishAccountTemplateByName(type, envId, name, message = "update const templateId = fsUtils.getTemplateId(type, envId, templateConfig); if (!templateConfig || !templateId) { - errorUtils.missingAccountTemplateId(name); + if (defer) { + deferredErrors.push({ kind: "missing_id", name }); + } else { + errorUtils.missingAccountTemplateId(name); + } return false; } @@ -641,10 +657,34 @@ async function publishAccountTemplateByName(type, envId, name, message = "update consola.success(`Account template updated: ${response.data.name_nl}`); return true; } else { - consola.error(`Account template update failed: ${name}`); + if (defer) { + deferredErrors.push({ kind: "update_failed", name }); + } else { + consola.error(`Account template update failed: ${name}`); + } return false; } } catch (error) { + if (defer) { + if (error.code === "ENOENT") { + deferredErrors.push({ + kind: "exception", + name, + message: `The path ${error.path} was not found, please ensure you've imported or created all required files`, + stack: error.stack, + rawError: error, + }); + } else { + deferredErrors.push({ + kind: "exception", + name, + message: error.message || String(error), + stack: error.stack, + rawError: error, + }); + } + return false; + } errorUtils.errorHandler(error); } } @@ -691,10 +731,15 @@ async function publishAccountTemplateById(type, envId, accountTemplateId, messag } async function publishAllAccountTemplates(type, envId, message = "updated through the Silverfin CLI") { + const deferredErrors = []; const templates = fsUtils.getAllTemplatesOfAType("accountTemplate"); for (const name of templates) { if (!name) continue; - await publishAccountTemplateByName(type, envId, name, message); + await publishAccountTemplateByName(type, envId, name, message, deferredErrors); + } + errorUtils.printAccountTemplateBatchErrorSummary(deferredErrors); + if (deferredErrors.length > 0) { + process.exitCode = 1; } } diff --git a/lib/utils/errorUtils.js b/lib/utils/errorUtils.js index 203569e..b7b7148 100644 --- a/lib/utils/errorUtils.js +++ b/lib/utils/errorUtils.js @@ -152,6 +152,39 @@ function printSharedPartBatchErrorSummary(errors) { } } +/** + * Print errors collected during publishAllAccountTemplates (after the loop). + * @param {Array<{ kind: string, name?: string, message?: string, stack?: string, rawError?: unknown }>} errors + */ +function printAccountTemplateBatchErrorSummary(errors) { + if (!errors || errors.length === 0) { + return; + } + + consola.log(""); + consola.error(`Account template update finished with ${errors.length} error(s):`); + + const hadMissingId = errors.some((e) => e.kind === "missing_id"); + + for (const e of errors) { + if (e.kind === "missing_id") { + consola.error( + `Account template ${e.name}: ID is missing. Aborted. Please check your command for typos and check if the folder name matches the name_nl` + ); + } else if (e.kind === "update_failed") { + consola.error(`Account template update failed: ${e.name}`); + } else if (e.kind === "exception") { + consola.error(e.name ? `Account template ${e.name}: ${e.message}` : e.message); + } + } + + if (hadMissingId) { + consola.log( + `Try running: ${chalk.bold("silverfin get-account-template-id --all")} (or ${chalk.bold('silverfin get-account-template-id --name ""')} for one template)` + ); + } +} + module.exports = { uncaughtErrors, errorHandler, @@ -163,4 +196,5 @@ module.exports = { printReconciliationBatchErrorSummary, printExportFileBatchErrorSummary, printSharedPartBatchErrorSummary, + printAccountTemplateBatchErrorSummary, }; From d550c3cd8fc5c070b6077fbd41ac73992cef70b7 Mon Sep 17 00:00:00 2001 From: Michiel Degezelle Date: Tue, 24 Mar 2026 15:05:56 +0100 Subject: [PATCH 6/6] Add tests --- .gitignore | 4 +- tests/lib/batchUpdateSummaries.test.js | 232 +++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 tests/lib/batchUpdateSummaries.test.js diff --git a/.gitignore b/.gitignore index 1ef6b16..09b90e6 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,6 @@ node_modules .env .DS_Store -./tmp \ No newline at end of file +./tmp +# Leftover from tests/bin/cli/import-reconciliation.test.js (mkdtemp) if a run is interrupted +tests/bin/cli/temp-* \ No newline at end of file diff --git a/tests/lib/batchUpdateSummaries.test.js b/tests/lib/batchUpdateSummaries.test.js new file mode 100644 index 0000000..e184d86 --- /dev/null +++ b/tests/lib/batchUpdateSummaries.test.js @@ -0,0 +1,232 @@ +const toolkit = require("../../index"); +const fsUtils = require("../../lib/utils/fsUtils"); +const SF = require("../../lib/api/sfApi"); +const { ReconciliationText } = require("../../lib/templates/reconciliationText"); +const { ExportFile } = require("../../lib/templates/exportFile"); +const { AccountTemplate } = require("../../lib/templates/accountTemplate"); +const { SharedPart } = require("../../lib/templates/sharedPart"); +const errorUtils = require("../../lib/utils/errorUtils"); + +jest.mock("../../lib/utils/apiUtils", () => ({ + checkRequiredEnvVariables: jest.fn(() => true), +})); + +jest.mock("../../lib/utils/fsUtils"); +jest.mock("../../lib/api/sfApi"); +jest.mock("../../lib/templates/reconciliationText"); +jest.mock("../../lib/templates/exportFile"); +jest.mock("../../lib/templates/accountTemplate"); +jest.mock("../../lib/templates/sharedPart"); + +jest.mock("consola", () => { + const consola = { + debug: jest.fn(), + success: jest.fn(), + error: jest.fn(), + log: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + }; + return { consola }; +}); + +const { consola } = require("consola"); + +describe("Batch update error summaries", () => { + const mockType = "firm"; + const mockEnvId = "100"; + const mockMessage = "batch test message"; + + beforeEach(() => { + jest.clearAllMocks(); + process.exitCode = 0; + fsUtils.getTemplateId.mockImplementation((_type, envId, config) => config?.id?.[envId] ?? null); + }); + + describe("errorUtils.print*BatchErrorSummary", () => { + it("printReconciliationBatchErrorSummary does nothing for empty array", () => { + errorUtils.printReconciliationBatchErrorSummary([]); + expect(consola.error).not.toHaveBeenCalled(); + }); + + it("printReconciliationBatchErrorSummary prints missing_id and deduped hint", () => { + errorUtils.printReconciliationBatchErrorSummary([{ kind: "missing_id", handle: "h1" }]); + expect(consola.error).toHaveBeenCalledWith("Reconciliation update finished with 1 error(s):"); + expect(consola.error).toHaveBeenCalledWith( + expect.stringContaining("Reconciliation h1: ID is missing") + ); + expect(consola.log).toHaveBeenCalledWith(expect.stringContaining("get-reconciliation-id --all")); + }); + + it("printExportFileBatchErrorSummary prints update_failed without missing-id hint", () => { + errorUtils.printExportFileBatchErrorSummary([{ kind: "update_failed", name: "exp1" }]); + expect(consola.error).toHaveBeenCalledWith("Export file update finished with 1 error(s):"); + expect(consola.error).toHaveBeenCalledWith("Export file update failed: exp1"); + const hintLogged = consola.log.mock.calls.some((c) => String(c[0]).includes("get-export-file-id")); + expect(hintLogged).toBe(false); + }); + + it("printSharedPartBatchErrorSummary prints exception line", () => { + errorUtils.printSharedPartBatchErrorSummary([ + { kind: "exception", name: "sp1", message: "Something broke" }, + ]); + expect(consola.error).toHaveBeenCalledWith("Shared part update finished with 1 error(s):"); + expect(consola.error).toHaveBeenCalledWith("Shared part sp1: Something broke"); + }); + + it("printSharedPartBatchErrorSummary prints deduped get-shared-part-id hint for missing_id", () => { + errorUtils.printSharedPartBatchErrorSummary([{ kind: "missing_id", name: "sp1" }]); + expect(consola.log).toHaveBeenCalledWith(expect.stringContaining("get-shared-part-id --all")); + }); + + it("printAccountTemplateBatchErrorSummary prints missing_id hint once for two rows", () => { + errorUtils.printAccountTemplateBatchErrorSummary([ + { kind: "missing_id", name: "at1" }, + { kind: "missing_id", name: "at2" }, + ]); + const logCalls = consola.log.mock.calls.map((c) => c[0]); + const hintCalls = logCalls.filter((m) => String(m).includes("get-account-template-id --all")); + expect(hintCalls.length).toBe(1); + }); + }); + + describe("publishAllReconciliations", () => { + it("defers missing-id errors to summary and sets exitCode", async () => { + fsUtils.getAllTemplatesOfAType.mockReturnValue(["good", "bad"]); + fsUtils.configExists.mockReturnValue(true); + fsUtils.readConfig.mockImplementation((_tt, handle) => + handle === "good" ? { id: { [mockEnvId]: "999" } } : { id: { [mockEnvId]: null } } + ); + + ReconciliationText.read.mockResolvedValue({ + handle: "good", + text: "x", + text_parts: [], + }); + SF.updateReconciliationText.mockResolvedValue({ data: { handle: "good" } }); + + await toolkit.publishAllReconciliations(mockType, mockEnvId, mockMessage); + + expect(consola.success).toHaveBeenCalledWith("Reconciliation updated: good"); + expect(consola.error).toHaveBeenCalledWith("Reconciliation update finished with 1 error(s):"); + expect(process.exitCode).toBe(1); + }); + }); + + describe("publishAllExportFiles", () => { + it("defers missing-id errors to summary and sets exitCode", async () => { + fsUtils.getAllTemplatesOfAType.mockReturnValue(["ok_export", "bad_export"]); + fsUtils.configExists.mockReturnValue(true); + fsUtils.readConfig.mockImplementation((_tt, name) => + name === "ok_export" ? { id: { [mockEnvId]: "888" } } : { id: {} } + ); + + ExportFile.read.mockResolvedValue({ + name_nl: "ok_export", + text: "x", + }); + SF.updateExportFile.mockResolvedValue({ data: { name_nl: "ok_export" } }); + + await toolkit.publishAllExportFiles(mockType, mockEnvId, mockMessage); + + expect(consola.success).toHaveBeenCalledWith("Export file updated: ok_export"); + expect(consola.error).toHaveBeenCalledWith("Export file update finished with 1 error(s):"); + expect(process.exitCode).toBe(1); + }); + }); + + describe("publishAllSharedParts", () => { + it("defers missing-id errors to summary and sets exitCode", async () => { + fsUtils.getAllTemplatesOfAType.mockReturnValue(["ok_sp", "bad_sp"]); + fsUtils.configExists.mockReturnValue(true); + fsUtils.readConfig.mockImplementation((_tt, name) => + name === "ok_sp" ? { id: { [mockEnvId]: "777" } } : { id: {} } + ); + + SharedPart.read.mockResolvedValue({ + name: "ok_sp", + text: "x", + }); + SF.updateSharedPart.mockResolvedValue({ data: { name: "ok_sp" } }); + + await toolkit.publishAllSharedParts(mockType, mockEnvId, mockMessage); + + expect(consola.success).toHaveBeenCalledWith("Shared part updated: ok_sp"); + expect(consola.error).toHaveBeenCalledWith("Shared part update finished with 1 error(s):"); + expect(process.exitCode).toBe(1); + }); + }); + + describe("publishAllAccountTemplates", () => { + it("defers missing-id errors to summary and sets exitCode", async () => { + fsUtils.getAllTemplatesOfAType.mockReturnValue(["ok_at", "bad_at"]); + fsUtils.configExists.mockReturnValue(true); + fsUtils.readConfig.mockImplementation((_tt, name) => + name === "ok_at" ? { id: { [mockEnvId]: "666" } } : { id: {} } + ); + + AccountTemplate.read.mockResolvedValue({ + name_nl: "ok_at", + text: "x", + mapping_list_ranges: [], + }); + SF.updateAccountTemplate.mockResolvedValue({ data: { name_nl: "ok_at" } }); + + await toolkit.publishAllAccountTemplates(mockType, mockEnvId, mockMessage); + + expect(consola.success).toHaveBeenCalledWith("Account template updated: ok_at"); + expect(consola.error).toHaveBeenCalledWith("Account template update finished with 1 error(s):"); + expect(process.exitCode).toBe(1); + }); + }); + + describe("publish*ByName with deferredErrors array", () => { + it("publishReconciliationByHandle pushes to deferredErrors instead of calling missingReconciliationId", async () => { + const deferred = []; + fsUtils.configExists.mockReturnValue(true); + fsUtils.readConfig.mockReturnValue({ id: {} }); + + await toolkit.publishReconciliationByHandle(mockType, mockEnvId, "solo", mockMessage, deferred); + + expect(deferred).toEqual([{ kind: "missing_id", handle: "solo" }]); + }); + + it("publishExportFileByName pushes update_failed when API returns no data", async () => { + const deferred = []; + fsUtils.configExists.mockReturnValue(true); + fsUtils.readConfig.mockReturnValue({ id: { [mockEnvId]: "1" } }); + ExportFile.read.mockResolvedValue({ name_nl: "x", text: "y" }); + SF.updateExportFile.mockResolvedValue(null); + + await toolkit.publishExportFileByName(mockType, mockEnvId, "e1", mockMessage, deferred); + + expect(deferred).toEqual([{ kind: "update_failed", name: "e1" }]); + }); + + it("publishSharedPartByName pushes missing_id to deferredErrors", async () => { + const deferred = []; + fsUtils.configExists.mockReturnValue(true); + fsUtils.readConfig.mockReturnValue({ id: {} }); + + await toolkit.publishSharedPartByName(mockType, mockEnvId, "sp_solo", mockMessage, deferred); + + expect(deferred).toEqual([{ kind: "missing_id", name: "sp_solo" }]); + }); + + it("publishAccountTemplateByName pushes update_failed when API returns no data", async () => { + const deferred = []; + fsUtils.configExists.mockReturnValue(true); + fsUtils.readConfig.mockReturnValue({ id: { [mockEnvId]: "1" } }); + AccountTemplate.read.mockResolvedValue({ + name_nl: "at1", + text: "y", + mapping_list_ranges: [], + }); + SF.updateAccountTemplate.mockResolvedValue(null); + + await toolkit.publishAccountTemplateByName(mockType, mockEnvId, "at1", mockMessage, deferred); + + expect(deferred).toEqual([{ kind: "update_failed", name: "at1" }]); + }); + }); +});