OBPIH-5847 Fix poor performance on PO show page #4266

awalkowiak · 2023-09-05T15:05:49Z

No description provided.

kchelstowski · 2023-09-05T15:24:54Z

grails-app/controllers/org/pih/warehouse/order/OrderController.groovy

+    def orderComments = {
+        Order order = Order.get(params.id)
+        render(template: "orderComments", model: [orderInstance: order])
+    }


I'm wondering if this is the result we wanted to have?
I imagine that now, when we go to the PO view page, and go between the tabs, we everytime fetch the same order instance (with all the fields etc).

I know that probably anyway it is faster now, but maybe we should return to the view only fields that we really need, so maybe to return some kind of command/dto objects, e.g.

class OrderCommentCommand { String orderId Comments comments }

(pseudocode below, probably could be simplified/be more clean)

def orderComments = { Order order = Order.get(params.id) OrderCommentCommand orderCommentCommand = new OrderCommentCommand(orderId: order?.id, comments: order?.comments) render(template: "orderComments", model: [orderCommentCommand: orderCommentCommand]) }

etc.

You still fetch the same order in your example

I know, but I don't return the whole order object to the view, but just the part, that we really need there (this is why I wrote "with all the fields etc")
If we wanted to improve it even more, we could create a query for that, so not to fetch the Order at the top, but only the part from the "command object" I presented.

PS. Data services could be a good candidate if we decided to create custom queries for those, and there to use the @Query annotation.

Initially, I wanted to pass the entire order instance object in params, but it was stringified, so I changed it to sending order's id only, and this additional get did not really affect the performance in this case since the rendering is a problematic part now. Plus adding a command for each tab adds more work to be done for this "quick fix"

Yup, those are only suggestions that came to my mind, if we wanted to improve the performance even more, but I agree with you that it adds some scope which we might not want to add now.
Still, even though I gave those suggestions, I think they wouldn't be a game changer, because we fetch only one order there, so the differences shouldn't be significant versus a situation where we'd have to loop over 100 orders.

I recorded a video to describe what I am seeing and what I think we need to do.
https://watch.screencastify.com/v/Wm2n3lW41EfiqMpAxT0M

I hadn't read @kchelstowski's comments before the video but we're basically saying the same thing

I know, but I don't return the whole order object to the view, but just the part, that we really need there (this is why I wrote "with all the fields etc")

So I agree with @kchelstowski on the problem but I'm not sure if we need separate command objects just yet.

The performance problems with the ajax requests are probably all bottled up in the template GSPs. Some of those are likely causing tons of N+1 queries. A more thoughtful approach would be to rewrite each of those to gather just the data needed for the page, hopefully in a single query.

I imagine there would be at least two methods

Order getOrderSummary(String id)

Order getOrderDetails(String id)

that queries the database (including joins) to gather all of the data needed for that particular tab.

If that's not enough we can add other methods as data gatherers for the other closures.

Order getOrderItemStatuses(String id)

Order getOrderItemDetails(String id)

etc

Hopefully, only one of these methods will be the expensive one but I'm hoping we're able to limit it use to just the tabs that require the expensive view-backed data (i.e. invoice or status data that is taking a long time to load).

Assuming we can get everything we need packed into the Order object (or a map) then we can leave the command object for later.

For now, the important part is just trying to add some service methods that pull as much data as possible in one query. And honestly, I just need the summary tab to load in under 1s right now. Taking a little longer for subsequent tabs is fine until we have time to tackle the underlying slowness.

kchelstowski · 2023-09-05T15:26:17Z

grails-app/domain/org/pih/warehouse/invoice/Invoice.groovy

+    boolean getIsRegularInvoice() {
+        return invoiceType?.code == InvoiceTypeCode.INVOICE
+    }
+


I know you've just followed the convention, but I think this would be a nice tech debt topic for the future to discuss whether boolean names should start with the is prefix, or it should be reserved for the getter.
Groovy is specific in this case, because we'll access the field without having to use get prefix, but I know in Java the "best practices" are NOT to use is for boolean field name and use it for the getter.

This is not the topic to discuss under this PR, but just wanted to point it out maybe for the future.

I know you've just followed the convention, but I think this would be a nice tech debt topic for the future to discuss whether boolean names should start with the is prefix

I agree we should have a tech huddle about this to enumerate the conventions and document the one we want to standardize on.

