Skip to content
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

Only pull single account data from SimpleFIN if syncing one account #483

Merged
merged 7 commits into from
Oct 30, 2024
Merged

Only pull single account data from SimpleFIN if syncing one account #483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9c910da
only pull single account data from SimpleFIN if syncing one account
psybers Oct 22, 2024
11e650d
add release note
psybers Oct 22, 2024
a9cab1d
fix linter
psybers Oct 22, 2024
7e4b273
remove debug print
psybers Oct 22, 2024
b95be31
Update src/app-simplefin/app-simplefin.js
psybers Oct 22, 2024
e89d9c7
Merge branch 'master' into sf-single-account
psybers Oct 23, 2024
913eb67
Merge branch 'master' into sf-single-account
psybers Oct 23, 2024
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
19 changes: 15 additions & 4 deletions src/app-simplefin/app-simplefin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ app.post(
}

try {
const accounts = await getAccounts(accessKey, null, null, true);
const accounts = await getAccounts(accessKey, null, null, null, true);

res.send({
status: 'ok',
Expand Down Expand Up @@ -91,7 +91,11 @@ app.post(
: startDate;
let results;
try {
results = await getTransactions(accessKey, new Date(earliestStartDate));
results = await getTransactions(
accessKey,
Array.isArray(accountId) ? accountId : [accountId],
new Date(earliestStartDate),
);
} catch (e) {
if (e.message === 'Forbidden') {
invalidToken(res);
Expand Down Expand Up @@ -293,12 +297,12 @@ async function getAccessKey(base64Token) {
});
}

async function getTransactions(accessKey, startDate, endDate) {
async function getTransactions(accessKey, accounts, startDate, endDate) {
const now = new Date();
startDate = startDate || new Date(now.getFullYear(), now.getMonth(), 1);
endDate = endDate || new Date(now.getFullYear(), now.getMonth() + 1, 1);
console.log(`${getDate(startDate)} - ${getDate(endDate)}`);
return await getAccounts(accessKey, startDate, endDate);
return await getAccounts(accessKey, accounts, startDate, endDate);
Comment on lines +300 to +305
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add input validation for accounts parameter.

The function should validate that the accounts parameter is either null or an array of non-empty strings to prevent potential API errors.

 async function getTransactions(accessKey, accounts, startDate, endDate) {
+  if (accounts !== null && (!Array.isArray(accounts) || accounts.some(id => !id || typeof id !== 'string'))) {
+    throw new Error('accounts parameter must be null or an array of non-empty strings');
+  }
   const now = new Date();
   startDate = startDate || new Date(now.getFullYear(), now.getMonth(), 1);
   endDate = endDate || new Date(now.getFullYear(), now.getMonth() + 1, 1);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function getTransactions(accessKey, accounts, startDate, endDate) {
const now = new Date();
startDate = startDate || new Date(now.getFullYear(), now.getMonth(), 1);
endDate = endDate || new Date(now.getFullYear(), now.getMonth() + 1, 1);
console.log(`${getDate(startDate)} - ${getDate(endDate)}`);
return await getAccounts(accessKey, startDate, endDate);
return await getAccounts(accessKey, accounts, startDate, endDate);
async function getTransactions(accessKey, accounts, startDate, endDate) {
if (accounts !== null && (!Array.isArray(accounts) || accounts.some(id => !id || typeof id !== 'string'))) {
throw new Error('accounts parameter must be null or an array of non-empty strings');
}
const now = new Date();
startDate = startDate || new Date(now.getFullYear(), now.getMonth(), 1);
endDate = endDate || new Date(now.getFullYear(), now.getMonth() + 1, 1);
console.log(`${getDate(startDate)} - ${getDate(endDate)}`);
return await getAccounts(accessKey, accounts, startDate, endDate);

}

function getDate(date) {
Expand All @@ -311,6 +315,7 @@ function normalizeDate(date) {

async function getAccounts(
accessKey,
accounts,
startDate,
endDate,
noTransactions = false,
matt-fidd marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -337,6 +342,12 @@ async function getAccounts(
params.push(`balances-only=1`);
}

if (accounts) {
accounts.forEach((id) => {
params.push(`account=${encodeURIComponent(id)}`);
});
}

let queryString = '';
if (params.length > 0) {
queryString += '?' + params.join('&');
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/483.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Enhancements
authors: [psybers]
---

SimpleFIN: when sync'ing a single account, only pull transactions for that account.