OBPIH-6746 Value in amount column of PO Invoice tab for regular POs#4880
OBPIH-6746 Value in amount column of PO Invoice tab for regular POs#4880awalkowiak merged 7 commits intorelease/0.9.2-hotfix1from
Conversation
alannadolny
requested review from
awalkowiak,
drodzewicz,
ewaterman and
kchelstowski
github-actions
bot
added
domain: frontend
| * Total shipment item value | ||
| * @deprecated From now should rely on the amount field instead always calculating it | ||
| */ | ||
| def getTotalAmount() { |
There was a problem hiding this comment.
This is being used in InvoiceService.getInvoiceItemsCsv. You should hopefully just need to switch to use amount instead of totalAmount
There was a problem hiding this comment.
Yup, @alannadolny please replace it in the service as well
| <preConditions onFail="MARK_RAN"> | ||
| <and> | ||
| <columnExists tableName="invoice_item" columnName="amount"/> | ||
| <not> |
There was a problem hiding this comment.
Weird that liquibase requires this clunky syntax and doesn't have a better solution like expectedResult=">0" or <sqlCheck notExpectedResult="0">
| </and> | ||
| </preConditions> | ||
|
|
||
| <sql> |
There was a problem hiding this comment.
can you add a comment here clarifying that this is only allowed because we've already migrated all prepayment invoices and so the only invoice items with a null amount at this point should be non-prepayment ones
There was a problem hiding this comment.
Also, I'm surprised you can't just do this:
UPDATE invoice_item
SET amount = quantity * IFNULL(unit_price, 0)
WHERE amount IS NULL;
| * Total shipment item value | ||
| * @deprecated From now should rely on the amount field instead always calculating it | ||
| */ | ||
| def getTotalAmount() { |
There was a problem hiding this comment.
Yup, @alannadolny please replace it in the service as well
| unitPriceAvailableToInvoice: orderAdjustment?.unitPriceAvailableToInvoice, | ||
| // Total amount and total prepayment amount are deprecated and amount field | ||
| // should be used instead (OBPIH-6398, OBPIH-6499) | ||
| totalAmount: totalAmount, |
There was a problem hiding this comment.
I wonder if technically these two fields should stay for now, because this was expected as a part of getPrepaymentItems. I am not sure if we have a defined API endpoint deprecation flow. However, there is really high probability we were the only ones using it.
There was a problem hiding this comment.
so what's the decision here? should I revert those properties?
| SELECT id, quantity * IFNULL(unit_price, 0) AS calculated_amount | ||
| FROM invoice_item | ||
| ) AS calculated_amount ON calculated_amount.id = item.id | ||
| SET item.amount = calculated_amount.calculated_amount |
There was a problem hiding this comment.
It's nitpicky, but calculated_amount.calculated_amount looks weird.
3 participants
No description provided.