I always forget what the correct convention for booleans is, but I prefer the Groovy/Grails convention of invoking the getter method through a property access so I think I like the way @awalkowiak did it above.

Boolean getIsApprovalRequired() { } if (requisition.origin.isApprovalRequired) { ... }

vs

Boolean IsApprovalRequired() { ... } if (requisition.isApprovalRequired()) { ... }

but I know in Java the "best practices" are NOT to use is for boolean field name and use it for the getter.

Unless I'm misunderstanding, Java does provide a design pattern for boolean properties that uses "is".

The slight difference is that Java is allowing for a is() method while get getIs() method allows us to use the Groovy getter convention.

Unless I'm misunderstanding, Java does provide a design pattern for boolean properties that uses "is".

Yes, but they use it for the getter, not the for the property itself. Even the jackson tool when serializing an object would return a boolean without the is.
Assuming you'd have a property:

private boolean isValid;

and if we returned this object from the API, it'd return it like:

{ "valid": true, }

The problem with naming the boolean using the is prefix is that naming the getter as getIs feels weird.

Once again - in Groovy we don't have that much problem with that, because don't have to use getters, but to access the properties directly, so we don't have to do: something.getIsValid(), but something.isValid, so it's fine.

https://stackoverflow.com/questions/5322648/for-a-boolean-field-what-is-the-naming-convention-for-its-getter-setter

jmiranda

comment below

jmiranda · 2023-09-11T22:02:48Z

grails-app/controllers/org/pih/warehouse/order/OrderController.groovy

+    def orderComments = {
+        Order order = Order.get(params.id)
+        render(template: "orderComments", model: [orderInstance: order])
+    }


I recorded a video to describe what I am seeing and what I think we need to do.
https://watch.screencastify.com/v/Wm2n3lW41EfiqMpAxT0M

I hadn't read @kchelstowski's comments before the video but we're basically saying the same thing

I know, but I don't return the whole order object to the view, but just the part, that we really need there (this is why I wrote "with all the fields etc")

So I agree with @kchelstowski on the problem but I'm not sure if we need separate command objects just yet.

The performance problems with the ajax requests are probably all bottled up in the template GSPs. Some of those are likely causing tons of N+1 queries. A more thoughtful approach would be to rewrite each of those to gather just the data needed for the page, hopefully in a single query.

I imagine there would be at least two methods

Order getOrderSummary(String id)

Order getOrderDetails(String id)

that queries the database (including joins) to gather all of the data needed for that particular tab.

If that's not enough we can add other methods as data gatherers for the other closures.

Order getOrderItemStatuses(String id)

Order getOrderItemDetails(String id)

etc

Hopefully, only one of these methods will be the expensive one but I'm hoping we're able to limit it use to just the tabs that require the expensive view-backed data (i.e. invoice or status data that is taking a long time to load).

Assuming we can get everything we need packed into the Order object (or a map) then we can leave the command object for later.

For now, the important part is just trying to add some service methods that pull as much data as possible in one query. And honestly, I just need the summary tab to load in under 1s right now. Taking a little longer for subsequent tabs is fine until we have time to tackle the underlying slowness.

jmiranda

The domain changes are making me a little uncomfortable as I don't know what side effects those have. Not sure what to do about that, but I guess we could have a tech huddle to discuss in more detail.

jmiranda · 2023-09-14T21:03:56Z

grails-app/domain/org/pih/warehouse/invoice/Invoice.groovy

+    boolean getIsRegularInvoice() {
+        return invoiceType?.code == InvoiceTypeCode.INVOICE
+    }
+


I know you've just followed the convention, but I think this would be a nice tech debt topic for the future to discuss whether boolean names should start with the is prefix

I agree we should have a tech huddle about this to enumerate the conventions and document the one we want to standardize on.

I always forget what the correct convention for booleans is, but I prefer the Groovy/Grails convention of invoking the getter method through a property access so I think I like the way @awalkowiak did it above.

Boolean getIsApprovalRequired() { } if (requisition.origin.isApprovalRequired) { ... }

vs

Boolean IsApprovalRequired() { ... } if (requisition.isApprovalRequired()) { ... }

but I know in Java the "best practices" are NOT to use is for boolean field name and use it for the getter.

Unless I'm misunderstanding, Java does provide a design pattern for boolean properties that uses "is".

The slight difference is that Java is allowing for a is() method while get getIs() method allows us to use the Groovy getter convention.

jmiranda · 2023-09-14T21:04:41Z

