sfi-2225 subscribe to statement data updates #10
Conversation
| const getCalculationBySbi = async (sbi, transaction) => { | ||
| return db.organisation.findOne({ | ||
| transaction, | ||
| lock: true, |
There was a problem hiding this comment.
What's the value in locking the rows if we're not going to update the locked record?
There was a problem hiding this comment.
Again, naively gust copied from other db retrievals, inserts, etc -- will remove as I assume this is only used for data entry
There was a problem hiding this comment.
Yeah you can lock it if you're going to update that record. If you're not going to update, no value in the lock. Because you're running in a transaction shouldn't cause any issues but it adds no value here.
| const getCompletedPaymentRequestByInvoiceNumber = async (invoiceNumber, transaction) => { | ||
| return db.paymentRequest.findOne({ | ||
| transaction, | ||
| lock: true, |
There was a problem hiding this comment.
same as above, any reason for lock?
| console.info(`Duplicate calculation received, skipping ${existingCalculation.sbi}`) | ||
| await transaction.rollback() | ||
| } else { | ||
| await savePlaceholderOrganisation({ sbi: calculation.sbi }, calculation.sbi) |
There was a problem hiding this comment.
Should this also include the FRN too?
There was a problem hiding this comment.
Calculation data does not have frn
| @@ -0,0 +1,13 @@ | |||
| const db = require('../../data') | |||
|
|
|||
| const getCalculationBySbi = async (sbi, transaction) => { | |||
There was a problem hiding this comment.
This function is called get calculation but returns an organisation?
There was a problem hiding this comment.
Fixing going to get by calculationReference to clear up comment below too
There was a problem hiding this comment.
Hmmm, GitHub dnt do gifs... noted
| const transaction = await db.sequelize.transaction() | ||
|
|
||
| try { | ||
| const existingCalculation = await getCalculationBySbi(calculation.sbi, transaction) |
There was a problem hiding this comment.
An existing calculation should be calculationRef not SBI. One SBI will have multiple legitimate calculations if they've had a post payment adjustment.
There was a problem hiding this comment.
I hadda comment next to this asking tht exact ques on which field to use
| const db = require('../../data') | ||
|
|
||
| const savePlaceholderOrganisation = async (organisation, sbi, transaction) => { | ||
| await db.organisation.findOrCreate({ |
There was a problem hiding this comment.
As you're doing this by PK you could use upsert rather than findOrCreate. However, I don't actually know if there's any actual benefit to doing that so more just for info.
|
Kudos, SonarCloud Quality Gate passed! |
This reverts commit ebe9fb9.
https://eaflood.atlassian.net/browse/SFI-2225