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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
node_modules
.env
.DS_Store
./tmp
./tmp
.cursor
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <url> -t <test-name>`. 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).

Expand Down
125 changes: 125 additions & 0 deletions bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -527,6 +529,129 @@ 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 <url>", "Specify the full Silverfin URL of the reconciliation in the company file (mandatory)")
.requiredOption("-t, --test <test-name>", "Specify the name of the test to use as data source (mandatory)")
.option("-h, --handle <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 } = urlData;
Comment on lines +541 to +543
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Scope period/reconciliation/account updates to the URL target.

Lines 542-543 parse a specific reconciliation URL, but Line 567 switches to matching by fiscal_year.end_date and the loops below enqueue every period/reconciliation/account block found in the YAML. That turns a targeted command into a multi-record writer and can overwrite unrelated remote custom data from the same test file. Use the URL’s period/reconciliation identity to filter the update set before building updates.

Also applies to: 561-613

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bin/cli.js` around lines 541 - 543, The code parses the target URL into
urlData via liquidTestUtils.extractURL (producing firmId, companyId and the
target period/reconciliation/account identity), but the update-building logic
(the loops that assemble the updates array and the fiscal_year.end_date matching
logic) currently selects records by fiscal_year.end_date and enqueues every
block found; change that logic to filter using the identity fields extracted
from options.url/urlData (e.g., periodId, reconciliationId, accountId or
equivalent) so only records whose period/reconciliation/account IDs match
urlData are included when constructing updates, and ensure subsequent
update/enqueue code uses firmId/companyId from urlData for scoping rather than
broadly matching fiscal_year.end_date.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the command is to set up a full test scenario in a company file — that requires pushing custom data to all levels and templates referenced in the test, not just the single reconciliation from the URL. The URL is used to identify the target company/firm/period, but the test YAML defines the complete data set needed for that scenario to work.


// Find the test data in the YAML files
const testData = textPropertyUtils.findTestData(options.test, options.handle);
consola.info(`Found test "${options.test}" in ${testData.handle}/tests/${testData.file}`);

// 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);
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, accountId, 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) {
for (const update of updates) {
consola.info(`[dry-run] ${update.level}: ${update.properties.length} properties`);
consola.log(JSON.stringify(update.properties, null, 2));
}
return;
}

