-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Updates to the @actual-budget/api package #464
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Definitely along the lines I was thinking. let me know when this is ready for a final review and I'll approve if it looks good. (see my main question about downloadBudget
)
@@ -160,6 +161,35 @@ handlers['api/load-budget'] = async function ({ id }) { | |||
} | |||
}; | |||
|
|||
handlers['api/download-budget'] = async function ({ syncId, password }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this function we want to check if the file is already downloaded, and if so do nothing right?
Or change the API the be api.loadBudget
and it would magically automatically download it if it's not already downloaded. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this function we want to check if the file is already downloaded, and if so do nothing right?
I assumed cloudStorage.download
already did that because download-budget
checks for the file-exists
error, but it seems like it doesn’t. I’ll add that!
Or change the API the be
api.loadBudget
and it would magically automatically download it if it's not already downloaded. What do you think?
Currently you have to use the sync id to download, and the budget id to open. (The budget id is helpfully returned from the handlers['download-budget']
API, once the download completes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Fortunately, get-budgets
returns sync file IDs for local budgets, so I’m able to fetch them that way).
Updated the code to look for a local budget and trigger a full sync on it instead of re-downloading if possible. I think this is ready for another review!
Also new docs can be previewed here: https://deploy-preview-76--zingy-bonbon-b1b286.netlify.app/Developers/using-the-API |
@jlongster Would you be able to take a look at this? |
Hi @j-f1, |
* Make it easier to build the bundle.api.js for the API * Remove budgetId parameter, move config to top level of API * that’s a breaking change * Add support for signing into the server in init() * Add api.downloadBudget(syncId, { password }) method * Fix lint errors * Refactor: extract out getSyncError * api/download-budget: sync if possible instead of downloading * Don’t bother with fetching remote files and installing key if the file is local * *groupId * FIx lint issues * Remove extra close+reopen * Refactor out duplicate load-budget logic * Trailing commas
While working on actualbudget/docs#76, I made some improvements to make the API easier to use.
@jlongster if possible I would love a review of these changes, especially around handling keys (in
packages/loot-core/src/server/api.js
). I want to make sure I’m guiding people down a correct and stable path here.