grails-app/domain/org/pih/warehouse/invoice/InvoiceItem.groovy

@@ -58,7 +59,7 @@ class InvoiceItem implements Serializable {
    User createdBy
    User updatedBy

-    static belongsTo = [invoice: Invoice]
+    static belongsTo = [Invoice, ShipmentItem, OrderAdjustment, OrderItem]


This worries me a tiny bit. Do we know what impact this is going to have?

@jmiranda could you elaborate here on why this worries you? I had to put it here in order to be able to add it in hasMany relation on OrderItem, ShipmentItem, and OrderAdjustment (which is crucial for this performance and code overall improvements)

It could generate more SQL than we're expecting and could consequently have unintended consequences on performance.

I think you should be able to define invoice item with invoice as a parent and hasMany on other domains without the need to define cascade behavior (which is what belongsTo is doing).

https://stackoverflow.com/questions/23905638/grails-domain-class-hasone-hasmany-without-belongsto

I assume you're encountering a Grails error without this. So post that here.

To resolve this just let me know

if it's possible to include back references on each of these. And if not, what is the error you're getting from Grails.

Also does this add any unnecessary SQL queries when retrieving invoice items. Does it go through each of those associations (in either case, with or without back reference).

awalkowiak · 2023-09-21T14:07:33Z

grails-app/controllers/org/pih/warehouse/order/OrderController.groovy

+        Order order = Order.get(params.id)
+        def orderItems = order?.orderItems
+        render(template: "orderSummary", model: [
+            isPurchaseOrder: order.isPurchaseOrder,


@jmiranda I started moving things to the controller that are needed by the template and since there are used properties from order and order items I decided to leave it as is -> fetch order and pull order items (and other required data) from it instead of fetching only order items.

Ok. I think we can refactor this a bit more when we move to Grails 3. As @kchelstowski mentioned this might look better as queries defined in the data service.

This is the one change I would like to see so we can move this in that direction when the time comes

def orderSummary = { Map orderSummary = orderService.getOrderSummary(String id) render render(template: "orderSummary", model: orderSummary) }

with the order summary method looking something like this. And since we know that we're just using this method for reading data we can add in some extra savings.

@Transactional(readOnly=true) Map getOrderSummary(String id) { Order order = Order.read(params.id) return [ isPurchaseOrder: order.isPurchaseOrder isPutawayOrder: order.isPutawayOrder, orderItems: orderItems, anySupplierCode: orderItems?.any { it.productSupplier?.supplierCode }, anyManufacturerName: orderItems?.any { it.productSupplier?.manufacturerName }, anyManufacturerCode: orderItems?.any { it.productSupplier?.manufacturerCode }, currencyCode: order.currencyCode ?: grailsApplication.config.openboxes.locale.defaultCurrencyCode, subtotal: order.subtotal, totalAdjustments: order.totalAdjustments, total: order.total ] }

This gives us a clean / DRY way of retrieving the data that can be refactored in the future. We'll also have an easier time unit testing and performance tuning in the future since all of the data retrieval is centralized in a single service method.

This is one step away from @kchelstowski's command object approach. And I don't see us needing that because the map works well enough as a DTO. The command object would be necessary if we then needed to add more methods and validation to the object.

awalkowiak · 2023-09-21T14:07:39Z

grails-app/controllers/org/pih/warehouse/order/OrderController.groovy

+        Order order = Order.get(params.id)
+        def orderItems = order?.orderItems
+        render(template: "orderSummary", model: [
+            isPurchaseOrder: order.isPurchaseOrder,


@jmiranda I started moving things to the controller that are needed by the template and since there are used properties from order and order items I decided to leave it as is -> fetch order and pull order items (and other required data) from it instead of fetching only order items.

awalkowiak · 2023-09-21T14:08:47Z

grails-app/views/order/show.gsp

@@ -306,6 +326,21 @@
                $(currentRow).hide();
              });
            }
+
+            function fetchOrderItemsDerivedStatus() {


@jmiranda Moving this to async improved the loading/rendering times by another ~2s

awalkowiak · 2023-09-21T14:11:16Z

@kchelstowski @jmiranda I added another improvement, please take a look:

moved order items derived status calculation to async action
reduced amount of unnecessary actions (looking at orderItems.any { ... } once instead on each item) on "Items summary" tab (initial tab)

jmiranda

We should go over this one in a tech huddle.

jmiranda · 2023-09-23T16:12:06Z

grails-app/domain/org/pih/warehouse/invoice/InvoiceItem.groovy

@@ -58,7 +59,7 @@ class InvoiceItem implements Serializable {
    User createdBy
    User updatedBy

-    static belongsTo = [invoice: Invoice]
+    static belongsTo = [Invoice, ShipmentItem, OrderAdjustment, OrderItem]


It could generate more SQL than we're expecting and could consequently have unintended consequences on performance.

I think you should be able to define invoice item with invoice as a parent and hasMany on other domains without the need to define cascade behavior (which is what belongsTo is doing).

https://stackoverflow.com/questions/23905638/grails-domain-class-hasone-hasmany-without-belongsto

I assume you're encountering a Grails error without this. So post that here.

jmiranda · 2023-09-23T16:13:54Z

grails-app/views/order/show.gsp

@@ -306,6 +326,21 @@
                $(currentRow).hide();
              });
            }
+
+            function fetchOrderItemsDerivedStatus() {


jmiranda · 2023-09-23T16:20:16Z

grails-app/controllers/org/pih/warehouse/order/OrderController.groovy

+        Order order = Order.get(params.id)
+        def orderItems = order?.orderItems
+        render(template: "orderSummary", model: [
+            isPurchaseOrder: order.isPurchaseOrder,


Ok. I think we can refactor this a bit more when we move to Grails 3. As @kchelstowski mentioned this might look better as queries defined in the data service.

This is the one change I would like to see so we can move this in that direction when the time comes

def orderSummary = { Map orderSummary = orderService.getOrderSummary(String id) render render(template: "orderSummary", model: orderSummary) }

with the order summary method looking something like this. And since we know that we're just using this method for reading data we can add in some extra savings.

@Transactional(readOnly=true) Map getOrderSummary(String id) { Order order = Order.read(params.id) return [ isPurchaseOrder: order.isPurchaseOrder isPutawayOrder: order.isPutawayOrder, orderItems: orderItems, anySupplierCode: orderItems?.any { it.productSupplier?.supplierCode }, anyManufacturerName: orderItems?.any { it.productSupplier?.manufacturerName }, anyManufacturerCode: orderItems?.any { it.productSupplier?.manufacturerCode }, currencyCode: order.currencyCode ?: grailsApplication.config.openboxes.locale.defaultCurrencyCode, subtotal: order.subtotal, totalAdjustments: order.totalAdjustments, total: order.total ] }

This gives us a clean / DRY way of retrieving the data that can be refactored in the future. We'll also have an easier time unit testing and performance tuning in the future since all of the data retrieval is centralized in a single service method.

This is one step away from @kchelstowski's command object approach. And I don't see us needing that because the map works well enough as a DTO. The command object would be necessary if we then needed to add more methods and validation to the object.

jmiranda · 2023-09-23T16:45:09Z

grails-app/controllers/org/pih/warehouse/order/OrderController.groovy

+            orderItems: orderItems,
+            anySupplierCode: orderItems?.any { it.productSupplier?.supplierCode },
+            anyManufacturerName: orderItems?.any { it.productSupplier?.manufacturerName },
+            anyManufacturerCode: orderItems?.any { it.productSupplier?.manufacturerCode },


I don't love this "any*" thing. If we need to do it this way, let's just grab "any" product supplier once and use the supplierCode, manufacturerName, manufacturerCode from that supplier.

jmiranda · 2023-09-23T16:51:45Z

grails-app/controllers/org/pih/warehouse/order/OrderController.groovy

+            currencyCode: order.currencyCode ?: grailsApplication.config.openboxes.locale.defaultCurrencyCode,
+            subtotal: order.subtotal,
+            totalAdjustments: order.totalAdjustments,
+            total: order.total,


This could be pushed off until later if we know that this is not a huge performance issue, but ...

... consider looking into some performance improvements for these three "properties" as well. I'm guessing this could at least be combined into a single method that pulls all of the data once and then returns the total, subtotal, and adjustments.

Map getOrderCosts() { // retrieve data necessary to calculate subtotal and adjustments return [ subtotal: subtotal, adjustments: adjustments, total: subtotal + adjustments ] }

jmiranda · 2023-09-23T16:53:42Z

grails-app/controllers/org/pih/warehouse/order/OrderController.groovy

+            isPutawayOrder: order.isPutawayOrder,
+            orderItems: orderItems,
+            currencyCode: order.currencyCode ?: grailsApplication.config.openboxes.locale.defaultCurrencyCode,
+            total: order.total,


Would love to see a getOrderItemSummary method here same as above.

This model will inform decisions about what the API should look like.

jmiranda · 2023-09-23T16:58:09Z

grails-app/domain/org/pih/warehouse/order/OrderAdjustment.groovy

@@ -59,9 +58,13 @@ class OrderAdjustment implements Serializable {

    static belongsTo = [order: Order, orderItem: OrderItem]

+    static hasMany = [invoiceItems: InvoiceItem]


I don't love adding new relationships that don't make sense. I don't understand why we want to do this. We're creating a new join table? How are we populating that join table?

I'm clearing missing something but here so I suggest we move this to a tech huddle.

It is not new. It was already on the InvoiceItem before as:
static hasMany = [shipmentItems: ShipmentItem, orderItems: OrderItem, orderAdjustments: OrderAdjustment]
and there are already join tables for these, these were just not used on the order items, order adjustments, and shipment items.

jmiranda

We have agreed on next steps.

jmiranda · 2023-09-25T14:41:42Z

grails-app/domain/org/pih/warehouse/invoice/InvoiceItem.groovy

@@ -58,7 +59,7 @@ class InvoiceItem implements Serializable {
    User createdBy
    User updatedBy

-    static belongsTo = [invoice: Invoice]
+    static belongsTo = [Invoice, ShipmentItem, OrderAdjustment, OrderItem]


To resolve this just let me know

if it's possible to include back references on each of these. And if not, what is the error you're getting from Grails.

Also does this add any unnecessary SQL queries when retrieving invoice items. Does it go through each of those associations (in either case, with or without back reference).

jmiranda · 2023-09-25T14:52:41Z

grails-app/controllers/org/pih/warehouse/order/OrderController.groovy

+            isPutawayOrder: order.isPutawayOrder,
+            orderItems: orderItems,
+            currencyCode: order.currencyCode ?: grailsApplication.config.openboxes.locale.defaultCurrencyCode,
+            total: order.total,


This model will inform decisions about what the API should look like.

jmiranda · 2023-09-25T14:58:22Z

We also decided to push this to the develop branch so we get a full regression test cycle run on it for 0.8.23.

jmiranda

See comments for requested changes. Once these are done I think we'll be good to merge.

jmiranda · 2023-10-06T03:45:16Z

grails-app/controllers/org/pih/warehouse/invoice/InvoiceController.groovy

@@ -64,6 +67,11 @@ class InvoiceController {
        def invoiceInstance = Invoice.get(params.id)
        if (invoiceInstance) {
            try {
+                invoiceInstance.invoiceItems?.each { InvoiceItem invoiceItem ->


Seeing this again really worries me. For now can you at least move lines 70-75 to the InvoiceService and just call invoiceService.deleteInvoice(invoice) here.

With the new deleteInvoiceItem(InvoiceItem invoiceItem) method suggested below ... this new method InvoiceService.deleteInvoice(Invoice) should look like this

void deleteInvoice(Invoice invoice) { invoice.invoiceItems.each { InvoiceItem invoiceItem -> deleteInvoiceItem(invoiceItem) } }

jmiranda · 2023-10-06T03:49:11Z

grails-app/services/org/pih/warehouse/invoice/InvoiceService.groovy

@@ -186,6 +186,9 @@ class InvoiceService {
        InvoiceItem invoiceItem = InvoiceItem.get(id)
        Invoice invoice = invoiceItem.invoice
        invoice.removeFromInvoiceItems(invoiceItem)
+        invoiceItem.orderItems?.each { OrderItem it -> it.removeFromInvoiceItems(invoiceItem) }
+        invoiceItem.orderAdjustments?.each { OrderAdjustment it -> it.removeFromInvoiceItems(invoiceItem) }
+        invoiceItem.shipmentItems?.each { ShipmentItem it -> it.removeFromInvoiceItems(invoiceItem) }


Move this to a new method. Add some error handling if it makes you feel better i.e. null check on invoiceItem

void deleteInvoiceItem(InvoiceItem invoiceItem) { invoiceItem.orderItems?.each { OrderItem it -> it.removeFromInvoiceItems(invoiceItem) } invoiceItem.orderAdjustments?.each { OrderAdjustment it -> it.removeFromInvoiceItems(invoiceItem) } invoiceItem.shipmentItems?.each { ShipmentItem it -> it.removeFromInvoiceItems(invoiceItem) } invoiceItem.delete() }

jmiranda · 2023-10-06T03:54:14Z

grails-app/views/order/_orderSummary.gsp

                            <td>
                                <div class="tag ${orderItem?.canceled ? 'tag-danger' : ''}">
-                                    <format:metadata obj="${!orderItem?.canceled && orderItemsDerivedStatus[orderItem?.id] ? orderItemsDerivedStatus[orderItem?.id] : orderItem?.orderItemStatusCode?.name()}"/>
+                                    <span class="${orderItem?.id}">${g.message(code: 'default.loading.label')}</span>


This feels odd. Why not use id=${orderItem.id} and class could be orderItemStatus or something like that.

@jmiranda It's done on purpose because the derived status is used on the first and second tabs and item id is used to find elements and replace them. If I'd use an id property here instead of class, then after entering the second tab only ids on the first tab (hidden at the moment) would be replaced with statuses and the second tab would be hanging with "loading..." labels, because jquery would replace the first id it found and would be happy about it.

- add proper hasMany relations on OrderItem, OrderAdjustment and ShipmentItem - remove getInvoiceItems methods with poor performance

…status

after recent domain changes

jmiranda

LGTM

* OBPIH-5847 Fix invoice items relations - add proper hasMany relations on OrderItem, OrderAdjustment and ShipmentItem - remove getInvoiceItems methods with poor performance * OBPIH-5847 Render tabs asynchronously on order show page * OBPIH-5847 Render order items derived status asynchronously * OBPIH-5847 Add methods in order service for getting summary and item status * OBPIH-5847 Fix deleting invoice items and invoices after recent domain changes

awalkowiak requested review from jmiranda, drodzewicz and kchelstowski September 5, 2023 15:05

kchelstowski reviewed Sep 5, 2023

View reviewed changes

awalkowiak changed the base branch from develop to release/0.8.22-hotfix3 September 8, 2023 08:10

awalkowiak marked this pull request as ready for review September 8, 2023 08:10

jmiranda reviewed Sep 11, 2023

View reviewed changes

jmiranda reviewed Sep 14, 2023

View reviewed changes

awalkowiak commented Sep 21, 2023

View reviewed changes

jmiranda reviewed Sep 23, 2023

View reviewed changes

jmiranda added the tech-huddle label Sep 23, 2023

jmiranda reviewed Sep 25, 2023

View reviewed changes

awalkowiak changed the base branch from release/0.8.22-hotfix3 to develop October 2, 2023 12:24

jmiranda requested changes Oct 6, 2023

View reviewed changes

awalkowiak added 4 commits October 6, 2023 11:22

OBPIH-5847 Fix invoice items relations

16141f7

- add proper hasMany relations on OrderItem, OrderAdjustment and ShipmentItem - remove getInvoiceItems methods with poor performance

OBPIH-5847 Render tabs asynchronously on order show page

2cdf907

OBPIH-5847 Render order items derived status asynchronously

a050227

OBPIH-5847 Add methods in order service for getting summary and item …

15ae351

…status

awalkowiak force-pushed the OBPIH-5847 branch from 2729230 to e00deb2 Compare October 6, 2023 11:07

OBPIH-5847 Fix deleting invoice items and invoices

d68c603

after recent domain changes

awalkowiak force-pushed the OBPIH-5847 branch from e00deb2 to d68c603 Compare October 6, 2023 11:09

jmiranda approved these changes Oct 9, 2023

View reviewed changes

awalkowiak merged commit 5ab86b2 into develop Oct 9, 2023
3 checks passed

awalkowiak deleted the OBPIH-5847 branch October 9, 2023 07:18

kchelstowski mentioned this pull request Dec 5, 2023

OBPIH-5990 Able to delete POSM when at least pending invoice exists #4400

Merged

		@@ -59,9 +58,13 @@ class OrderAdjustment implements Serializable {

		static belongsTo = [order: Order, orderItem: OrderItem]

		static hasMany = [invoiceItems: InvoiceItem]

OBPIH-5847 Fix poor performance on PO show page #4266

OBPIH-5847 Fix poor performance on PO show page #4266

Conversation

awalkowiak commented Sep 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmiranda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmiranda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awalkowiak commented Sep 21, 2023 • edited

jmiranda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmiranda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmiranda commented Sep 25, 2023

jmiranda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmiranda left a comment

Choose a reason for hiding this comment

awalkowiak commented Sep 21, 2023 •

edited