Skip to content

Match And Kill Round 1#37

Merged
vyasworks merged 7 commits intomainfrom
match-and-kill
Feb 3, 2023
Merged

Match And Kill Round 1#37
vyasworks merged 7 commits intomainfrom
match-and-kill

Conversation

@chelsea-EYDS
Copy link
Contributor

No description provided.

@chelsea-EYDS chelsea-EYDS changed the title WIP Match And Kill Round 1 Jan 28, 2023
}
}

public async getMerchantIds(location_id: number): Promise<number[]> {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be getMerchantIdsByLocationId()

Copy link
Contributor

@vyasworks vyasworks left a comment

Choose a reason for hiding this comment

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

I understand this is a draft PR.

I have added my comments (you might have already considered them or they may be obvious)

I think this is in a good shape to work on top of too add more layers and heuristics


public async getMerchantIds(location_id: number): Promise<number[]> {
const merchant_ids = await this.transactionRepo.manager.query(`
SELECT DISTINCT "Merchant ID"
Copy link
Contributor

Choose a reason for hiding this comment

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

This function should be in location service

SELECT DISTINCT "Merchant ID"
FROM public.master_location_data ml
WHERE ml."GARMS Location" = ${location_id}
AND "Type" != 'Bank'
Copy link
Contributor

Choose a reason for hiding this comment

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

add Bank in constants.ts

return merchant_ids.map((itm: any) => parseInt(itm['Merchant ID']));
}

public async getPTIDs(location_id: number): Promise<string[]> {
Copy link
Contributor

Choose a reason for hiding this comment

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

The naming is unclear here, should it be getPTLocdByGarmsLocId


public async getPTIDs(location_id: number): Promise<string[]> {
const pt_ids = await this.transactionRepo.manager.query(`
SELECT DISTINCT "Location"
Copy link
Contributor

Choose a reason for hiding this comment

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

Fn shd be in location service

SELECT DISTINCT "Location"
FROM public.master_location_data ml
WHERE ml."GARMS Location" = ${location_id}
`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Location Repository should be able to query this without a RAW query?

}

public async getMethod(sbc_code: number) {
const payment_method = await this.paymentMethodRepo.findOne({
Copy link
Contributor

Choose a reason for hiding this comment

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

Shd live in payment Service

}

public async reconcilePOS(date: string, location_id: number) {
const locations = await this.getLocationList();
Copy link
Contributor

Choose a reason for hiding this comment

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

Semantics are confusing here, making it hard to read a bit.

the parameter location_id -> denotes pos location or garms loc?

}
}

public async reconcilePOS(date: string, location_id: number) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we name this reconcilePOSBySalesLocation?

Also, this is a great example of what I call Layer

}
}

async queryCashTransactions(
Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure about the function of this query in this service, please clarify.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should return all of the payments for Cash/Cheque (to match against TDI17).

I am trying to query by the current and last deposit dates. This will probably need to be changed. I have not yet found a rule for date range that works for each location.

We should add a and match=false. Maybe then we can query for anything less than the current deposit date, as all of the previous payments should be accounted for? (and if they are not then an exception could be thrown?)

(a: any, b: any) => a + parseFloat(b.transaction_amt),
0
);

Copy link
Contributor

Choose a reason for hiding this comment

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

We can add line by line matching here?

@chelsea-EYDS chelsea-EYDS changed the base branch from module-updates to main January 30, 2023 19:38

//TODO this is temporary for testing the parsed garms json only
async readAndParseGarms(filename: string, filebuffer: Buffer) {
// TODO define return type
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Define Return Types/Outputs

Add another match layer

@vyasworks vyasworks marked this pull request as ready for review February 3, 2023 00:04
@vyasworks
Copy link
Contributor

WIP Merge... Follwup PR to fix issues

@vyasworks vyasworks merged commit 3675c07 into main Feb 3, 2023
@chelsea-EYDS chelsea-EYDS deleted the match-and-kill branch March 22, 2023 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants