From 4c3a0e566f3e981406c8d50f31c856a8701d368f Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 16 Mar 2026 20:12:12 +0100 Subject: [PATCH 01/14] Add helper function to post TPs --- lib/api/sfApi.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/api/sfApi.js b/lib/api/sfApi.js index 6e54717d..66aa9759 100644 --- a/lib/api/sfApi.js +++ b/lib/api/sfApi.js @@ -572,6 +572,18 @@ async function getAllPeriodCustom(firmId, companyId, periodId) { return items; } +async function updateReconciliationCustom(firmId, companyId, periodId, reconciliationId, properties) { + const instance = AxiosFactory.createInstance("firm", firmId); + try { + const response = await instance.post(`/companies/${companyId}/periods/${periodId}/reconciliations/${reconciliationId}/custom`, { properties }); + apiUtils.responseSuccessHandler(response); + return response; + } catch (error) { + const response = await apiUtils.responseErrorHandler(error); + return response; + } +} + async function getWorkflows(firmId, companyId, periodId) { const instance = AxiosFactory.createInstance("firm", firmId); try { @@ -761,6 +773,7 @@ module.exports = { getCompanyCustom, getPeriodCustom, getAllPeriodCustom, + updateReconciliationCustom, getWorkflows, getWorkflowInformation, findReconciliationInWorkflow, From 0c5811d9fcd32c67284934b4d318e7efd517c899 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 16 Mar 2026 20:18:34 +0100 Subject: [PATCH 02/14] Add utils to parse YAML files and create POST payload --- lib/utils/textPropertyUtils.js | 130 +++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 lib/utils/textPropertyUtils.js diff --git a/lib/utils/textPropertyUtils.js b/lib/utils/textPropertyUtils.js new file mode 100644 index 00000000..7fc0aeb0 --- /dev/null +++ b/lib/utils/textPropertyUtils.js @@ -0,0 +1,130 @@ +const fs = require("fs"); +const path = require("path"); +const yaml = require("yaml"); +const { consola } = require("consola"); +const fsUtils = require("./fsUtils"); + +/** + * Transform a YAML custom properties object into the Silverfin API format. + * Input: flat object with dot-notation keys (e.g. "namespace.key.subkey": value) + * Output: array of { namespace, key, value } objects + */ +function transformCustomToProperties(customData) { + const namespaceMap = new Map(); + + for (const [fullKey, value] of Object.entries(customData)) { + const keyParts = fullKey.split("."); + + if (keyParts.length < 2) { + consola.warn(`Skipping key "${fullKey}" — expected namespace.key format`); + continue; + } + + const namespace = keyParts[0]; + const key = keyParts[1]; + const namespaceKey = `${namespace}.${key}`; + + if (keyParts.length === 2) { + if (!namespaceMap.has(namespaceKey)) { + namespaceMap.set(namespaceKey, { namespace, key, value }); + } + } else { + if (!namespaceMap.has(namespaceKey)) { + namespaceMap.set(namespaceKey, { namespace, key, value: {} }); + } + const subKey = keyParts.slice(2).join("."); + namespaceMap.get(namespaceKey).value[subKey] = value; + } + } + + return Array.from(namespaceMap.values()); +} + +/** + * Find a test by name across YAML files in a template's tests/ folder. + * If handle is provided, only search that handle's folder. + * If not, scan all reconciliation_texts folders and use reconciliationId to disambiguate. + * Returns { custom, handle, periodKey } or exits with an error. + */ +function findTestData(testName, handle, reconciliationId, firmId) { + const templateType = "reconciliationText"; + const baseDir = path.join(process.cwd(), fsUtils.FOLDERS[templateType]); + + if (!fs.existsSync(baseDir)) { + consola.error(`Directory not found: ${fsUtils.FOLDERS[templateType]}`); + process.exit(1); + } + + const handleDirs = handle ? [handle] : fs.readdirSync(baseDir).filter((entry) => { + return fs.statSync(path.join(baseDir, entry)).isDirectory(); + }); + + const matches = []; + + for (const dir of handleDirs) { + const testsDir = path.join(baseDir, dir, "tests"); + if (!fs.existsSync(testsDir)) continue; + + const yamlFiles = fs.readdirSync(testsDir).filter((f) => f.endsWith(".yml")); + + for (const file of yamlFiles) { + const filePath = path.join(testsDir, file); + const content = fs.readFileSync(filePath, "utf-8"); + const parsed = yaml.parse(content, { maxAliasCount: 10000 }); + + if (!parsed || !parsed[testName]) continue; + + const testData = parsed[testName]; + const periods = testData?.data?.periods; + if (!periods) continue; + + const periodKey = Object.keys(periods)[0]; + const reconciliations = periods[periodKey]?.reconciliations; + if (!reconciliations) continue; + + for (const [reconHandle, reconData] of Object.entries(reconciliations)) { + if (reconData?.custom) { + matches.push({ + handle: dir, + reconHandle, + periodKey, + custom: reconData.custom, + file, + }); + } + } + } + } + + if (matches.length === 0) { + consola.error(`Test "${testName}" not found in any YAML file`); + process.exit(1); + } + + if (matches.length === 1) { + return matches[0]; + } + + // Multiple matches — disambiguate using reconciliationId from config.json + if (reconciliationId && firmId) { + for (const match of matches) { + try { + const config = fsUtils.readConfig(templateType, match.handle); + const templateId = config?.id?.[firmId]; + if (String(templateId) === String(reconciliationId)) { + return match; + } + } catch { + // config not found for this handle, skip + } + } + } + + consola.error( + `Test "${testName}" found in multiple templates: ${matches.map((m) => m.handle).join(", ")}. ` + + `Use --handle to specify which one.` + ); + process.exit(1); +} + +module.exports = { transformCustomToProperties, findTestData }; From d48e3282a202e990841bda1019499516b595a6fa Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 16 Mar 2026 20:20:59 +0100 Subject: [PATCH 03/14] Add consola commands --- .gitignore | 3 ++- bin/cli.js | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 1ef6b16c..d3240e69 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ node_modules .env .DS_Store -./tmp \ No newline at end of file +./tmp +.cursor \ No newline at end of file diff --git a/bin/cli.js b/bin/cli.js index 79858a13..2891bf5a 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -19,6 +19,8 @@ const { runCommandChecks } = require("../lib/cli/utils"); const { CwdValidator } = require("../lib/cli/cwdValidator"); const { AutoCompletions } = require("../lib/cli/autoCompletions"); const fsUtils = require("../lib/utils/fsUtils"); +const textPropertyUtils = require("../lib/utils/textPropertyUtils"); +const liquidTestUtils = require("../lib/utils/liquidTestUtils"); const firmIdDefault = cliUtils.loadDefaultFirmId(); cliUtils.handleUncaughtErrors(); @@ -527,6 +529,43 @@ program liquidTestGenerator.testGenerator(options.url, testName, reconciledStatus); }); +// Update Text Properties from Liquid Test data +program + .command("update-text-properties") + .description("Upload custom text properties from a Liquid Test YAML file to a reconciliation in a company file") + .requiredOption("-u, --url ", "Specify the full Silverfin URL of the reconciliation in the company file (mandatory)") + .requiredOption("-t, --test ", "Specify the name of the test to use as data source (mandatory)") + .option("-h, --handle ", "Specify the reconciliation handle to narrow down the YAML file search (optional)") + .option("--dry-run", "Only transform and display the properties without uploading (optional)", false) + .action(async (options) => { + // Parse URL to extract IDs + const urlData = liquidTestUtils.extractURL(options.url); + const { firmId, companyId, ledgerId: periodId, reconciliationId } = urlData; + + // Find the test data in the YAML files + const testData = textPropertyUtils.findTestData(options.test, options.handle, reconciliationId, firmId); + consola.info(`Found test "${options.test}" in ${testData.handle}/tests/${testData.file}`); + + // Transform custom properties to API format + const properties = textPropertyUtils.transformCustomToProperties(testData.custom); + consola.info(`Transformed ${properties.length} properties`); + + if (options.dryRun) { + consola.info("Dry run — properties that would be uploaded:"); + consola.log(JSON.stringify(properties, null, 2)); + return; + } + + // Upload to Silverfin + const response = await SF.updateReconciliationCustom(firmId, companyId, periodId, reconciliationId, properties); + if (response && response.status >= 200 && response.status < 300) { + consola.success("Text properties updated successfully"); + } else { + consola.error("Failed to update text properties"); + process.exit(1); + } + }); + // Check Liquid Test dependencies for a reconciliation template program .command("check-dependencies") From 6fe5858f04b4cf164c868a724c28b0fc2fdebe06 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 16 Mar 2026 21:30:50 +0100 Subject: [PATCH 04/14] Fix to skip null period in textPropertyUtils --- lib/utils/textPropertyUtils.js | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/utils/textPropertyUtils.js b/lib/utils/textPropertyUtils.js index 7fc0aeb0..6a2bb0c3 100644 --- a/lib/utils/textPropertyUtils.js +++ b/lib/utils/textPropertyUtils.js @@ -78,19 +78,20 @@ function findTestData(testName, handle, reconciliationId, firmId) { const periods = testData?.data?.periods; if (!periods) continue; - const periodKey = Object.keys(periods)[0]; - const reconciliations = periods[periodKey]?.reconciliations; - if (!reconciliations) continue; - - for (const [reconHandle, reconData] of Object.entries(reconciliations)) { - if (reconData?.custom) { - matches.push({ - handle: dir, - reconHandle, - periodKey, - custom: reconData.custom, - file, - }); + for (const [periodKey, periodData] of Object.entries(periods)) { + const reconciliations = periodData?.reconciliations; + if (!reconciliations) continue; + + for (const [reconHandle, reconData] of Object.entries(reconciliations)) { + if (reconData?.custom) { + matches.push({ + handle: dir, + reconHandle, + periodKey, + custom: reconData.custom, + file, + }); + } } } } From b3b059e9911e1e326abbdceac18289e07e9a71c4 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 16 Mar 2026 21:34:05 +0100 Subject: [PATCH 05/14] Handle anchors and aliases --- lib/utils/textPropertyUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils/textPropertyUtils.js b/lib/utils/textPropertyUtils.js index 6a2bb0c3..3d924d76 100644 --- a/lib/utils/textPropertyUtils.js +++ b/lib/utils/textPropertyUtils.js @@ -70,7 +70,7 @@ function findTestData(testName, handle, reconciliationId, firmId) { for (const file of yamlFiles) { const filePath = path.join(testsDir, file); const content = fs.readFileSync(filePath, "utf-8"); - const parsed = yaml.parse(content, { maxAliasCount: 10000 }); + const parsed = yaml.parse(content, { maxAliasCount: 10000, merge: true }); if (!parsed || !parsed[testName]) continue; From 40984d89946d74ab5c736dc8c2242e64554ec841 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 16 Mar 2026 21:48:19 +0100 Subject: [PATCH 06/14] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03f42861..ed773fa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ All notable changes to this project will be documented in this file. +## [1.55.0] (16/03/2026) +Added `update-text-properties` command. It uploads custom text properties from a Liquid Test YAML file to a reconciliation in a company file. Usage: `silverfin update-text-properties -u -t `. Supports `--handle` for faster YAML file lookup and `--dry-run` to preview the payload without uploading. + ## [1.54.0] (17/02/2026) Added `create-test` command support for account templates (fetches template data, period data, and custom data). From fd48970b79c37c6f1ab3eaf71962209ffd0dc997 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 23 Mar 2026 15:46:30 +0100 Subject: [PATCH 07/14] Support all custom property levels in update-text-properties Extract and push custom data from all 4 levels in liquid test YAML: company, period, reconciliation, and account. Company/period/account endpoints send one property per request. Reconciliation and account IDs are resolved via the API by handle/number. --- bin/cli.js | 105 ++++++++++++++++++++++++++++----- lib/api/sfApi.js | 51 ++++++++++++++++ lib/utils/textPropertyUtils.js | 87 +++++++++++++-------------- 3 files changed, 184 insertions(+), 59 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index 2891bf5a..77603a7c 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -540,29 +540,106 @@ program .action(async (options) => { // Parse URL to extract IDs const urlData = liquidTestUtils.extractURL(options.url); - const { firmId, companyId, ledgerId: periodId, reconciliationId } = urlData; + const { firmId, companyId } = urlData; // Find the test data in the YAML files - const testData = textPropertyUtils.findTestData(options.test, options.handle, reconciliationId, firmId); + const testData = textPropertyUtils.findTestData(options.test, options.handle); consola.info(`Found test "${options.test}" in ${testData.handle}/tests/${testData.file}`); - // Transform custom properties to API format - const properties = textPropertyUtils.transformCustomToProperties(testData.custom); - consola.info(`Transformed ${properties.length} properties`); + // Collect all updates to perform + const updates = []; + + // Company-level custom + if (testData.company?.custom) { + const properties = textPropertyUtils.transformCustomToProperties(testData.company.custom); + updates.push({ level: "company", properties, apply: () => SF.updateCompanyCustom(firmId, companyId, properties) }); + } + + // Fetch periods once if needed for resolving period dates + let periodsArray = null; + + for (const [periodKey, periodEntry] of Object.entries(testData.periods)) { + // Resolve period date to period ID + if (!periodsArray) { + const periodsResponse = await SF.getPeriods(firmId, companyId); + periodsArray = periodsResponse?.data || []; + } + const period = periodsArray.find((p) => p.fiscal_year.end_date === periodKey); + if (!period) { + consola.warn(`Period "${periodKey}" not found in company — skipping`); + continue; + } + const targetPeriodId = period.id; + + // Period-level custom + if (periodEntry.custom) { + const properties = textPropertyUtils.transformCustomToProperties(periodEntry.custom); + updates.push({ level: `period [${periodKey}]`, properties, apply: () => SF.updatePeriodCustom(firmId, companyId, targetPeriodId, properties) }); + } + + // Reconciliation-level custom + for (const [reconHandle, customData] of Object.entries(periodEntry.reconciliations)) { + const properties = textPropertyUtils.transformCustomToProperties(customData); + updates.push({ + level: `reconciliation [${reconHandle}] in ${periodKey}`, + properties, + apply: async () => { + const recon = await SF.findReconciliationInWorkflows(firmId, reconHandle, companyId, targetPeriodId); + if (!recon) { + consola.error(`Reconciliation "${reconHandle}" not found in any workflow for period ${periodKey}`); + return null; + } + return SF.updateReconciliationCustom(firmId, companyId, targetPeriodId, recon.id, properties); + }, + }); + } + + // Account-level custom + for (const [accountNumber, customData] of Object.entries(periodEntry.accounts)) { + const properties = textPropertyUtils.transformCustomToProperties(customData); + updates.push({ + level: `account [${accountNumber}] in ${periodKey}`, + properties, + apply: async () => { + const account = await SF.findAccountByNumber(firmId, companyId, targetPeriodId, accountNumber); + if (!account) { + consola.error(`Account "${accountNumber}" not found for period ${periodKey}`); + return null; + } + return SF.updateAccountCustom(firmId, companyId, targetPeriodId, account.account.id, properties); + }, + }); + } + } + + if (updates.length === 0) { + consola.warn("No custom properties found in this test"); + return; + } + + consola.info(`Found ${updates.length} custom update(s) to apply`); if (options.dryRun) { - consola.info("Dry run — properties that would be uploaded:"); - consola.log(JSON.stringify(properties, null, 2)); + for (const update of updates) { + consola.info(`[dry-run] ${update.level}: ${update.properties.length} properties`); + consola.log(JSON.stringify(update.properties, null, 2)); + } return; } - // Upload to Silverfin - const response = await SF.updateReconciliationCustom(firmId, companyId, periodId, reconciliationId, properties); - if (response && response.status >= 200 && response.status < 300) { - consola.success("Text properties updated successfully"); - } else { - consola.error("Failed to update text properties"); - process.exit(1); + // Apply all updates + for (const update of updates) { + consola.start(`Updating ${update.level} (${update.properties.length} properties)...`); + const response = await update.apply(); + if (!response) continue; + // Handle both single response (reconciliation) and array of responses (company/period/account) + const responses = Array.isArray(response) ? response : [response]; + const failed = responses.filter((r) => !r || r.status < 200 || r.status >= 300); + if (failed.length === 0) { + consola.success(`${update.level}: updated`); + } else { + consola.error(`${update.level}: ${failed.length}/${responses.length} failed`); + } } }); diff --git a/lib/api/sfApi.js b/lib/api/sfApi.js index 66aa9759..0ef6d667 100644 --- a/lib/api/sfApi.js +++ b/lib/api/sfApi.js @@ -584,6 +584,54 @@ async function updateReconciliationCustom(firmId, companyId, periodId, reconcili } } +async function updateCompanyCustom(firmId, companyId, properties) { + const instance = AxiosFactory.createInstance("firm", firmId); + const results = []; + for (const prop of properties) { + try { + const response = await instance.post(`/companies/${companyId}/custom`, prop); + apiUtils.responseSuccessHandler(response); + results.push(response); + } catch (error) { + const response = await apiUtils.responseErrorHandler(error); + results.push(response); + } + } + return results; +} + +async function updatePeriodCustom(firmId, companyId, periodId, properties) { + const instance = AxiosFactory.createInstance("firm", firmId); + const results = []; + for (const prop of properties) { + try { + const response = await instance.post(`/companies/${companyId}/periods/${periodId}/custom`, prop); + apiUtils.responseSuccessHandler(response); + results.push(response); + } catch (error) { + const response = await apiUtils.responseErrorHandler(error); + results.push(response); + } + } + return results; +} + +async function updateAccountCustom(firmId, companyId, periodId, accountId, properties) { + const instance = AxiosFactory.createInstance("firm", firmId); + const results = []; + for (const prop of properties) { + try { + const response = await instance.post(`/companies/${companyId}/periods/${periodId}/accounts/${accountId}/custom`, prop); + apiUtils.responseSuccessHandler(response); + results.push(response); + } catch (error) { + const response = await apiUtils.responseErrorHandler(error); + results.push(response); + } + } + return results; +} + async function getWorkflows(firmId, companyId, periodId) { const instance = AxiosFactory.createInstance("firm", firmId); try { @@ -774,6 +822,9 @@ module.exports = { getPeriodCustom, getAllPeriodCustom, updateReconciliationCustom, + updateCompanyCustom, + updatePeriodCustom, + updateAccountCustom, getWorkflows, getWorkflowInformation, findReconciliationInWorkflow, diff --git a/lib/utils/textPropertyUtils.js b/lib/utils/textPropertyUtils.js index 3d924d76..f408797c 100644 --- a/lib/utils/textPropertyUtils.js +++ b/lib/utils/textPropertyUtils.js @@ -43,10 +43,12 @@ function transformCustomToProperties(customData) { /** * Find a test by name across YAML files in a template's tests/ folder. * If handle is provided, only search that handle's folder. - * If not, scan all reconciliation_texts folders and use reconciliationId to disambiguate. - * Returns { custom, handle, periodKey } or exits with an error. + * If not, scan all reconciliation_texts folders. + * Extracts custom data from all 4 levels: company, period, reconciliation, account. + * Returns { file, handle, company, periods } where periods contains per-period custom, + * reconciliation custom, and account custom data. */ -function findTestData(testName, handle, reconciliationId, firmId) { +function findTestData(testName, handle) { const templateType = "reconciliationText"; const baseDir = path.join(process.cwd(), fsUtils.FOLDERS[templateType]); @@ -59,8 +61,6 @@ function findTestData(testName, handle, reconciliationId, firmId) { return fs.statSync(path.join(baseDir, entry)).isDirectory(); }); - const matches = []; - for (const dir of handleDirs) { const testsDir = path.join(baseDir, dir, "tests"); if (!fs.existsSync(testsDir)) continue; @@ -75,56 +75,53 @@ function findTestData(testName, handle, reconciliationId, firmId) { if (!parsed || !parsed[testName]) continue; const testData = parsed[testName]; + const result = { file, handle: dir, company: null, periods: {} }; + + // Company-level custom + if (testData?.data?.company?.custom) { + result.company = { custom: testData.data.company.custom }; + } + + // Period-level data const periods = testData?.data?.periods; - if (!periods) continue; - - for (const [periodKey, periodData] of Object.entries(periods)) { - const reconciliations = periodData?.reconciliations; - if (!reconciliations) continue; - - for (const [reconHandle, reconData] of Object.entries(reconciliations)) { - if (reconData?.custom) { - matches.push({ - handle: dir, - reconHandle, - periodKey, - custom: reconData.custom, - file, - }); + if (periods) { + for (const [periodKey, periodData] of Object.entries(periods)) { + if (!periodData) continue; + + const periodEntry = { custom: null, reconciliations: {}, accounts: {} }; + + // Period-level custom + if (periodData.custom) { + periodEntry.custom = periodData.custom; } - } - } - } - } - if (matches.length === 0) { - consola.error(`Test "${testName}" not found in any YAML file`); - process.exit(1); - } + // Reconciliation-level custom + if (periodData.reconciliations) { + for (const [reconHandle, reconData] of Object.entries(periodData.reconciliations)) { + if (reconData?.custom) { + periodEntry.reconciliations[reconHandle] = reconData.custom; + } + } + } - if (matches.length === 1) { - return matches[0]; - } + // Account-level custom + if (periodData.accounts) { + for (const [accountNumber, accountData] of Object.entries(periodData.accounts)) { + if (accountData?.custom) { + periodEntry.accounts[accountNumber] = accountData.custom; + } + } + } - // Multiple matches — disambiguate using reconciliationId from config.json - if (reconciliationId && firmId) { - for (const match of matches) { - try { - const config = fsUtils.readConfig(templateType, match.handle); - const templateId = config?.id?.[firmId]; - if (String(templateId) === String(reconciliationId)) { - return match; + result.periods[periodKey] = periodEntry; } - } catch { - // config not found for this handle, skip } + + return result; } } - consola.error( - `Test "${testName}" found in multiple templates: ${matches.map((m) => m.handle).join(", ")}. ` + - `Use --handle to specify which one.` - ); + consola.error(`Test "${testName}" not found in any YAML file`); process.exit(1); } From bfc7a5c32ffeb9cf24c3b39c22a1142038401f71 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 23 Mar 2026 15:51:09 +0100 Subject: [PATCH 08/14] Bump version to 1.55.0 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 287b023c..1f9d64c9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "silverfin-cli", - "version": "1.54.0", + "version": "1.55.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "silverfin-cli", - "version": "1.54.0", + "version": "1.55.0", "license": "MIT", "dependencies": { "axios": "^1.6.2", diff --git a/package.json b/package.json index 71536fd6..aa436728 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "silverfin-cli", - "version": "1.54.0", + "version": "1.55.0", "description": "Command line tool for Silverfin template development", "main": "index.js", "license": "MIT", From 06d247eb334a956ea09b750f88b03ef4815ad438 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 23 Mar 2026 16:01:25 +0100 Subject: [PATCH 09/14] Fix account ID guard and exit code on update failures --- bin/cli.js | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index 77603a7c..0ec2cf11 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -602,11 +602,12 @@ program properties, apply: async () => { const account = await SF.findAccountByNumber(firmId, companyId, targetPeriodId, accountNumber); - if (!account) { - consola.error(`Account "${accountNumber}" not found for period ${periodKey}`); + const accountId = account?.account?.id; + if (!accountId) { + consola.error(`Account "${accountNumber}" could not be resolved for period ${periodKey}`); return null; } - return SF.updateAccountCustom(firmId, companyId, targetPeriodId, account.account.id, properties); + return SF.updateAccountCustom(firmId, companyId, targetPeriodId, accountId, properties); }, }); } @@ -628,19 +629,27 @@ program } // Apply all updates + let hadFailures = false; for (const update of updates) { consola.start(`Updating ${update.level} (${update.properties.length} properties)...`); const response = await update.apply(); - if (!response) continue; + if (!response) { + hadFailures = true; + continue; + } // Handle both single response (reconciliation) and array of responses (company/period/account) const responses = Array.isArray(response) ? response : [response]; const failed = responses.filter((r) => !r || r.status < 200 || r.status >= 300); if (failed.length === 0) { consola.success(`${update.level}: updated`); } else { + hadFailures = true; consola.error(`${update.level}: ${failed.length}/${responses.length} failed`); } } + if (hadFailures) { + process.exitCode = 1; + } }); // Check Liquid Test dependencies for a reconciliation template From 1e85a88af9947da3bbb0abe4251fd9d0f7d3e602 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 29 Jun 2026 15:30:31 +0200 Subject: [PATCH 10/14] Address PR #249 review comments - R1: paginate company periods via getAllPeriods so companies with >200 periods no longer silently skip updates - R2: guard null fiscal_year when matching periods to YAML period keys - R3: skip malformed YAML test files (try/catch) instead of crashing - R4: add unit tests for textPropertyUtils (transform + findTestData) - R5: clarify the command description to reflect all-level writes and add a confirmation prompt summarising the target firm/levels (--yes to skip) - R6: validate firm/company parsed from the URL --- bin/cli.js | 26 ++++- lib/api/sfApi.js | 29 ++++++ lib/utils/textPropertyUtils.js | 8 +- tests/lib/utils/textPropertyUtils.test.js | 120 ++++++++++++++++++++++ 4 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 tests/lib/utils/textPropertyUtils.test.js diff --git a/bin/cli.js b/bin/cli.js index 0ec2cf11..ec50a94d 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -532,15 +532,22 @@ program // Update Text Properties from Liquid Test data program .command("update-text-properties") - .description("Upload custom text properties from a Liquid Test YAML file to a reconciliation in a company file") + .description( + "Upload custom text properties from a Liquid Test YAML file to a company file. Pushes custom data at the company, period, reconciliation and account levels for every entry referenced in the test scenario (not only the reconciliation in the URL). The URL identifies the target firm/company." + ) .requiredOption("-u, --url ", "Specify the full Silverfin URL of the reconciliation in the company file (mandatory)") .requiredOption("-t, --test ", "Specify the name of the test to use as data source (mandatory)") .option("-h, --handle ", "Specify the reconciliation handle to narrow down the YAML file search (optional)") .option("--dry-run", "Only transform and display the properties without uploading (optional)", false) + .option("-y, --yes", "Skip the confirmation prompt before uploading (optional)", false) .action(async (options) => { // Parse URL to extract IDs const urlData = liquidTestUtils.extractURL(options.url); const { firmId, companyId } = urlData; + if (!firmId || !companyId) { + consola.error("Could not determine the firm and company from the URL. Double-check the Silverfin URL and try again."); + return; + } // Find the test data in the YAML files const testData = textPropertyUtils.findTestData(options.test, options.handle); @@ -555,16 +562,15 @@ program updates.push({ level: "company", properties, apply: () => SF.updateCompanyCustom(firmId, companyId, properties) }); } - // Fetch periods once if needed for resolving period dates + // Fetch all periods once (paginated) if needed for resolving period dates let periodsArray = null; for (const [periodKey, periodEntry] of Object.entries(testData.periods)) { // Resolve period date to period ID if (!periodsArray) { - const periodsResponse = await SF.getPeriods(firmId, companyId); - periodsArray = periodsResponse?.data || []; + periodsArray = await SF.getAllPeriods(firmId, companyId); } - const period = periodsArray.find((p) => p.fiscal_year.end_date === periodKey); + const period = periodsArray.find((p) => p.fiscal_year?.end_date === periodKey); if (!period) { consola.warn(`Period "${periodKey}" not found in company — skipping`); continue; @@ -628,6 +634,16 @@ program return; } + // Summarise what will be written and where, then confirm (unless --yes) + const firmName = firmCredentials.getFirmName(firmId); + consola.warn(`About to write custom data to company ${companyId} on firm ${firmName ? `${firmName} (${firmId})` : firmId}:`); + for (const update of updates) { + consola.warn(` • ${update.level}: ${update.properties.length} properties`); + } + if (!options.yes) { + cliUtils.promptConfirmation(); + } + // Apply all updates let hadFailures = false; for (const update of updates) { diff --git a/lib/api/sfApi.js b/lib/api/sfApi.js index 0ef6d667..068a42a6 100644 --- a/lib/api/sfApi.js +++ b/lib/api/sfApi.js @@ -505,6 +505,34 @@ async function getPeriods(firmId, companyId, page = 1) { } } +async function getAllPeriods(firmId, companyId) { + const fetchPage = async (page) => { + const response = await getPeriods(firmId, companyId, page); + if (!response || !response.data) { + return []; + } + + return response.data || []; + }; + + const items = []; + let page = 1; + let hasMore = true; + + while (hasMore && page <= MAX_PAGES) { + const pageData = await fetchPage(page); + items.push(...pageData); + hasMore = pageData.length === PER_PAGE; + page++; + } + + if (page > MAX_PAGES) { + consola.warn(`Reached maximum page limit (${MAX_PAGES}) while fetching company periods. There might be more data available.`); + } + + return items; +} + function findPeriod(periodId, periodsArray) { return periodsArray.find((period) => period.id == periodId); } @@ -816,6 +844,7 @@ module.exports = { createTestRun, createPreviewRun, getPeriods, + getAllPeriods, findPeriod, getCompanyDrop, getCompanyCustom, diff --git a/lib/utils/textPropertyUtils.js b/lib/utils/textPropertyUtils.js index f408797c..22831d33 100644 --- a/lib/utils/textPropertyUtils.js +++ b/lib/utils/textPropertyUtils.js @@ -70,7 +70,13 @@ function findTestData(testName, handle) { for (const file of yamlFiles) { const filePath = path.join(testsDir, file); const content = fs.readFileSync(filePath, "utf-8"); - const parsed = yaml.parse(content, { maxAliasCount: 10000, merge: true }); + let parsed; + try { + parsed = yaml.parse(content, { maxAliasCount: 10000, merge: true }); + } catch (error) { + consola.warn(`Skipping malformed YAML file "${filePath}": ${error.message}`); + continue; + } if (!parsed || !parsed[testName]) continue; diff --git a/tests/lib/utils/textPropertyUtils.test.js b/tests/lib/utils/textPropertyUtils.test.js new file mode 100644 index 00000000..876a424b --- /dev/null +++ b/tests/lib/utils/textPropertyUtils.test.js @@ -0,0 +1,120 @@ +jest.mock("fs"); +jest.mock("consola"); +jest.mock("yaml"); + +const fs = require("fs"); +const yaml = require("yaml"); +const { consola } = require("consola"); +const { transformCustomToProperties, findTestData } = require("../../../lib/utils/textPropertyUtils"); + +describe("textPropertyUtils", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe("transformCustomToProperties", () => { + it("maps a simple namespace.key to a flat property", () => { + const result = transformCustomToProperties({ "ns.key": "value" }); + expect(result).toEqual([{ namespace: "ns", key: "key", value: "value" }]); + }); + + it("maps a nested namespace.key.subkey to an object value", () => { + const result = transformCustomToProperties({ "ns.key.sub": "value" }); + expect(result).toEqual([{ namespace: "ns", key: "key", value: { sub: "value" } }]); + }); + + it("merges multiple subkeys under the same namespace.key", () => { + const result = transformCustomToProperties({ "ns.key.a": "1", "ns.key.b": "2" }); + expect(result).toEqual([{ namespace: "ns", key: "key", value: { a: "1", b: "2" } }]); + }); + + it("collapses deeper-than-3 segment keys into a single dotted subkey", () => { + // Documented as a 3-segment-max assumption in practice; this locks current behaviour. + const result = transformCustomToProperties({ "ns.key.a.b": "v" }); + expect(result).toEqual([{ namespace: "ns", key: "key", value: { "a.b": "v" } }]); + }); + + it("warns and skips keys without a namespace.key shape", () => { + const result = transformCustomToProperties({ single: "value" }); + expect(result).toEqual([]); + expect(consola.warn).toHaveBeenCalled(); + }); + }); + + describe("findTestData", () => { + const yamlPayload = { + my_test: { + data: { + company: { custom: { "co.k": "cv" } }, + periods: { + "2024-12-31": { + custom: { "p.k": "pv" }, + reconciliations: { my_handle: { custom: { "r.k": "rv" } } }, + accounts: { "610000": { custom: { "a.k": "av" } } }, + }, + }, + }, + }, + }; + + it("extracts custom data at all four levels when scoped by handle", () => { + fs.existsSync.mockReturnValue(true); + fs.readdirSync.mockReturnValue(["my_handle_liquid_test.yml"]); + fs.readFileSync.mockReturnValue("yaml-content"); + yaml.parse.mockReturnValue(yamlPayload); + + const result = findTestData("my_test", "my_handle"); + + expect(result.handle).toBe("my_handle"); + expect(result.file).toBe("my_handle_liquid_test.yml"); + expect(result.company).toEqual({ custom: { "co.k": "cv" } }); + expect(result.periods["2024-12-31"].custom).toEqual({ "p.k": "pv" }); + expect(result.periods["2024-12-31"].reconciliations.my_handle).toEqual({ "r.k": "rv" }); + expect(result.periods["2024-12-31"].accounts["610000"]).toEqual({ "a.k": "av" }); + }); + + it("scans all template folders when no handle is given", () => { + fs.existsSync.mockReturnValue(true); + fs.statSync.mockReturnValue({ isDirectory: () => true }); + fs.readdirSync.mockImplementation((p) => (String(p).endsWith("tests") ? ["my_test_liquid_test.yml"] : ["handle_a"])); + fs.readFileSync.mockReturnValue("yaml-content"); + yaml.parse.mockReturnValue({ my_test: { data: { periods: {} } } }); + + const result = findTestData("my_test"); + + expect(result.handle).toBe("handle_a"); + expect(result.file).toBe("my_test_liquid_test.yml"); + }); + + it("skips a malformed YAML file and continues to the next one", () => { + fs.existsSync.mockReturnValue(true); + fs.readdirSync.mockReturnValue(["bad_liquid_test.yml", "good_liquid_test.yml"]); + fs.readFileSync.mockReturnValue("yaml-content"); + yaml.parse + .mockImplementationOnce(() => { + throw new Error("bad indentation"); + }) + .mockImplementationOnce(() => ({ my_test: { data: { periods: {} } } })); + + const result = findTestData("my_test", "my_handle"); + + expect(consola.warn).toHaveBeenCalledWith(expect.stringContaining("malformed YAML")); + expect(result.file).toBe("good_liquid_test.yml"); + }); + + it("errors and exits when the test name is not found", () => { + fs.existsSync.mockReturnValue(true); + fs.readdirSync.mockReturnValue(["my_handle_liquid_test.yml"]); + fs.readFileSync.mockReturnValue("yaml-content"); + yaml.parse.mockReturnValue({ some_other_test: {} }); + const exitSpy = jest.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + + expect(() => findTestData("missing_test", "my_handle")).toThrow("process.exit"); + expect(consola.error).toHaveBeenCalled(); + + exitSpy.mockRestore(); + }); + }); +}); From aeddf101b52505167340218cef814fc2bbeaa370 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Fri, 3 Jul 2026 09:50:54 +0200 Subject: [PATCH 11/14] Address review: batch alignment, non-aborting batch errors, exit codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - updatePeriodCustom / updateAccountCustom now POST a single bulk { properties: [...] } body, matching updateReconciliationCustom. Verified live: the period/reconciliation/account /custom endpoints accept the bulk shape (201); the company /custom endpoint does NOT (400 'namespace is missing...'), so updateCompanyCustom keeps its per-property loop, with a comment documenting the API difference. - New apiUtils.batchResponseErrorHandler: logs and returns the error response instead of process.exit(1) on 422/403, so one failed write inside update-text-properties is counted via hadFailures and the remaining updates still run (was: the whole CLI aborted mid-batch). - update-text-properties exit codes: an unparseable URL and a period listed in the YAML but absent from the company (incl. getAllPeriods returning []) now set process.exitCode = 1, consistent with the reconciliation/account not-found paths — a partially seeded scenario is detectable by scripts/CI. --- bin/cli.js | 7 ++++++- lib/api/sfApi.js | 46 +++++++++++++++++++------------------------ lib/utils/apiUtils.js | 16 +++++++++++++++ 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index ec50a94d..9215ad06 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -546,6 +546,7 @@ program const { firmId, companyId } = urlData; if (!firmId || !companyId) { consola.error("Could not determine the firm and company from the URL. Double-check the Silverfin URL and try again."); + process.exitCode = 1; return; } @@ -572,7 +573,11 @@ program } const period = periodsArray.find((p) => p.fiscal_year?.end_date === periodKey); if (!period) { - consola.warn(`Period "${periodKey}" not found in company — skipping`); + // Skipping still seeds the rest of the scenario, but the partial seed must + // be detectable: flag it as a failure, consistent with the + // reconciliation/account not-found paths. + consola.error(`Period "${periodKey}" not found in company — skipping (marked as failure)`); + process.exitCode = 1; continue; } const targetPeriodId = period.id; diff --git a/lib/api/sfApi.js b/lib/api/sfApi.js index 068a42a6..eb8a7d52 100644 --- a/lib/api/sfApi.js +++ b/lib/api/sfApi.js @@ -600,6 +600,12 @@ async function getAllPeriodCustom(firmId, companyId, periodId) { return items; } +// The period/reconciliation/account `/custom` endpoints accept a BULK +// `{ properties: [...] }` body (verified live); the company `/custom` endpoint +// does NOT — it returns 400 unless each custom is POSTed as a single +// `{ namespace, key, value }` object, hence the per-property loop there. +// All four use batchResponseErrorHandler so one failed write (e.g. 422) is +// counted as a failure by the caller instead of aborting the whole batch. async function updateReconciliationCustom(firmId, companyId, periodId, reconciliationId, properties) { const instance = AxiosFactory.createInstance("firm", firmId); try { @@ -607,8 +613,7 @@ async function updateReconciliationCustom(firmId, companyId, periodId, reconcili apiUtils.responseSuccessHandler(response); return response; } catch (error) { - const response = await apiUtils.responseErrorHandler(error); - return response; + return apiUtils.batchResponseErrorHandler(error); } } @@ -621,8 +626,7 @@ async function updateCompanyCustom(firmId, companyId, properties) { apiUtils.responseSuccessHandler(response); results.push(response); } catch (error) { - const response = await apiUtils.responseErrorHandler(error); - results.push(response); + results.push(apiUtils.batchResponseErrorHandler(error)); } } return results; @@ -630,34 +634,24 @@ async function updateCompanyCustom(firmId, companyId, properties) { async function updatePeriodCustom(firmId, companyId, periodId, properties) { const instance = AxiosFactory.createInstance("firm", firmId); - const results = []; - for (const prop of properties) { - try { - const response = await instance.post(`/companies/${companyId}/periods/${periodId}/custom`, prop); - apiUtils.responseSuccessHandler(response); - results.push(response); - } catch (error) { - const response = await apiUtils.responseErrorHandler(error); - results.push(response); - } + try { + const response = await instance.post(`/companies/${companyId}/periods/${periodId}/custom`, { properties }); + apiUtils.responseSuccessHandler(response); + return response; + } catch (error) { + return apiUtils.batchResponseErrorHandler(error); } - return results; } async function updateAccountCustom(firmId, companyId, periodId, accountId, properties) { const instance = AxiosFactory.createInstance("firm", firmId); - const results = []; - for (const prop of properties) { - try { - const response = await instance.post(`/companies/${companyId}/periods/${periodId}/accounts/${accountId}/custom`, prop); - apiUtils.responseSuccessHandler(response); - results.push(response); - } catch (error) { - const response = await apiUtils.responseErrorHandler(error); - results.push(response); - } + try { + const response = await instance.post(`/companies/${companyId}/periods/${periodId}/accounts/${accountId}/custom`, { properties }); + apiUtils.responseSuccessHandler(response); + return response; + } catch (error) { + return apiUtils.batchResponseErrorHandler(error); } - return results; } async function getWorkflows(firmId, companyId, periodId) { diff --git a/lib/utils/apiUtils.js b/lib/utils/apiUtils.js index 5069698e..7be87760 100644 --- a/lib/utils/apiUtils.js +++ b/lib/utils/apiUtils.js @@ -59,9 +59,25 @@ async function responseErrorHandler(error) { throw error; } +// Like responseErrorHandler, but NEVER terminates the process — for batch flows +// (e.g. the update*Custom writes behind `update-text-properties`) where a single +// failed write (422/403/...) must not abort the remaining updates. Logs the error +// and returns the error response so callers can count it as a failure. +function batchResponseErrorHandler(error) { + if (error?.response) { + const { status, statusText, data, config } = error.response; + consola.debug(`Response Status: ${status} (${statusText}) - method: ${config?.method} - url: ${config?.url}`); + consola.error(`Response Error (${status}): ${JSON.stringify(data?.error ?? data)}`); + return error.response; + } + // No HTTP response (network-level error): not a per-item failure, so rethrow. + throw error; +} + module.exports = { checkAuthorizePartners, checkRequiredEnvVariables, responseSuccessHandler, responseErrorHandler, + batchResponseErrorHandler, }; From 309830e41c5d5ed025f5ff8303fc6363ac5b5ccb Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 6 Jul 2026 14:35:27 +0200 Subject: [PATCH 12/14] Fix period resolution ambiguity and mid-batch aborts in update-text-properties - Resolve YAML period keys via findPeriodByKey: match period ids (the capture fallback key for periods without a fiscal year), prefer the year-end period when several periods share a fiscal_year.end_date, and fail the entry instead of writing to an arbitrary period when the key stays ambiguous. - Wrap the apply loop in try/catch so a thrown error (network failure, lookup 500) counts as a per-item failure with exit code 1 instead of killing the process mid-batch without a summary. - Guard findReconciliationInWorkflows against an undefined workflows response (handled 404) so it warns 'not found' instead of throwing. --- bin/cli.js | 15 +++++++-- lib/api/sfApi.js | 2 +- lib/utils/textPropertyUtils.js | 28 ++++++++++++++++- tests/lib/utils/textPropertyUtils.test.js | 37 ++++++++++++++++++++++- 4 files changed, 76 insertions(+), 6 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index 9215ad06..dab63603 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -571,12 +571,12 @@ program if (!periodsArray) { periodsArray = await SF.getAllPeriods(firmId, companyId); } - const period = periodsArray.find((p) => p.fiscal_year?.end_date === periodKey); + const { period, error: periodError } = textPropertyUtils.findPeriodByKey(periodsArray, periodKey); if (!period) { // Skipping still seeds the rest of the scenario, but the partial seed must // be detectable: flag it as a failure, consistent with the // reconciliation/account not-found paths. - consola.error(`Period "${periodKey}" not found in company — skipping (marked as failure)`); + consola.error(`${periodError} — skipping (marked as failure)`); process.exitCode = 1; continue; } @@ -653,7 +653,16 @@ program let hadFailures = false; for (const update of updates) { consola.start(`Updating ${update.level} (${update.properties.length} properties)...`); - const response = await update.apply(); + // A throw (network error, unexpected lookup failure) must count as a + // per-item failure, not abort the batch mid-way with no summary. + let response; + try { + response = await update.apply(); + } catch (error) { + hadFailures = true; + consola.error(`${update.level}: failed (${error.message})`); + continue; + } if (!response) { hadFailures = true; continue; diff --git a/lib/api/sfApi.js b/lib/api/sfApi.js index eb8a7d52..b7f37a07 100644 --- a/lib/api/sfApi.js +++ b/lib/api/sfApi.js @@ -698,7 +698,7 @@ async function findReconciliationInWorkflows(firmId, reconciliationHandle, compa // Get data from all workflows const responseWorkflows = await getWorkflows(firmId, companyId, periodId); // Check in each workflow - for (const workflow of responseWorkflows.data) { + for (const workflow of responseWorkflows?.data ?? []) { const reconciliationInformation = await findReconciliationInWorkflow(firmId, reconciliationHandle, companyId, periodId, workflow.id); // Found if (reconciliationInformation) { diff --git a/lib/utils/textPropertyUtils.js b/lib/utils/textPropertyUtils.js index 22831d33..610a2840 100644 --- a/lib/utils/textPropertyUtils.js +++ b/lib/utils/textPropertyUtils.js @@ -40,6 +40,32 @@ function transformCustomToProperties(customData) { return Array.from(namespaceMap.values()); } +/** + * Resolve a YAML period key to a company period. + * Captured YAML keys are the period's fiscal_year.end_date when present, otherwise the + * period id (lib/dataCapture.js, lib/deepCapture.js). Several periods can share the same + * fiscal_year.end_date (e.g. monthly bookkeeping periods within one fiscal year); in that + * case the year-end period (own end_date === key) is the intended target. Refuses to pick + * an arbitrary period when the key stays ambiguous. + * Returns { period } on success, { error } with a reason otherwise. + */ +function findPeriodByKey(periodsArray, periodKey) { + const key = String(periodKey); + + const byId = periodsArray.find((p) => String(p.id) === key); + if (byId) return { period: byId }; + + const byFiscalYear = periodsArray.filter((p) => p.fiscal_year?.end_date === key); + if (byFiscalYear.length === 1) return { period: byFiscalYear[0] }; + if (byFiscalYear.length > 1) { + const yearEnd = byFiscalYear.filter((p) => p.end_date === key); + if (yearEnd.length === 1) return { period: yearEnd[0] }; + return { error: `Period key "${key}" is ambiguous: ${byFiscalYear.length} periods share that fiscal year end date` }; + } + + return { error: `Period "${key}" not found in company` }; +} + /** * Find a test by name across YAML files in a template's tests/ folder. * If handle is provided, only search that handle's folder. @@ -131,4 +157,4 @@ function findTestData(testName, handle) { process.exit(1); } -module.exports = { transformCustomToProperties, findTestData }; +module.exports = { transformCustomToProperties, findTestData, findPeriodByKey }; diff --git a/tests/lib/utils/textPropertyUtils.test.js b/tests/lib/utils/textPropertyUtils.test.js index 876a424b..26f456d2 100644 --- a/tests/lib/utils/textPropertyUtils.test.js +++ b/tests/lib/utils/textPropertyUtils.test.js @@ -5,7 +5,7 @@ jest.mock("yaml"); const fs = require("fs"); const yaml = require("yaml"); const { consola } = require("consola"); -const { transformCustomToProperties, findTestData } = require("../../../lib/utils/textPropertyUtils"); +const { transformCustomToProperties, findTestData, findPeriodByKey } = require("../../../lib/utils/textPropertyUtils"); describe("textPropertyUtils", () => { beforeEach(() => { @@ -117,4 +117,39 @@ describe("textPropertyUtils", () => { exitSpy.mockRestore(); }); }); + + describe("findPeriodByKey", () => { + const yearEnd2024 = { id: 101, end_date: "2024-12-31", fiscal_year: { end_date: "2024-12-31" } }; + const monthly202401 = { id: 102, end_date: "2024-01-31", fiscal_year: { end_date: "2024-12-31" } }; + const monthly202402 = { id: 103, end_date: "2024-02-29", fiscal_year: { end_date: "2024-12-31" } }; + const noFiscalYear = { id: 205, end_date: "2025-03-31", fiscal_year: null }; + + it("resolves a unique fiscal year end date", () => { + const { period, error } = findPeriodByKey([monthly202401, { id: 300, end_date: "2023-12-31", fiscal_year: { end_date: "2023-12-31" } }], "2023-12-31"); + expect(error).toBeUndefined(); + expect(period.id).toBe(300); + }); + + it("prefers the year-end period when several periods share a fiscal year end date", () => { + const { period } = findPeriodByKey([monthly202401, monthly202402, yearEnd2024], "2024-12-31"); + expect(period.id).toBe(101); + }); + + it("resolves an id-based key for periods without a fiscal year", () => { + const { period } = findPeriodByKey([yearEnd2024, noFiscalYear], "205"); + expect(period.id).toBe(205); + }); + + it("returns an error instead of picking an arbitrary period when the key stays ambiguous", () => { + const { period, error } = findPeriodByKey([monthly202401, monthly202402], "2024-12-31"); + expect(period).toBeUndefined(); + expect(error).toContain("ambiguous"); + }); + + it("returns an error when no period matches", () => { + const { period, error } = findPeriodByKey([yearEnd2024], "2030-12-31"); + expect(period).toBeUndefined(); + expect(error).toContain("not found"); + }); + }); }); From 962ae39d813581f29b1815d3dc9fd5e032b1d82c Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 6 Jul 2026 15:14:05 +0200 Subject: [PATCH 13/14] Resolve update-text-properties test file via config.json, add --file override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same test name can exist in several YAML files in one tests/ folder (base + TY24/TY25 year variants) with different periods and data, and findTestData silently used the first file in directory order — seeding data from the wrong variant. findTestData now reads the file referenced by the template config.json "test" key (the same file run-test uses). A new --file option overrides it to select a specific variant. When the test name matches in several templates, the command lists the candidates and asks for --handle instead of guessing. --- bin/cli.js | 3 +- lib/utils/textPropertyUtils.js | 147 +++++++++++++-------- tests/lib/utils/textPropertyUtils.test.js | 149 +++++++++++++++++----- 3 files changed, 214 insertions(+), 85 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index dab63603..298c8e73 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -538,6 +538,7 @@ program .requiredOption("-u, --url ", "Specify the full Silverfin URL of the reconciliation in the company file (mandatory)") .requiredOption("-t, --test ", "Specify the name of the test to use as data source (mandatory)") .option("-h, --handle ", "Specify the reconciliation handle to narrow down the YAML file search (optional)") + .option("--file ", "Specify the exact YAML file name (inside the template's tests/ folder) to read the test from, instead of the test file referenced by the template's config.json (optional)") .option("--dry-run", "Only transform and display the properties without uploading (optional)", false) .option("-y, --yes", "Skip the confirmation prompt before uploading (optional)", false) .action(async (options) => { @@ -551,7 +552,7 @@ program } // Find the test data in the YAML files - const testData = textPropertyUtils.findTestData(options.test, options.handle); + const testData = textPropertyUtils.findTestData(options.test, options.handle, options.file); consola.info(`Found test "${options.test}" in ${testData.handle}/tests/${testData.file}`); // Collect all updates to perform diff --git a/lib/utils/textPropertyUtils.js b/lib/utils/textPropertyUtils.js index 610a2840..3d31646a 100644 --- a/lib/utils/textPropertyUtils.js +++ b/lib/utils/textPropertyUtils.js @@ -67,14 +67,33 @@ function findPeriodByKey(periodsArray, periodKey) { } /** - * Find a test by name across YAML files in a template's tests/ folder. - * If handle is provided, only search that handle's folder. - * If not, scan all reconciliation_texts folders. + * Read the test file path referenced by a template's config.json ("test" key). + * Local read instead of fsUtils.readConfig: that helper creates a missing + * config.json as a side effect and exits the process on parse errors — both + * wrong while scanning across template folders. + */ +function readConfigTestPath(baseDir, dir) { + try { + const config = JSON.parse(fs.readFileSync(path.join(baseDir, dir, "config.json"), "utf-8")); + return config.test || null; + } catch { + return null; + } +} + +/** + * Find a test by name in the templates' liquid test YAML files. + * Per template, only ONE file is considered: the one referenced by the template's + * config.json "test" key (the same file run-test uses), or — when fileName is + * provided — exactly tests/, so year variants (e.g. *_TY25_*.yml) can be + * selected explicitly. If handle is provided, only that template folder is searched. + * When the test name matches in several templates, the match is refused instead of + * guessing: the candidates are listed and --handle is requested. * Extracts custom data from all 4 levels: company, period, reconciliation, account. * Returns { file, handle, company, periods } where periods contains per-period custom, * reconciliation custom, and account custom data. */ -function findTestData(testName, handle) { +function findTestData(testName, handle, fileName) { const templateType = "reconciliationText"; const baseDir = path.join(process.cwd(), fsUtils.FOLDERS[templateType]); @@ -87,74 +106,92 @@ function findTestData(testName, handle) { return fs.statSync(path.join(baseDir, entry)).isDirectory(); }); + const matches = []; for (const dir of handleDirs) { - const testsDir = path.join(baseDir, dir, "tests"); - if (!fs.existsSync(testsDir)) continue; - - const yamlFiles = fs.readdirSync(testsDir).filter((f) => f.endsWith(".yml")); - - for (const file of yamlFiles) { - const filePath = path.join(testsDir, file); - const content = fs.readFileSync(filePath, "utf-8"); - let parsed; - try { - parsed = yaml.parse(content, { maxAliasCount: 10000, merge: true }); - } catch (error) { - consola.warn(`Skipping malformed YAML file "${filePath}": ${error.message}`); - continue; - } + const relativeTestPath = fileName ? path.join("tests", fileName) : readConfigTestPath(baseDir, dir); + if (!relativeTestPath) continue; + + const filePath = path.join(baseDir, dir, relativeTestPath); + if (!fs.existsSync(filePath)) continue; + + const content = fs.readFileSync(filePath, "utf-8"); + let parsed; + try { + parsed = yaml.parse(content, { maxAliasCount: 10000, merge: true }); + } catch (error) { + consola.warn(`Skipping malformed YAML file "${filePath}": ${error.message}`); + continue; + } - if (!parsed || !parsed[testName]) continue; + if (!parsed || !parsed[testName]) continue; - const testData = parsed[testName]; - const result = { file, handle: dir, company: null, periods: {} }; + matches.push({ file: path.basename(relativeTestPath), handle: dir, testData: parsed[testName] }); + } - // Company-level custom - if (testData?.data?.company?.custom) { - result.company = { custom: testData.data.company.custom }; - } + if (matches.length === 0) { + if (fileName) { + consola.error(`Test "${testName}" not found in any YAML file named "${fileName}"`); + } else { + consola.error( + `Test "${testName}" not found in any test file referenced by a template's config.json. Use --file to read a specific YAML file instead (e.g. a year variant not referenced in config.json).` + ); + } + process.exit(1); + } - // Period-level data - const periods = testData?.data?.periods; - if (periods) { - for (const [periodKey, periodData] of Object.entries(periods)) { - if (!periodData) continue; + if (matches.length > 1) { + consola.error(`Test "${testName}" found in multiple templates:`); + for (const match of matches) { + consola.error(` - ${match.handle}/tests/${match.file}`); + } + consola.error("Re-run with --handle to select one."); + process.exit(1); + } - const periodEntry = { custom: null, reconciliations: {}, accounts: {} }; + const { file, handle: dir, testData } = matches[0]; + const result = { file, handle: dir, company: null, periods: {} }; - // Period-level custom - if (periodData.custom) { - periodEntry.custom = periodData.custom; - } + // Company-level custom + if (testData?.data?.company?.custom) { + result.company = { custom: testData.data.company.custom }; + } - // Reconciliation-level custom - if (periodData.reconciliations) { - for (const [reconHandle, reconData] of Object.entries(periodData.reconciliations)) { - if (reconData?.custom) { - periodEntry.reconciliations[reconHandle] = reconData.custom; - } - } - } + // Period-level data + const periods = testData?.data?.periods; + if (periods) { + for (const [periodKey, periodData] of Object.entries(periods)) { + if (!periodData) continue; - // Account-level custom - if (periodData.accounts) { - for (const [accountNumber, accountData] of Object.entries(periodData.accounts)) { - if (accountData?.custom) { - periodEntry.accounts[accountNumber] = accountData.custom; - } - } + const periodEntry = { custom: null, reconciliations: {}, accounts: {} }; + + // Period-level custom + if (periodData.custom) { + periodEntry.custom = periodData.custom; + } + + // Reconciliation-level custom + if (periodData.reconciliations) { + for (const [reconHandle, reconData] of Object.entries(periodData.reconciliations)) { + if (reconData?.custom) { + periodEntry.reconciliations[reconHandle] = reconData.custom; } + } + } - result.periods[periodKey] = periodEntry; + // Account-level custom + if (periodData.accounts) { + for (const [accountNumber, accountData] of Object.entries(periodData.accounts)) { + if (accountData?.custom) { + periodEntry.accounts[accountNumber] = accountData.custom; + } } } - return result; + result.periods[periodKey] = periodEntry; } } - consola.error(`Test "${testName}" not found in any YAML file`); - process.exit(1); + return result; } module.exports = { transformCustomToProperties, findTestData, findPeriodByKey }; diff --git a/tests/lib/utils/textPropertyUtils.test.js b/tests/lib/utils/textPropertyUtils.test.js index 26f456d2..16c286ce 100644 --- a/tests/lib/utils/textPropertyUtils.test.js +++ b/tests/lib/utils/textPropertyUtils.test.js @@ -57,11 +57,35 @@ describe("textPropertyUtils", () => { }, }; - it("extracts custom data at all four levels when scoped by handle", () => { - fs.existsSync.mockReturnValue(true); - fs.readdirSync.mockReturnValue(["my_handle_liquid_test.yml"]); - fs.readFileSync.mockReturnValue("yaml-content"); - yaml.parse.mockReturnValue(yamlPayload); + // Simulate a repo layout: each handle dir has a config.json whose "test" key + // points at the canonical YAML file, mirroring how run-test resolves it. + const mockRepo = (configs, yamlContents) => { + fs.existsSync.mockImplementation((p) => { + const file = String(p); + if (file.endsWith("config.json")) return true; + if (file.endsWith(".yml")) return Object.keys(yamlContents).some((name) => file.endsWith(name)); + return true; // base dir and everything else + }); + fs.statSync.mockReturnValue({ isDirectory: () => true }); + fs.readdirSync.mockReturnValue(Object.keys(configs)); + fs.readFileSync.mockImplementation((p) => { + const file = String(p); + if (file.endsWith("config.json")) { + const dir = Object.keys(configs).find((d) => file.includes(`/${d}/`)); + return JSON.stringify(configs[dir] ?? {}); + } + return "yaml-content:" + file; + }); + yaml.parse.mockImplementation((content) => { + const name = Object.keys(yamlContents).find((n) => String(content).endsWith(n)); + const payload = yamlContents[name]; + if (payload instanceof Error) throw payload; + return payload; + }); + }; + + it("extracts custom data at all four levels from the config-referenced file when scoped by handle", () => { + mockRepo({ my_handle: { test: "tests/my_handle_liquid_test.yml" } }, { "my_handle_liquid_test.yml": yamlPayload }); const result = findTestData("my_test", "my_handle"); @@ -73,46 +97,113 @@ describe("textPropertyUtils", () => { expect(result.periods["2024-12-31"].accounts["610000"]).toEqual({ "a.k": "av" }); }); - it("scans all template folders when no handle is given", () => { - fs.existsSync.mockReturnValue(true); - fs.statSync.mockReturnValue({ isDirectory: () => true }); - fs.readdirSync.mockImplementation((p) => (String(p).endsWith("tests") ? ["my_test_liquid_test.yml"] : ["handle_a"])); - fs.readFileSync.mockReturnValue("yaml-content"); - yaml.parse.mockReturnValue({ my_test: { data: { periods: {} } } }); + it("scans all templates' config-referenced test files when no handle is given", () => { + mockRepo( + { + handle_a: { test: "tests/handle_a_liquid_test.yml" }, + handle_b: { test: "tests/handle_b_liquid_test.yml" }, + }, + { + "handle_a_liquid_test.yml": { some_other_test: {} }, + "handle_b_liquid_test.yml": { my_test: { data: { periods: {} } } }, + } + ); const result = findTestData("my_test"); - expect(result.handle).toBe("handle_a"); - expect(result.file).toBe("my_test_liquid_test.yml"); + expect(result.handle).toBe("handle_b"); + expect(result.file).toBe("handle_b_liquid_test.yml"); }); - it("skips a malformed YAML file and continues to the next one", () => { - fs.existsSync.mockReturnValue(true); - fs.readdirSync.mockReturnValue(["bad_liquid_test.yml", "good_liquid_test.yml"]); - fs.readFileSync.mockReturnValue("yaml-content"); - yaml.parse - .mockImplementationOnce(() => { - throw new Error("bad indentation"); - }) - .mockImplementationOnce(() => ({ my_test: { data: { periods: {} } } })); + it("skips a template with a malformed YAML file and continues with the others", () => { + mockRepo( + { + handle_a: { test: "tests/bad_liquid_test.yml" }, + handle_b: { test: "tests/good_liquid_test.yml" }, + }, + { + "bad_liquid_test.yml": new Error("bad indentation"), + "good_liquid_test.yml": { my_test: { data: { periods: {} } } }, + } + ); - const result = findTestData("my_test", "my_handle"); + const result = findTestData("my_test"); expect(consola.warn).toHaveBeenCalledWith(expect.stringContaining("malformed YAML")); expect(result.file).toBe("good_liquid_test.yml"); }); - it("errors and exits when the test name is not found", () => { - fs.existsSync.mockReturnValue(true); - fs.readdirSync.mockReturnValue(["my_handle_liquid_test.yml"]); - fs.readFileSync.mockReturnValue("yaml-content"); - yaml.parse.mockReturnValue({ some_other_test: {} }); + it("skips templates whose config.json has no test key", () => { + mockRepo( + { + handle_a: {}, + handle_b: { test: "tests/handle_b_liquid_test.yml" }, + }, + { "handle_b_liquid_test.yml": { my_test: { data: { periods: {} } } } } + ); + + const result = findTestData("my_test"); + + expect(result.handle).toBe("handle_b"); + }); + + it("errors and exits when the test name is not found, pointing at --file", () => { + mockRepo({ my_handle: { test: "tests/my_handle_liquid_test.yml" } }, { "my_handle_liquid_test.yml": { some_other_test: {} } }); const exitSpy = jest.spyOn(process, "exit").mockImplementation(() => { throw new Error("process.exit"); }); expect(() => findTestData("missing_test", "my_handle")).toThrow("process.exit"); - expect(consola.error).toHaveBeenCalled(); + expect(consola.error).toHaveBeenCalledWith(expect.stringContaining("--file")); + + exitSpy.mockRestore(); + }); + + it("refuses to guess when the test name exists in multiple templates", () => { + const payload = { my_test: { data: { periods: {} } } }; + mockRepo( + { + handle_a: { test: "tests/handle_a_liquid_test.yml" }, + handle_b: { test: "tests/handle_b_liquid_test.yml" }, + }, + { "handle_a_liquid_test.yml": payload, "handle_b_liquid_test.yml": payload } + ); + const exitSpy = jest.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + + expect(() => findTestData("my_test")).toThrow("process.exit"); + expect(consola.error).toHaveBeenCalledWith(expect.stringContaining("multiple templates")); + expect(consola.error).toHaveBeenCalledWith(expect.stringContaining("handle_a/tests/handle_a_liquid_test.yml")); + expect(consola.error).toHaveBeenCalledWith(expect.stringContaining("--handle")); + + exitSpy.mockRestore(); + }); + + it("reads the exact tests/ when fileName is given, overriding config.json", () => { + mockRepo( + { my_handle: { test: "tests/my_handle_liquid_test.yml" } }, + { + "my_handle_liquid_test.yml": { my_test: { data: { periods: { "2023-12-31": {} } } } }, + "my_handle_TY25_liquid_test.yml": { my_test: { data: { periods: { "2024-12-31": {} } } } }, + } + ); + + const result = findTestData("my_test", "my_handle", "my_handle_TY25_liquid_test.yml"); + + expect(result.file).toBe("my_handle_TY25_liquid_test.yml"); + expect(Object.keys(result.periods)).toEqual(["2024-12-31"]); + expect(yaml.parse).toHaveBeenCalledTimes(1); + }); + + it("errors when fileName matches no file containing the test", () => { + mockRepo({ my_handle: { test: "tests/my_handle_liquid_test.yml" } }, { "my_handle_liquid_test.yml": { my_test: {} } }); + const exitSpy = jest.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + + expect(() => findTestData("my_test", "my_handle", "does_not_exist.yml")).toThrow("process.exit"); + expect(consola.error).toHaveBeenCalledWith(expect.stringContaining('named "does_not_exist.yml"')); exitSpy.mockRestore(); }); From 374155f8ad6330de5850d730bce6a4c8434a8f2f Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Mon, 6 Jul 2026 15:14:27 +0200 Subject: [PATCH 14/14] Document config.json test file resolution and --file in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f70dccd..e16a77e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ All notable changes to this project will be documented in this file. ## [1.56.4] (06/07/2026) -Added `update-text-properties` command. It uploads custom text properties from a Liquid Test YAML file to a company file at company, period, reconciliation and account levels for the entries referenced in the test scenario. Usage: `silverfin update-text-properties -u -t `. Supports `--handle` for faster YAML file lookup, `--dry-run` to preview the payload, and `--yes` to skip the confirmation prompt. +Added `update-text-properties` command. It uploads custom text properties from a Liquid Test YAML file to a company file at company, period, reconciliation and account levels for the entries referenced in the test scenario. Usage: `silverfin update-text-properties -u -t `. The YAML file read is the one referenced by the template's `config.json` `test` key (same as `run-test`); pass `--file ` to read another file, e.g. a year variant like `*_TY25_liquid_test.yml`. When the test name exists in several templates the command lists them and asks for `--handle` instead of guessing. Also supports `--dry-run` to preview the payload and `--yes` to skip the confirmation prompt. ## [1.56.3] (03/07/2026) Improve `run-test --status` output for CI: surface the underlying error message when a run ends in `test_error`/`internal_error` (previously reported as a bare `FAILED` with no reason), and suppress the progress spinner when stdout is not a TTY (it flooded CI logs with hundreds of "Running tests.." frames).