[CCFPCM-0426] PART 2 db normalization #274
Conversation
chelsea-EYDS
force-pushed
the
CCFPCM-426-PART-2
branch
2 times, most recently
from
9c911c7
to
5a64f46
Compare
| @@ -10,11 +10,13 @@ import { BankLocationEntity, MerchantEntity } from '.'; | |||
| export class MinistryLocationEntity extends BaseLocationEntity { | |||
| @OneToMany(() => BankLocationEntity, (bank) => bank.location, { | |||
| cascade: true, | |||
| eager: true, | |||
There was a problem hiding this comment.
Please justify these eagers
There was a problem hiding this comment.
We almost always need these together - at least to integrate with the current code easily. Could be removed later if we refactor.
There was a problem hiding this comment.
I removed the eager from pos deposit and cash deposit, in the case of locations though we need the banks/merchants in most cases
There was a problem hiding this comment.
...not sure if my response is sufficient - please let me know if there is any action needed to resolve 😁
| ): Promise<NormalizedLocation[]> { | ||
| const locations = await this.locationRepo.find({ | ||
| ): Promise<MinistryLocationEntity[]> { | ||
| return await this.ministryLocationRepo.find({ |
There was a problem hiding this comment.
If you're returning straight, I'm pretty sure you don't need the await, as .find is already an async function whose results will be properly returned
| @@ -19,7 +19,7 @@ export class CashDepositMock extends CashDepositEntity { | |||
| this.deposit_date = dateRange.maxDate; | |||
| this.metadata = metadata; | |||
| this.deposit_amt_cdn = amount; | |||
| this.pt_location_id = location.pt_location_id; | |||
| this.pt_location_id = location.banks[0].bank_id; | |||
There was a problem hiding this comment.
Is this always true that pt_location_id corresponds to this? A location won't always have a set of banks right?
I know this is a mock, but I'm curious
There was a problem hiding this comment.
Yes, a location always has a set of banks.
There was a problem hiding this comment.
Hmm, you are correct though, not all banks will necessarily have an entry in the cash deposit files. I am wondering if we should include the method on this table.
(all banks have a location_id, all location_ids have banks, all cash_deposit rows have a pt_location_id which corresponds to a bank_id which corresponds to a location, but since we're storing the bank_id for non cash as well then it's possible that banks[0] would not correspond to a cash type bank_id.)
chelsea-EYDS
force-pushed
the
CCFPCM-426-PART-2
branch
from
5a64f46
to
5f11805
Compare
chelsea-EYDS
force-pushed
the
CCFPCM-426-PART-2
branch
from
478dfd3
to
773beef
Compare
chelsea-EYDS
force-pushed
the
CCFPCM-426-PART-2
branch
from
9479755
to
a1f1c07
Compare
migrations: FK joins on new tables migrations: FK joins on new tables
chelsea-EYDS
force-pushed
the
CCFPCM-426-PART-2
branch
from
467a7b5
to
9fd5d6b
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
CCFPCM-0426
Objective: