What method is getting called when generating an invoice for a PO that wasn't pre-paid? If this API is used in non-prepay related flows we don't want to have it use the prepaymentInvoiceService

nvm I see the first line in the method checks that it has a prepayment invoice already.

Have you considered moving these methods to a PrepaymentInvoiceController since you're also doing that for the service methods?

I think it's not worth it, to be honest. Later we will add PrepaymentInvoiceApiController, so this one will have only one endpoint.

why is this needed? You can't just call mockDomains in setup?

is this not going to affect non-prepayment invoices? Or is the assumption that quantityInvoiced is always 0 for that because we don't support partial invoicing?

What's your plan for eventually handling cancellations/returns/reductions of quantity?

quantityInvoiced loops through all the invoices and sums the quantity, which works fine for the case where there are no quantity adjustments but... if a shipment item has quantity 3 and we cancelled or returned the shipment, we generate an invoice with quantity 0. In that scenario quantityInvoiced for the shipment item is 0, and isInvoiceable will be true (which is incorrect). We'll need some way to note that even if we invoice for less than the quantity shipped for that shipment item, we've still "fully invoiced" it.

Same thought for reducing quantity (shipment has quantity 3 but in reality they shipped 2 so we only want to invoice for quantity 2 (or whatever that case looks like))

Side note: I remember chatting with Manon about whether we need to be able to split a single shipment item up into multiple invoices (Ex: we have a shipment item with quantity 3 which has an invoice item with quantity 1 and another invoice item with quantity 2) and I thiiiiink she said we don't need the ability to do that (probably worth confirming).

So maybe it'd just be a matter of changing the getQuantityInvoiced method to return 0 for shipment items without an invoice and shipmentItem.quantity for shipment items that do have an invoice. So we ignore the quantity in the invoiceItem for the purpose of determining if the shipment item has been fully invoiced. (Which we can do because we can safely assume that each shipmentItem will only ever have a single invoiceItem so if it was invoiced at all, it's done.)

I know the work to implement cancellations isn't covered by this ticket, I just want to make sure it'll eventually be able to work within this system.

Generally my preference would be to not have services call other services but instead move common code to shared util classes (Ex: InvoiceNumberGenerator, InvoiceItemGenerator (or whatever names)). It breaks the code up into more manageable units (easier testing too).

I don't expect you to make any kind of change like that here though. Just noting down my thoughts.

@ewaterman could you expand on that? I am a bit surprised you suggest not to inject services in services. It's a known practice throughout spring community.
Even if for example we wanted to just fetch a product and used productService.findProduct (or anything like this).
How would you move such method then? If it were supposed to be a util class, anyway it would have to be a bean, so it would evaluate to a "service" anyway.