-
Notifications
You must be signed in to change notification settings - Fork 2
Implement reverse test #249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 13 commits
4c3a0e5
0c5811d
d48e328
6fe5858
b3b059e
40984d8
fd48970
bfc7a5c
06d247e
ec00fec
41cc5cf
1e85a88
37a92be
aeddf10
2fe2422
309830e
962ae39
374155f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,4 +4,4 @@ node_modules | |
| ./tmp | ||
| .cursor | ||
| /tmp | ||
| /tmp-* | ||
| /tmp-* | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,145 @@ 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 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 <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) | ||
| .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; | ||
|
Benjvandam marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // 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 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) { | ||
| periodsArray = await SF.getAllPeriods(firmId, companyId); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 Major: if (!periodsArray) {
try {
periodsArray = await SF.getAllPeriods(firmId, companyId);
} catch (error) {
consola.error(`Could not fetch periods: ${error.message}`);
process.exitCode = 1;
return;
}
} |
||
| } | ||
| const period = periodsArray.find((p) => p.fiscal_year?.end_date === periodKey); | ||
| if (!period) { | ||
| consola.warn(`Period "${periodKey}" not found in company — skipping`); | ||
|
Benjvandam marked this conversation as resolved.
Outdated
|
||
| 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; | ||
| } | ||
|
|
||
| // 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) { | ||
| consola.start(`Updating ${update.level} (${update.properties.length} properties)...`); | ||
| const response = await update.apply(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 Major: Concretely,
Any of these mid-batch turns what should be one failed Suggested fix — wrap the resolve+apply step so lookup failures are treated the same as write failures: for (const update of updates) {
consola.start(`Updating ${update.level} (${update.properties.length} properties)...`);
let response;
try {
response = await update.apply();
} catch (error) {
hadFailures = true;
consola.error(`${update.level}: failed to resolve/update — ${error.message}`);
continue;
}
if (!response) {
hadFailures = true;
continue;
}
...
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — the apply loop now wraps
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stale: This was fixed. |
||
| 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`); | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| if (hadFailures) { | ||
| process.exitCode = 1; | ||
| } | ||
| }); | ||
|
|
||
| // Check Liquid Test dependencies for a reconciliation template | ||
| program | ||
| .command("check-dependencies") | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.