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.

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

New: handle oneZero scrape #628

Open
wants to merge 2 commits into
base: feature/one-zerop-bank
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions packages/main/src/backend/configManager/configManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { decrypt, encrypt } from '@/backend/configManager/encryption/crypto';
import { existsSync, promises as fs } from 'fs';
import configExample from './defaultConfig';
import logger from '/@/logging/logger';
import type { CompanyTypes } from 'israeli-bank-scrapers-core';

export async function getConfig(configPath: string = configFilePath): Promise<Config> {
const configFromFile = await getConfigFromFile(configPath);
Expand Down Expand Up @@ -35,6 +36,23 @@ export async function updateConfig(configPath: string, configToUpdate: Config):
await fs.writeFile(configPath, encryptedConfigStr);
}

export async function updateOtpLongTermToken(
key: CompanyTypes,
otpLongTermToken: string,
configPath: string = configFilePath,
): Promise<void> {
Comment on lines +39 to +43
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename to companyId

Suggested change
export async function updateOtpLongTermToken(
key: CompanyTypes,
otpLongTermToken: string,
configPath: string = configFilePath,
): Promise<void> {
export async function updateOtpLongTermToken(
companyId: CompanyTypes,
otpLongTermToken: string,
configPath: string = configFilePath,
): Promise<void> {

const config = await getConfig(configPath);
const account = config.scraping.accountsToScrape.find((acc) => acc.key === key) as {
loginFields: { otpLongTermToken: string };
};
if (account) {
account.loginFields.otpLongTermToken = otpLongTermToken;
await updateConfig(configPath, config);
} else {
throw new Error(`Account with key ${key} not found`);
}
Comment on lines +48 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

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

return early: if not account, throw. Otherwise, continue.

Suggested change
if (account) {
account.loginFields.otpLongTermToken = otpLongTermToken;
await updateConfig(configPath, config);
} else {
throw new Error(`Account with key ${key} not found`);
}
if (!account) {
throw new Error(`Account with key ${key} not found`);
}
account.loginFields.otpLongTermToken = otpLongTermToken;
await updateConfig(configPath, config);

}

async function getConfigFromFile(configPath: string) {
if (existsSync(configPath)) {
return fs.readFile(configPath, {
Expand Down
49 changes: 33 additions & 16 deletions packages/main/src/backend/import/bankScraper.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { type AccountToScrapeConfig } from '@/backend/commonTypes';
import { CompanyTypes, createScraper, SCRAPERS, type ScraperOptions } from 'israeli-bank-scrapers-core';
import { CompanyTypes, createScraper, type ScraperOptions, SCRAPERS } from 'israeli-bank-scrapers-core';
import { ipcMain } from 'electron';
import { EventNames, type EventPublisher } from '@/backend/eventEmitters/EventEmitter';
import { updateOtpLongTermToken } from '@/backend/configManager/configManager';

export const inputVendors = Object.keys(SCRAPERS)
// Deprecated. see https://github.com/eshaham/israeli-bank-scrapers/blob/07ecd3de0c4aa051f119aa943493f0cda943158c/src/definitions.ts#L26-L29
.filter((key) => key !== CompanyTypes.hapoalimBeOnline)
.map((key) => ({
key,
...SCRAPERS[key as CompanyTypes],
}));
.map((key) => ({ key, ...SCRAPERS[key as CompanyTypes] }));

Comment on lines 7 to 10
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't remove this comment

interface ScrapeParameters {
companyId: AccountToScrapeConfig['key'];
Expand All @@ -23,20 +22,38 @@ export async function scrape(
{ companyId, credentials, startDate, timeout, showBrowser = false }: ScrapeParameters,
emitProgressEvent: EmitProgressEventFunction,
chromePath: string,
eventPublisher: EventPublisher,
) {
const options: ScraperOptions = {
companyId, // mandatory; one of 'hapoalim', 'discount', 'otsarHahayal', 'leumiCard', 'isracard', 'amex'
startDate, // the date to fetch transactions from (can't be before the minimum allowed time difference for the scraper)
combineInstallments: false, // if set to true, all installment transactions will be combine into the first one
showBrowser, // shows the browser while scraping, good for debugging (default false)
verbose: false, // include more debug info about in the output
companyId,
startDate,
combineInstallments: false,
showBrowser,
verbose: false,
executablePath: chromePath,
defaultTimeout: timeout,
};

const scraper = createScraper(options);
scraper.onProgress((eventCompanyId: string, payload: { type: string }) => {
emitProgressEvent(companyId, payload.type);
});
const scrapeResult = await scraper.scrape(credentials);
return scrapeResult;
scraper.onProgress((eventCompanyId, payload) => emitProgressEvent(companyId, payload.type));

if (companyId === CompanyTypes.oneZero) {
const creds = credentials as typeof credentials & { otpLongTermToken: string; phoneNumber: string };
if (!creds.otpLongTermToken) {
await scraper.triggerTwoFactorAuth(creds.phoneNumber);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure how the process is implemented, but I see the triggerTwoFactorAuth is called during the scraping on israeli-bank-scrapers, see triggerTwoFactorAuth, so why running it in our code too?

await eventPublisher.emit(EventNames.GET_OTP);
Comment on lines +43 to +44
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Use emitProgressEvent instead of eventPublisher.emit.
  2. Is the scraper not emitting events of OTP itself?

const otpCode = await new Promise<string>((resolve) => {
ipcMain.once('get-otp-response', (event, input) => resolve(input));
});
Comment on lines +45 to +47
Copy link
Collaborator

Choose a reason for hiding this comment

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

All IPC calls are declared here: packages/main/src/handlers/index.ts

I expect this call to follow that pattern.

const result = await scraper.getLongTermTwoFactorToken(otpCode);
if ('longTermTwoFactorAuthToken' in result) {
await updateOtpLongTermToken(companyId, result.longTermTwoFactorAuthToken);
creds.otpLongTermToken = result.longTermTwoFactorAuthToken;
} else {
throw new Error('Failed to get long-term two-factor auth token');
}
Comment on lines +48 to +54
Copy link
Collaborator

Choose a reason for hiding this comment

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

use result.success

Suggested change
const result = await scraper.getLongTermTwoFactorToken(otpCode);
if ('longTermTwoFactorAuthToken' in result) {
await updateOtpLongTermToken(companyId, result.longTermTwoFactorAuthToken);
creds.otpLongTermToken = result.longTermTwoFactorAuthToken;
} else {
throw new Error('Failed to get long-term two-factor auth token');
}
const result = await scraper.getLongTermTwoFactorToken(otpCode);
if (result.success) {
await updateOtpLongTermToken(companyId, result.longTermTwoFactorAuthToken);
creds.otpLongTermToken = result.longTermTwoFactorAuthToken;
} else {
throw new Error('Failed to get long-term two-factor auth token');
}

}
}
Comment on lines +40 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function must be extracted into a function. More than that, think when more banks will require OTP, I expect the "if" to be extracted into a function to.

I hope you understand what I mean. Here should be only handleOtp function, and inside it there will be if-else/switch-case that will direct into bank-specific function.


return await scraper.scrape(credentials);
}
34 changes: 4 additions & 30 deletions packages/main/src/backend/import/importTransactions.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
import { configFilePath, userDataPath } from '@/app-globals';
import { userDataPath } from '@/app-globals';
import {
type AccountToScrapeConfig,
type Config,
type EnrichedTransaction,
type FinancialAccountDetails,
type ScraperScrapingResult,
} from '@/backend/commonTypes';
import { getConfig } from '@/backend/configManager/configManager';
import * as bankScraper from '@/backend/import/bankScraper';
import Bottleneck from 'bottleneck';
import { type Transaction } from 'israeli-bank-scrapers-core/lib/transactions';
import _ from 'lodash';
import moment from 'moment';
// import * as categoryCalculation from '@/backend/import/categoryCalculationScript';
import {
AccountStatus,
BudgetTrackingEventEmitter,
DownalodChromeEvent,
EventNames,
ImporterEvent,
type EventPublisher,
ImporterEvent,
} from '../eventEmitters/EventEmitter';
import { calculateTransactionHash } from '../transactions/transactions';
import getChrome from './downloadChromium';
Expand Down Expand Up @@ -97,26 +92,6 @@ function emitChromeDownload(eventPublisher: EventPublisher, percent: number) {
eventPublisher.emit(EventNames.DOWNLOAD_CHROME, new DownalodChromeEvent(percent));
}

export async function getFinancialAccountDetails(): Promise<FinancialAccountDetails[]> {
Comment on lines 93 to 94
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is it? I see it is unused..?

If it is not related to your work, I prefer you to open a new PR for this, so if there will be a problem with this remove, it will be easy to revert it.

const config = await getConfig(configFilePath);
const eventEmitter = new BudgetTrackingEventEmitter();

const startDate = moment().subtract(30, 'days').startOf('day').toDate();

const companyIdToTransactions = await scrapeFinancialAccountsAndFetchTransactions(
config.scraping,
startDate,
eventEmitter,
);
const financialAccountDetails: { name: string; accountNumber: string }[] = [];
Object.keys(companyIdToTransactions).forEach((companyId) => {
let accountNumbers = companyIdToTransactions[companyId].map((transaction) => transaction.accountNumber);
accountNumbers = _.uniq(accountNumbers);
accountNumbers.forEach((accountNumber) => financialAccountDetails.push({ name: companyId, accountNumber }));
});
return financialAccountDetails;
}

async function fetchTransactions(
account: AccountToScrapeConfig,
startDate: Date,
Expand All @@ -142,6 +117,7 @@ async function fetchTransactions(
},
emitImporterProgressEvent,
chromePath,
eventPublisher,
);
if (!scrapeResult.success) {
throw new Error(`${scrapeResult.errorType}: ${scrapeResult.errorMessage}`);
Expand Down Expand Up @@ -193,14 +169,12 @@ async function postProcessTransactions(

function enrichTransaction(transaction: Transaction, companyId: string, accountNumber: string): EnrichedTransaction {
const hash = calculateTransactionHash(transaction, companyId, accountNumber);
// const category = categoryCalculation.getCategoryNameByTransactionDescription(transaction.description);
const enrichedTransaction: EnrichedTransaction = {
return {
...transaction,
accountNumber,
// category,
hash,
Comment on lines 171 to 175
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I see we don't have a category calculation anymore.

Anyway, please return the comment back.

Yes, I agree that developer should avoid code comments, but this project is in low maintainance and I prefer to keep such comments for the next time I will search something.

};
return enrichedTransaction;
}

function transactionsDateComparator(t1: Transaction, t2: Transaction) {
Expand Down
6 changes: 0 additions & 6 deletions packages/main/src/manual/setupHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import { FETCH_YNAB_ACCOUNT_DATA_STATUS, type YnabAccountDataType, type YnabConfig } from '@/backend/commonTypes';
import { getConfig } from '@/backend/configManager/configManager';
import { getYnabAccountDetails, isAccessTokenValid } from '@/backend/export/outputVendors/ynab/ynab';
// import { getFinancialAccountDetails } from '@/backend/import/importTransactions';

export async function getYnabAccountData(_: unknown, ynabOptions: YnabConfig['options']): Promise<YnabAccountDataType> {
const config = await getConfig();
Expand All @@ -13,21 +12,16 @@ export async function getYnabAccountData(_: unknown, ynabOptions: YnabConfig['op
status: FETCH_YNAB_ACCOUNT_DATA_STATUS.INVALID_ACCESS_TOKEN,
};
}
// const ynabAccountDataPromise = getYnabAccountDetails(config.outputVendors);
// const financialAccountDetailsPromise = getFinancialAccountDetails();
// const [ynabAccountData, financialAccountDetails] = await Promise.all([ynabAccountDataPromise, financialAccountDetailsPromise]);
const ynabAccountData = await getYnabAccountDetails(
config.outputVendors,
ynabOptions.budgetId,
ynabOptions.accessToken,
);
// const financialAccountDetails = [];

return {
status: ynabAccountData.categories
? FETCH_YNAB_ACCOUNT_DATA_STATUS.SUCCESS
: FETCH_YNAB_ACCOUNT_DATA_STATUS.INVALID_BUDGET_ID,
ynabAccountData,
// financialAccountDetails
};
}
2 changes: 2 additions & 0 deletions packages/renderer/src/components/App.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { StoresProvider } from '../store';
import './App.css';
import Body from './Body';
import GetOtp from './GetOtp';
import TopBar from './topBar/TopBar';

function App() {
Expand All @@ -9,6 +10,7 @@ function App() {
<div className="App">
<TopBar />
<Body />
<GetOtp />
</div>
</StoresProvider>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/renderer/src/components/GetOtp.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useEffect, useState } from 'react';
import { Button, FormControl, Modal } from 'react-bootstrap';
import { observer } from 'mobx-react-lite';
import { useConfigStore } from '/@/store/ConfigStore';
import { useConfigStore } from '../store/ConfigStore';
import { sendOTPResponse } from '#preload';

const GetOtp = () => {
Expand Down
Loading