feat(mizrahi): scrape extra info #890
Conversation
|
Thank you, I hope I will review it soon |
|
Can I list you as a code owner for Mizrahi #830? |
|
Pull request has been marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
| const PENDING_TRANSACTIONS_PAGE = '/osh/legacy/legacy-Osh-p420'; | ||
| const PENDING_TRANSACTIONS_IFRAME = 'p420.aspx'; | ||
| const CHANGE_PASSWORD_URL = /https:\/\/www\.mizrahi-tefahot\.co\.il\/login\/index\.html#\/change-pass/; | ||
| const DATE_FORMAT = 'DD/MM/YYYY'; | ||
| const MAX_ROWS_PER_REQUEST = 10000000000; | ||
| const TRANSACTION_DETAILS_REQUEST_CONCURRENCY = 1; | ||
| const TRANSACTION_DETAILS_REQUEST_WAIT_TIME = 500; // ms |
There was a problem hiding this comment.
You can add _MS to the variable name and save the comment.
| const TRANSACTION_DETAILS_REQUEST_WAIT_TIME = 500; // ms | |
| const TRANSACTION_DETAILS_REQUEST_WAIT_TIME_MS = 500; // ms |
| })); | ||
|
|
||
| const cookies = await this.page.cookies(); |
There was a problem hiding this comment.
Page-level cookie API is deprecated. Use Browser.cookies or BrowserContext.cookies instead.
| const headersMap: Record<string, any> = {}; | ||
| const response = await Promise.any(TRANSACTIONS_REQUEST_URLS.map(async (url) => { | ||
| const request = await this.page.waitForRequest(url); | ||
| const data = createDataFromRequest(request, this.options.startDate); | ||
| const headers = createHeadersFromRequest(request); | ||
| headersMap[url] = createHeadersFromRequest(request); | ||
|
|
||
| return fetchPostWithinPage<ScrapedTransactionsResult>(this.page, url, data, headers); | ||
| return fetchPostWithinPage<ScrapedTransactionsResult>(this.page, url, data, headersMap[url]); | ||
| })); | ||
|
|
||
| const cookies = await this.page.cookies(); | ||
| const headers = Object.values(headersMap)[0]; | ||
| headers.Cookie = cookies.map((cookie) => `${cookie.name}=${cookie.value}`).join('; '); |
There was a problem hiding this comment.
I don't like this headersMap solution which become another headers object.
But I can't find another way right now.
| const recordsWithDetails = originalRecords | ||
| .map((record, index) => ({ record, index })) | ||
| .filter(({ record }) => record.MC02ShowDetailsEZ === '1'); |
There was a problem hiding this comment.
Better to filter and then map
| return memo; | ||
| } | ||
|
|
||
| async function getExtraScrap(originalRecords: ScrapedTransaction[], currentTxns: Transaction[], headers: Headers): Promise<Transaction[]> { |
| const promises = recordsWithDetails.map(({ record }) => getTransactionExtraScrap(record, headers)); | ||
| let accounts: Array<ExtraTransactionResult | null> = []; | ||
| while (promises.length > 0) { | ||
| const currentPromises = promises.splice(0, TRANSACTION_DETAILS_REQUEST_CONCURRENCY); | ||
| accounts = accounts.concat(await Promise.all(currentPromises)); | ||
| await sleep(TRANSACTION_DETAILS_REQUEST_WAIT_TIME); | ||
| } |
There was a problem hiding this comment.
I think you're trying to do a batch proccesing here, but you missed something.
Once you run an async function, it runs! When you called getTransactionExtraScrap(record, headers), the function executed. It happened on the function call and not on the await call.
In your code, the promises hold all the executions, and you just waiting them one-by-one, it is not logical.
| const txnsWithExtra = currentTxns.map((txn, i) => { | ||
| const extraDetailIndex = recordsWithDetails.findIndex(({ index }) => index === i); | ||
| const extraDetails = extraDetailIndex !== -1 ? accounts[extraDetailIndex] : undefined; | ||
| const currentTxn = { ...txn }; | ||
| if (extraDetails) { | ||
| currentTxn.memo = simplifyExtraTransactionResultsToMemo(extraDetails); | ||
| } | ||
| return currentTxn; | ||
| }); |
There was a problem hiding this comment.
I guess you want to clone the current transaction, what do you think about this?
| const txnsWithExtra = currentTxns.map((txn, i) => { | |
| const extraDetailIndex = recordsWithDetails.findIndex(({ index }) => index === i); | |
| const extraDetails = extraDetailIndex !== -1 ? accounts[extraDetailIndex] : undefined; | |
| const currentTxn = { ...txn }; | |
| if (extraDetails) { | |
| currentTxn.memo = simplifyExtraTransactionResultsToMemo(extraDetails); | |
| } | |
| return currentTxn; | |
| }); | |
| const txnsWithExtra = currentTxns.map(({ ...currentTxn }, i) => { | |
| const extraDetailIndex = recordsWithDetails.findIndex(({ index }) => index === i); | |
| const extraDetails = extraDetailIndex !== -1 ? accounts[extraDetailIndex] : undefined; | |
| if (extraDetails) { | |
| currentTxn.memo = simplifyExtraTransactionResultsToMemo(extraDetails); | |
| } | |
| return currentTxn; | |
| }); |
| function simplifyExtraTransactionResultsToMemo(extraResult: ExtraTransactionResult): string { | ||
| let memo = ''; | ||
| extraResult.body.fields.forEach(field => | ||
| field?.forEach(group => | ||
| group?.Records.forEach(record => | ||
| record?.Fields.forEach((fieldRecord) => { | ||
| memo += `${fieldRecord.Label} ${fieldRecord.Value}; `; | ||
| }), | ||
| ), | ||
| ), | ||
| ); | ||
| return memo; | ||
| } |
There was a problem hiding this comment.
I'm not sure we want to build a memo by ourself.
Maybe you can add an example of an object and we can learn what are you building here.
|
Thanks for the review, I'll make the fixes as soon as I can. |
Fix #784.
Changes:
/Online/api/OSH/getMaherBerurimSMF) ifadditionalTransactionInformationistrue.txn.memoin the formatfield1: value1; field2: value2; ...;Status and todos:
I don't have a lot of experience with
puppeteer, would love to get some inputs about the unchecked boxes.Thanks