// 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) {
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
program
.command("check-dependencies")
Expand Down
64 changes: 64 additions & 0 deletions lib/api/sfApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,66 @@ 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 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 {
Expand Down Expand Up @@ -761,6 +821,10 @@ module.exports = {
getCompanyCustom,
getPeriodCustom,
getAllPeriodCustom,
updateReconciliationCustom,
updateCompanyCustom,
updatePeriodCustom,
updateAccountCustom,
getWorkflows,
getWorkflowInformation,
findReconciliationInWorkflow,
Expand Down
128 changes: 128 additions & 0 deletions lib/utils/textPropertyUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
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;
Comment on lines +31 to +36
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Build real nested objects for deep dotted keys.

Lines 35-36 collapse the remaining path into a single "a.b" key, so ns.key.a.b is serialized differently from ns.key.a. Any property deeper than three segments will be uploaded in the wrong shape.

🐛 Possible fix
     } else {
       if (!namespaceMap.has(namespaceKey)) {
         namespaceMap.set(namespaceKey, { namespace, key, value: {} });
       }
-      const subKey = keyParts.slice(2).join(".");
-      namespaceMap.get(namespaceKey).value[subKey] = value;
+      let current = namespaceMap.get(namespaceKey).value;
+      for (const part of keyParts.slice(2, -1)) {
+        if (typeof current[part] !== "object" || current[part] === null) {
+          current[part] = {};
+        }
+        current = current[part];
+      }
+      current[keyParts[keyParts.length - 1]] = value;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/utils/textPropertyUtils.js` around lines 31 - 36, The code currently
collapses deep dotted keys into a single string key by using subKey =
keyParts.slice(2).join("."), causing e.g. ns.key.a.b to become { "a.b": value }
instead of a nested object; update the logic where namespaceMap is populated
(look for namespaceMap, namespaceKey, keyParts, subKey) to build real nested
objects for keyParts.slice(2) by iterating the remaining segments and creating
nested plain objects until the final segment where you assign value (instead of
joining with "."); ensure namespaceMap.get(namespaceKey).value is mutated to
contain the proper nested structure rather than a single dotted-key entry.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 nesting levels doesn't happen in practice

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Benjvandam, understood! Thanks for the context. I'll note that for future reviews.


✏️ Learnings added
Learnt from: Benjvandam
Repo: silverfin/silverfin-cli PR: 249
File: lib/utils/textPropertyUtils.js:31-36
Timestamp: 2026-03-23T15:03:32.071Z
Learning: In `lib/utils/textPropertyUtils.js` (silverfin-cli), custom property keys in the Liquid Test YAML files follow at most a 3-segment dot-notation format (`namespace.key.subkey`). Keys with 4 or more segments (e.g., `namespace.key.a.b`) do not occur in practice, so collapsing deeper segments with `keyParts.slice(2).join(".")` is intentional and sufficient.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

}
Comment on lines +27 to +37
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential data inconsistency when mixing simple and nested keys for the same namespace.key.

If the input contains both a simple key (e.g., "ns.key": "value") and nested keys (e.g., "ns.key.subkey": "nested"), the order of iteration determines the result:

  • Simple key first → the nested key will overwrite .value with an object containing subkey
  • Nested key first → the simple key is ignored (line 28 check !namespaceMap.has)

This could lead to unexpected behavior depending on object property iteration order.

🛡️ Proposed fix to handle mixed keys consistently
     if (keyParts.length === 2) {
-      if (!namespaceMap.has(namespaceKey)) {
-        namespaceMap.set(namespaceKey, { namespace, key, value });
-      }
+      // Simple key always sets the value directly (overwrite if exists)
+      const existing = namespaceMap.get(namespaceKey);
+      if (existing && typeof existing.value === "object") {
+        consola.warn(`Key "${fullKey}" conflicts with nested keys under same namespace.key — skipping`);
+        continue;
+      }
+      namespaceMap.set(namespaceKey, { namespace, key, value });
     } else {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/utils/textPropertyUtils.js` around lines 27 - 37, The code currently has
inconsistent behavior when mixing simple keys ("ns.key") and nested keys
("ns.key.sub") because namespaceMap entries are set once and later keys either
get ignored or overwrite the value; update the logic in the block that handles
keyParts and namespaceMap so conflicts are merged deterministically: whenever
adding a nested key (keyParts.length > 2) and namespaceMap already has a
primitive value for namespaceKey, convert that entry's value into an object and
store the original primitive under a reserved property (e.g., "" or "_value");
conversely, when adding a simple key and an entry exists whose value is an
object, store the simple value under that reserved property instead of replacing
the object; use the existing symbols namespaceMap, namespaceKey, keyParts,
subKey and ensure you consistently create/merge objects rather than overwriting
or ignoring entries.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't happen in the YAML files in practice

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Benjvandam, understood! Since the YAML files in practice won't mix simple keys (e.g., "ns.key": "value") and nested keys (e.g., "ns.key.subkey": "nested") for the same namespace.key, the current behavior is safe. I'll note this for future reviews.


✏️ Learnings added
Learnt from: Benjvandam
Repo: silverfin/silverfin-cli PR: 249
File: lib/utils/textPropertyUtils.js:27-37
Timestamp: 2026-03-23T15:05:12.258Z
Learning: In `lib/utils/textPropertyUtils.js` (silverfin-cli), custom property keys in Liquid Test YAML files never mix simple keys (`namespace.key`) and nested keys (`namespace.key.subkey`) for the same `namespace.key`. Therefore, the existing logic in `transformCustomToProperties` that ignores duplicate `namespaceKey` entries is intentional and safe — no conflict-resolution handling is needed.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: Benjvandam
Repo: silverfin/silverfin-cli PR: 249
File: lib/utils/textPropertyUtils.js:31-36
Timestamp: 2026-03-23T15:03:32.071Z
Learning: In `lib/utils/textPropertyUtils.js` (silverfin-cli), custom property keys in the Liquid Test YAML files follow at most a 3-segment dot-notation format (`namespace.key.subkey`). Keys with 4 or more segments (e.g., `namespace.key.a.b`) do not occur in practice, so collapsing deeper segments with `keyParts.slice(2).join(".")` is intentional and sufficient.

}

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.
* 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) {
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();
});

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, merge: true });

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) {
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;
}

// Reconciliation-level custom
if (periodData.reconciliations) {
for (const [reconHandle, reconData] of Object.entries(periodData.reconciliations)) {
if (reconData?.custom) {
periodEntry.reconciliations[reconHandle] = reconData.custom;
}
}
}

// Account-level custom
if (periodData.accounts) {
for (const [accountNumber, accountData] of Object.entries(periodData.accounts)) {
if (accountData?.custom) {
periodEntry.accounts[accountNumber] = accountData.custom;
}
}
}

result.periods[periodKey] = periodEntry;
}
}

return result;
}
}

consola.error(`Test "${testName}" not found in any YAML file`);
process.exit(1);
}

module.exports = { transformCustomToProperties, findTestData };
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
Loading