Extract logic from lib index file to dedicated modules - Resolves #90 #91
Conversation
brafdlog
changed the title
brafdlog
changed the title
brafdlog
force-pushed
the
breakLibIndex
branch
from
f9b01ac to
892c357
Compare
|
I'm trying to think about a way to force us to import only from Maybe this: |
| export async function printYnabAccountData() { | ||
| const ynabAccountData = await getYnabAccountDetails(); | ||
| const companyIdToAccountNumbers = await getFinancialAccountNumbers(); | ||
| console.log(); | ||
| console.log(); | ||
| console.log('------YNAB Budgets------'); | ||
| console.log(ynabAccountData.budgets); | ||
| console.log(); | ||
| console.log('------YNAB Accounts------'); | ||
| console.log(ynabAccountData.accounts); | ||
| console.log(); | ||
| console.log('------Financial institutions account numbers------'); | ||
| console.log(companyIdToAccountNumbers); | ||
| console.log(); | ||
| } |
There was a problem hiding this comment.
I considered this. Since the setup of ynab requires matching account ids in ynab and the account ids in the financial and fetching all those ids is annoying, I wrote these to make it easier to do the setup. This is another flow of setup scripts (in addition to the flows of config and scrape) so I extracted it to a file of its own.
I get your point that this is ynab specific so I will leave it in ynab and reexport it from here
There was a problem hiding this comment.
On second thought I think we should keep it separated in the setup helpers. As you can see this is very specific code that is only relevant to the setup flow. It uses ynab's getYnabAccountDetails as a building block but the rest is not related to the ynab api and I don't want to mix this code with the regular flow.
WDYT?
There was a problem hiding this comment.
If I understand you correctly, this is just an "external tool" to easily config the ynab at the first time (?).
If so, yes, let's keep it far from the library/module code. Even in a different folder, tools or something.
But to understand, if it not in the regular config+scrape flow, how can you run it? By running the code itself? How it published?
There was a problem hiding this comment.
Do not pay too much attention to where I said put it. Maybe the file is in the right place.
There was a problem hiding this comment.
At this point it is running the code itself. We could create a ui flow for setup that will do this behind the scenes and instead of printing it to the console will help you fill the config. But at this stage it is just a code tool.
| async function getFinancialAccountNumbers() { | ||
| const config = await configManager.getConfig(); | ||
|
|
||
| const startDate = moment() | ||
| .subtract(30, 'days') | ||
| .startOf('day') | ||
| .toDate(); | ||
|
|
||
| console.log('Fetching data from financial institutions to determine the account numbers'); | ||
| const companyIdToTransactions = await scrapeFinancialAccountsAndFetchTransactions(config, startDate); | ||
| const companyIdToAccountNumbers: Record<string, string[]> = {}; | ||
| Object.keys(companyIdToTransactions).forEach((companyId) => { | ||
| let accountNumbers = companyIdToTransactions[companyId].map((transaction) => transaction.accountNumber); | ||
| accountNumbers = _.uniq(accountNumbers); | ||
| companyIdToAccountNumbers[companyId] = accountNumbers; | ||
| }); | ||
| return companyIdToAccountNumbers; | ||
| } |
There was a problem hiding this comment.
Why not in bankScraper module?
There was a problem hiding this comment.
This I will move into the importTransactions module since it is related logic and uses the scrapeFinancialAccountsAndFetchTransactions function defined there
brafdlog
force-pushed
the
breakLibIndex
branch
from
892c357 to
1349e6a
Compare
Opened a separate issue for this #93 |
…nfig-25.1.0 Bump jest-config from 24.9.0 to 25.1.0
Extract the logic from the lib index file into dedicated modules for the different parts of the flow.
Probably easier to review the changes commit by commit because of the folder structure changes.