Skip to content

Create tests for account templates #234

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.

Sign up for GitHub

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

Open
michieldegezelle wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Create tests for account templates #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6569561
add more tests for AT's (WIP)
delphine-demeulenaere Apr 8, 2025
817eb57
Add update to changelog and bump version
michieldegezelle Jul 28, 2025
8151798
Activate create-test command for account templates
michieldegezelle Dec 17, 2025
bb835b7
Fix tests for AT
michieldegezelle Dec 17, 2025
5270726
Add tests for account templates liquid tests
michieldegezelle Dec 17, 2025
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@ For example: `silverfin update-reconciliation --id 12345`
- Add tests for the exportFile class

## [1.38.0] (04/07/2025)
- Added a changelog.md file and logic to display the changes when updating to latest version
- Added a changelog.md file and logic to display the changes when updating to latest version
Copy link

@coderabbitai coderabbitai bot Dec 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing changelog entry for the new account template test creation feature.

This PR introduces significant new functionality (create-test support for account templates), but the CHANGELOG does not document this change. Consider adding an entry similar to:

+## [1.49.0] (17/12/2025)
+- Added `create-test` command support for account templates (fetches template data, period data, and custom data)
+
 ## [1.48.0] (25/09/2025)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In CHANGELOG.md around line 54, the changelog is missing an entry for the new
"create-test" support for account templates; add a new changelog entry under the
appropriate unreleased or version section with the release/version header and
date (if applicable) and a concise bullet summarizing the feature (e.g., "Added
create-test support for account templates — enables creating test instances from
account templates via X API/CLI"), include PR number and author/maintainer
initials if available, and place it in the correct section (Added/Changed) so
the change is recorded for release notes.

54 changes: 54 additions & 0 deletions lib/api/sfApi.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,30 @@ async function removeSharedPartFromAccountTemplate(type, envId, sharedPartId, ac
}
}

async function getAccountTemplateCustom(type, envId, companyId, periodId, accountTemplateId, page = 1) {
const instance = AxiosFactory.createInstance(type, envId);
try {
const response = await instance.get(`/companies/${companyId}/periods/${periodId}/accounts/${accountTemplateId}/custom`, { params: { page: page, per_page: 200 } });
apiUtils.responseSuccessHandler(response);
return response;
} catch (error) {
const response = await apiUtils.responseErrorHandler(error);
return response;
}
}

async function getAccountTemplateResults(type, envId, companyId, periodId, accountTemplateId) {
const instance = AxiosFactory.createInstance(type, envId);
try {
const response = await instance.get(`/companies/${companyId}/periods/${periodId}/accounts/${accountTemplateId}/results`);
apiUtils.responseSuccessHandler(response);
return response;
} catch (error) {
const response = await apiUtils.responseErrorHandler(error);
return response;
}
}

async function createTestRun(firmId, attributes, templateType) {
const instance = AxiosFactory.createInstance("firm", firmId);
let response;
Expand Down Expand Up @@ -615,6 +639,33 @@ async function getAccountDetails(firmId, companyId, periodId, accountId) {
}
}

async function findAccountByNumber(firmId, companyId, periodId, accountNumber, page = 1) {
const instance = AxiosFactory.createInstance("firm", firmId);
try {
const response = await instance.get(`companies/${companyId}/periods/${periodId}/accounts`, {
params: { page: page },
});

const accounts = response.data;

// No data - end of pagination
if (accounts.length === 0) {
return null;
}

// Look for the account in this page
const account = accounts.find((acc) => acc.account.number === accountNumber);
if (account) {
return account;
}

// Not found in this page, try next page
return findAccountByNumber(firmId, companyId, periodId, accountNumber, page + 1);
} catch (error) {
apiUtils.responseErrorHandler(error);
}
}
Comment on lines +642 to +667
Copy link

@coderabbitai coderabbitai bot Dec 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing return statement in error handler will silently swallow errors.

The catch block calls apiUtils.responseErrorHandler(error) but doesn't return or rethrow, causing the function to implicitly return undefined. This masks errors and makes debugging difficult.

   } catch (error) {
-    apiUtils.responseErrorHandler(error);
+    const response = await apiUtils.responseErrorHandler(error);
+    return response;
   }
 }

Additionally, consider adding a MAX_PAGES limit similar to getAllPeriodCustom to prevent unbounded recursion if the API returns unexpected data.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In lib/api/sfApi.js around lines 642 to 667, the catch block calls
apiUtils.responseErrorHandler(error) but neither returns its result nor
rethrows, causing the function to implicitly return undefined and swallow
errors; update the catch to rethrow or return the handled error (e.g., return
apiUtils.responseErrorHandler(error) or throw the error after handling) so
callers get an explicit failure, and add a MAX_PAGES limit (mirror
getAllPeriodCustom) by introducing a constant and checking the page parameter to
stop recursion and return null or throw a clear error when the limit is exceeded
to avoid unbounded recursion.


// Liquid Linter
// attributes should be JSON
async function verifyLiquid(firmId, attributes) {
Expand Down Expand Up @@ -674,6 +725,8 @@ module.exports = {
findAccountTemplateByName,
addSharedPartToAccountTemplate,
removeSharedPartFromAccountTemplate,
getAccountTemplateCustom,
getAccountTemplateResults,
readTestRun,
createTestRun,
createPreviewRun,
Expand All @@ -688,6 +741,7 @@ module.exports = {
findReconciliationInWorkflow,
findReconciliationInWorkflows,
getAccountDetails,
findAccountByNumber,
verifyLiquid,
getFirmDetails,
};
Loading
Loading