Skip to content

[CHORE] Fix heuristic bug on eager matching #49

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

Merged
vyasworks merged 1 commit into from
Feb 8, 2023
Merged

[CHORE] Fix heuristic bug on eager matching #49

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 12 additions & 9 deletions apps/backend/src/reconciliation/pos-reconciliation.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,32 +32,35 @@ export class POSReconciliationService {

// Basic EAGER! 1:1 matching

for (const payment of payments) {
for (const [pindex, payment] of payments.entries()) {
// console.log(`Processing payment for ${payment.amount} - ${payment.id} - ${payment.timestamp}`)
for (const deposit of deposits) {
for (const [dindex, deposit] of deposits.entries()) {
// console.log(`Processing deposit ${deposit.id} for ${deposit.transaction_amt} - ${deposit.timestamp}`)

// TODO:make this a strategy pattern
if (
payment.amount === deposit.transaction_amt &&
payment.method === deposit.card_vendor &&
deposit.match === false &&
differenceInSeconds(payment.timestamp, deposit.timestamp) < 240
) {

// mutate the original array to use in next heurisitc match
Copy link
Contributor Author

@vyasworks vyasworks Feb 8, 2023

Choose a reason for hiding this comment

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

This similar problem will exist in cash totals as well, needs fix

Copy link
Contributor

@chelsea-EYDS chelsea-EYDS Feb 8, 2023

Choose a reason for hiding this comment

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

Could we implement the layered approach here, but invert it. So first we check 1:1 matching where difference in seconds is very small, then if no matches, continue to broaden the window until there are none yet left to be matched for the date?

Copy link
Contributor Author

@vyasworks vyasworks Feb 8, 2023

Choose a reason for hiding this comment

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

start with 10 seconds and broaden until 240 (or whatever upper limit is) seconds? Then yes.

if more, then no.

Copy link
Contributor

@chelsea-EYDS chelsea-EYDS Feb 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Okay, so there is no case in which a transaction occurs and the deposit could be delayed greater than 240 seconds?

Copy link
Contributor Author

@vyasworks vyasworks Feb 8, 2023

Choose a reason for hiding this comment

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

there could be but having a greater time span make more possibility for bad matches than good ones? this could be wrong but how I understand it now. we can verify with BAs.

// after this deposit is used up, mark it as true and don't use it again
payments[pindex].match = true;
deposits[dindex].match = true;
payments[pindex].deposit_id = deposit.id;
deposits[dindex].matched_payment_id = payment.id;

matches.push({
payment,
deposit
});

break;
}
}
}

for (const match of matches) {
match.payment.match = true;
match.deposit.match = true;
match.payment.deposit_id = match.deposit.id;
match.deposit.matched_payment_id = match.payment.id;
}
return matches;
}

Expand Down