OBPIH-6690 Add option to edit unit price on invoice items made from order adjustments (fix after QA)#4849
Conversation
alannadolny
requested review from
awalkowiak,
drodzewicz,
ewaterman and
kchelstowski
github-actions
bot
added
domain: frontend
| inverse: inverse, | ||
| isCanceled: orderItem?.canceled ?: orderAdjustment?.canceled, | ||
| quantityAvailableToInvoice: quantityAvailableToInvoice, | ||
| amountAvailableToInvoice: orderAdjustment?.amountAvailableToInvoice, |
There was a problem hiding this comment.
do we only need this field for adjustments? It'll be null for regular invoice items. I know these are just for display but intuitively it feels like both fields should work for both cases (with quantity being 1 or 0 for adjustments).
All these fields are starting to feel confusing to me... Shouldn't amountAvailableToInvoice be based on the amount field? Maybe a transient would apply best here (though I think this class is starting to get overloaded with those).
There was a problem hiding this comment.
do we only need this field for adjustments? It'll be null for regular invoice items. I know these are just for display but intuitively it feels like both fields should work for both cases (with quantity being 1 or 0 for adjustments).
This is InvoiceItem's toJson. So in reality this handles all cases, not only adjustments
All these fields are starting to feel confusing to me... Shouldn't amountAvailableToInvoice be based on the amount field? Maybe a transient would apply best here (though I think this class is starting to get overloaded with those).
Yeah this might be confusing, the unit price and amount for order adjustments, but I would say those two are the same (potentially only differs with sign). So perhaps it is misleading in some places. Tomorrow I'll take a deeper look at the partial invoicing of adjustments, if there is some potential for some simplification.
There was a problem hiding this comment.
This is InvoiceItem's toJson. So in reality this handles all cases, not only adjustments
So we'd need amountAvailableToInvoice to look at the shipment or order item as well then right?
Yeah this might be confusing, the unit price and amount for order adjustments, but I would say those two are the same (potentially only differs with sign). So perhaps it is misleading in some places. Tomorrow I'll take a deeper look at the partial invoicing of adjustments, if there is some potential for some simplification.
Sounds good!
There was a problem hiding this comment.
So we'd need amountAvailableToInvoice to look at the shipment or order item as well then right?
We're not allowing direct amount update on non-adjustment related items
| const validateUnitPrice = (row) => { | ||
| if (row?.unitPrice === 0 || Math.abs(row?.unitPrice) > Math.abs(row?.amount)) { | ||
| const amountAvailableToInvoice = | ||
| Math.abs(row?.amountAvailableToInvoice) + Math.abs(row?.amount); |
There was a problem hiding this comment.
I think this is a case where we have "amount available outside this invoice item" plus "amount from this item" to determine full available amount
There was a problem hiding this comment.
I just thought it'd be amount available to invoice = current amount available to invoice - amount we're invoicing for in this row
…rder adjustments (fix after QA) (openboxes#4849)
…rder adjustments (fix after QA) (openboxes#4849)
3 participants
No description provided.