OBPIH-6497 [backend] Determine encumbered and invoiceable items or adjustments (fix after QA)#4778
OBPIH-6497 [backend] Determine encumbered and invoiceable items or adjustments (fix after QA)#4778awalkowiak merged 3 commits intodevelopfrom
Conversation
alannadolny
requested review from
awalkowiak,
drodzewicz,
ewaterman and
kchelstowski
github-actions
bot
added
the
domain: backend
| Boolean isInvoiceable() { | ||
| return encumbered && (canceled || quantityAvailableToInvoice > 0) | ||
| return hasPrepaymentInvoice && | ||
| !hasRegularInvoice && |
There was a problem hiding this comment.
Is this !hasRegularInvoice correct? What if an order item will be split across multiple shipments and then multiple final invoices (= multiple regular invoices)?
There was a problem hiding this comment.
btw, for order adjustments it makes sense, but I am not sue if it makes sense for order items
There was a problem hiding this comment.
yes an order item can be split across multiple shipments, but I'm not sure if we support a shipment item being split across multiple invoices. I think we expect that a shipment item can only be invoiced once. This is because if we want to invoice a quantity 2 but a quantity 4 was shipped, then that indicates 2 qty was returned (or whatever) and the shipment item should then be considered fully invoiced (even though we only invoiced quantity 2 of 4).
There was a problem hiding this comment.
btw here's an example of what an invoice would look like if we have multiple shipments for each order item
|
|
||
| Boolean isInvoiceable() { | ||
| return !isInvoiced && order.placed | ||
| return hasPrepaymentInvoice && !hasRegularInvoice && order.placed |
There was a problem hiding this comment.
Is this because isInvoiced used for non-prepay flows?
There was a problem hiding this comment.
isInvoiced looks for posted invoices, and here we want to have both posted and not yet posted invoices
| orderStatus | hasRegularInvoice || isInvoiceable | ||
| OrderStatus.PENDING | false || false | ||
| OrderStatus.PLACED | false || true | ||
| OrderStatus.PENDING | false || false |
There was a problem hiding this comment.
hasRegularInvoice should be true here right? Otherwise it;s the same as the first case
|
I will be taking over this one |
plus remove isEncumbered for clarity
There was a problem hiding this comment.
Can we please follow the convention for properties like quantity?
quantityDescriptionOfQuantity
like
quantityOrdered
quantityShipped
quantityOnHand
quantityAvailableToPromise
I've seen a few times where the descriptor leaks into the beginning of the property name
invoicedQuantity
postedQuantityInvoicedInStandardUom
these should probably be
quantityInvoiced
and
quantityPosted
quantityPostedInStandardUom
If there's a good reason place the descriptor upfront please let me know ... otherwise it should always be.
quantity<descriptor>
5 participants
No description provided.