Hi everybody -
I saw this in ShoppingCartItem: +++ applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java (working copy) /** Sets the quantity for the item and validates the change in quantity, etc */ public void setQuantity(double quantity, LocalDispatcher dispatcher, ShoppingCart cart, boolean triggerExternalOps, boolean resetShipGroup) throws CartItemModifyException { this.setQuantity((int) quantity, dispatcher, cart, triggerExternalOps, resetShipGroup, !cart.getOrderType().equals("PURCHASE_ORDER")); } Why is the last parameter, which is "updateProductPrice", disabled when the cart is a purchase order? Was it because of the way purchase order items' prices are set? Because it is causing a bug now when a purchase order's quantities are modified: the price is being set to the old quantity, before the modification. A simple solution would be to change !cart.getOrderType().equals("PURCHASE_ORDER") to true, so that prices are always updated. But would this break any functionality? Si _______________________________________________ Dev mailing list [hidden email] http://lists.ofbiz.org/mailman/listinfo/dev |
Si,
I wrote that code, and committed it just before you've committed a patch to add the isModifiedPrice to the shopping cart items: is the bug you are seeing related to this? Unfortunately, I cannot have a look at this right now (I'll be back on this the day after tomorrow) so I'm trying to explain why I did it in that way and how it should work. Before my modification, it was not possible, when calling the setQuantity() method, to tell the system to not run the updatePrice method; so I've added a new argument to the setQuantity method called updateProductPrice; for backward compatibility I've changed the original setQuantity methods to pass in a default value (true) for this argument. About the code snippet in your mail: I've hadded the code !cart.getOrderType().equals("PURCHASE_ORDER") because that code was originally inside the setQuantity method: if (!cart.getOrderType().equals("PURCHASE_ORDER")) { updatePrice(...); } (If I well remember the code was this). That said, I think that with this modification I've not changed the original behaviour... however I could be wrong so if you need to change smething in it, please do it :-) Thanks, Jacopo Si Chen wrote: > Hi everybody - > > I saw this in ShoppingCartItem: > +++ > applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java > (working copy) > /** Sets the quantity for the item and validates the change in > quantity, etc */ > public void setQuantity(double quantity, LocalDispatcher dispatcher, > ShoppingCart cart, boolean triggerExternalOps, boolean resetShipGroup) > throws CartItemModifyException { > this.setQuantity((int) quantity, dispatcher, cart, > triggerExternalOps, resetShipGroup, > !cart.getOrderType().equals("PURCHASE_ORDER")); > } > > Why is the last parameter, which is "updateProductPrice", disabled when > the cart is a purchase order? Was it because of the way purchase order > items' prices are set? > > Because it is causing a bug now when a purchase order's quantities are > modified: the price is being set to the old quantity, before the > modification. > > A simple solution would be to change > !cart.getOrderType().equals("PURCHASE_ORDER") to true, so that prices > are always updated. But would this break any functionality? > > Si > > _______________________________________________ > Dev mailing list > [hidden email] > http://lists.ofbiz.org/mailman/listinfo/dev > _______________________________________________ Dev mailing list [hidden email] http://lists.ofbiz.org/mailman/listinfo/dev |
Jacopo,
Yes. It is related to this. Why do you want the system not to run updatePrice when calling setQuantity? Was it just because of the old code that was inside setQuantity? Or did you notice a bug by having it there? Si Jacopo Cappellato wrote: > Si, > > I wrote that code, and committed it just before you've committed a > patch to add the isModifiedPrice to the shopping cart items: is the > bug you are seeing related to this? > Unfortunately, I cannot have a look at this right now (I'll be back on > this the day after tomorrow) so I'm trying to explain why I did it in > that way and how it should work. > > Before my modification, it was not possible, when calling the > setQuantity() method, to tell the system to not run the updatePrice > method; so I've added a new argument to the setQuantity method called > updateProductPrice; for backward compatibility I've changed the > original setQuantity methods to pass in a default value (true) for > this argument. > About the code snippet in your mail: I've hadded the code > !cart.getOrderType().equals("PURCHASE_ORDER") because that code was > originally inside the setQuantity method: > > if (!cart.getOrderType().equals("PURCHASE_ORDER")) { > updatePrice(...); > } > > (If I well remember the code was this). > > That said, I think that with this modification I've not changed the > original behaviour... however I could be wrong so if you need to > change smething in it, please do it :-) > > Thanks, > > Jacopo > > > Si Chen wrote: > >> Hi everybody - >> >> I saw this in ShoppingCartItem: >> +++ >> applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java >> (working copy) >> /** Sets the quantity for the item and validates the change in >> quantity, etc */ >> public void setQuantity(double quantity, LocalDispatcher >> dispatcher, ShoppingCart cart, boolean triggerExternalOps, boolean >> resetShipGroup) throws CartItemModifyException { >> this.setQuantity((int) quantity, dispatcher, cart, >> triggerExternalOps, resetShipGroup, >> !cart.getOrderType().equals("PURCHASE_ORDER")); >> } >> >> Why is the last parameter, which is "updateProductPrice", disabled >> when the cart is a purchase order? Was it because of the way >> purchase order items' prices are set? >> >> Because it is causing a bug now when a purchase order's quantities >> are modified: the price is being set to the old quantity, before the >> modification. >> >> A simple solution would be to change >> !cart.getOrderType().equals("PURCHASE_ORDER") to true, so that prices >> are always updated. But would this break any functionality? >> >> Si >> >> _______________________________________________ >> Dev mailing list >> [hidden email] >> http://lists.ofbiz.org/mailman/listinfo/dev >> > > > _______________________________________________ > Dev mailing list > [hidden email] > http://lists.ofbiz.org/mailman/listinfo/dev > _______________________________________________ Dev mailing list [hidden email] http://lists.ofbiz.org/mailman/listinfo/dev |
Si,
there are situations in which there is the need to create a shopping cart item (and set the quantity of it) passing to it a unit price: for example when a quote item is put in the cart. So I've added the new setQuantity(...) method with the new updatePrice flag. However, the default behaviour should be the same unless you explicitly call the new method. If not, please let me know and I'll fix it. Jacopo Si Chen wrote: > Jacopo, > > Yes. It is related to this. Why do you want the system not to run > updatePrice when calling setQuantity? Was it just because of the old > code that was inside setQuantity? Or did you notice a bug by having it > there? > > Si > > Jacopo Cappellato wrote: > >> Si, >> >> I wrote that code, and committed it just before you've committed a >> patch to add the isModifiedPrice to the shopping cart items: is the >> bug you are seeing related to this? >> Unfortunately, I cannot have a look at this right now (I'll be back on >> this the day after tomorrow) so I'm trying to explain why I did it in >> that way and how it should work. >> >> Before my modification, it was not possible, when calling the >> setQuantity() method, to tell the system to not run the updatePrice >> method; so I've added a new argument to the setQuantity method called >> updateProductPrice; for backward compatibility I've changed the >> original setQuantity methods to pass in a default value (true) for >> this argument. >> About the code snippet in your mail: I've hadded the code >> !cart.getOrderType().equals("PURCHASE_ORDER") because that code was >> originally inside the setQuantity method: >> >> if (!cart.getOrderType().equals("PURCHASE_ORDER")) { >> updatePrice(...); >> } >> >> (If I well remember the code was this). >> >> That said, I think that with this modification I've not changed the >> original behaviour... however I could be wrong so if you need to >> change smething in it, please do it :-) >> >> Thanks, >> >> Jacopo >> >> >> Si Chen wrote: >> >>> Hi everybody - >>> >>> I saw this in ShoppingCartItem: >>> +++ >>> applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java >>> (working copy) >>> /** Sets the quantity for the item and validates the change in >>> quantity, etc */ >>> public void setQuantity(double quantity, LocalDispatcher >>> dispatcher, ShoppingCart cart, boolean triggerExternalOps, boolean >>> resetShipGroup) throws CartItemModifyException { >>> this.setQuantity((int) quantity, dispatcher, cart, >>> triggerExternalOps, resetShipGroup, >>> !cart.getOrderType().equals("PURCHASE_ORDER")); >>> } >>> >>> Why is the last parameter, which is "updateProductPrice", disabled >>> when the cart is a purchase order? Was it because of the way >>> purchase order items' prices are set? >>> >>> Because it is causing a bug now when a purchase order's quantities >>> are modified: the price is being set to the old quantity, before the >>> modification. >>> >>> A simple solution would be to change >>> !cart.getOrderType().equals("PURCHASE_ORDER") to true, so that prices >>> are always updated. But would this break any functionality? >>> >>> Si >>> >>> _______________________________________________ >>> Dev mailing list >>> [hidden email] >>> http://lists.ofbiz.org/mailman/listinfo/dev >>> >> >> >> _______________________________________________ >> Dev mailing list >> [hidden email] >> http://lists.ofbiz.org/mailman/listinfo/dev >> > > _______________________________________________ > Dev mailing list > [hidden email] > http://lists.ofbiz.org/mailman/listinfo/dev > _______________________________________________ Dev mailing list [hidden email] http://lists.ofbiz.org/mailman/listinfo/dev |
Ok everybody -
I can fix it by changing OrderServices.java line 3897 from cartItem.setQuantity(qty.doubleValue(), dispatcher, cart, true, false); to cartItem.setQuantity(qty.doubleValue(), dispatcher, cart, true, false, true); So instead of calling your method which currently does not change prices for new purchase orders, I'll just call the next method and force it to update prices no matter what. Right now it's already updating prices for sales orders, so this change will just fix purchase orders which will take care of it... Si Jacopo Cappellato wrote: > Si, > > there are situations in which there is the need to create a shopping > cart item (and set the quantity of it) passing to it a unit price: for > example when a quote item is put in the cart. > So I've added the new setQuantity(...) method with the new updatePrice > flag. > However, the default behaviour should be the same unless you > explicitly call the new method. If not, please let me know and I'll > fix it. > > Jacopo > > > > Si Chen wrote: > >> Jacopo, >> >> Yes. It is related to this. Why do you want the system not to run >> updatePrice when calling setQuantity? Was it just because of the old >> code that was inside setQuantity? Or did you notice a bug by having >> it there? >> >> Si >> >> Jacopo Cappellato wrote: >> >>> Si, >>> >>> I wrote that code, and committed it just before you've committed a >>> patch to add the isModifiedPrice to the shopping cart items: is the >>> bug you are seeing related to this? >>> Unfortunately, I cannot have a look at this right now (I'll be back >>> on this the day after tomorrow) so I'm trying to explain why I did >>> it in that way and how it should work. >>> >>> Before my modification, it was not possible, when calling the >>> setQuantity() method, to tell the system to not run the updatePrice >>> method; so I've added a new argument to the setQuantity method >>> called updateProductPrice; for backward compatibility I've changed >>> the original setQuantity methods to pass in a default value (true) >>> for this argument. >>> About the code snippet in your mail: I've hadded the code >>> !cart.getOrderType().equals("PURCHASE_ORDER") because that code was >>> originally inside the setQuantity method: >>> >>> if (!cart.getOrderType().equals("PURCHASE_ORDER")) { >>> updatePrice(...); >>> } >>> >>> (If I well remember the code was this). >>> >>> That said, I think that with this modification I've not changed the >>> original behaviour... however I could be wrong so if you need to >>> change smething in it, please do it :-) >>> >>> Thanks, >>> >>> Jacopo >>> >>> >>> Si Chen wrote: >>> >>>> Hi everybody - >>>> >>>> I saw this in ShoppingCartItem: >>>> +++ >>>> applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java >>>> (working copy) >>>> /** Sets the quantity for the item and validates the change in >>>> quantity, etc */ >>>> public void setQuantity(double quantity, LocalDispatcher >>>> dispatcher, ShoppingCart cart, boolean triggerExternalOps, boolean >>>> resetShipGroup) throws CartItemModifyException { >>>> this.setQuantity((int) quantity, dispatcher, cart, >>>> triggerExternalOps, resetShipGroup, >>>> !cart.getOrderType().equals("PURCHASE_ORDER")); >>>> } >>>> >>>> Why is the last parameter, which is "updateProductPrice", disabled >>>> when the cart is a purchase order? Was it because of the way >>>> purchase order items' prices are set? >>>> >>>> Because it is causing a bug now when a purchase order's quantities >>>> are modified: the price is being set to the old quantity, before >>>> the modification. >>>> >>>> A simple solution would be to change >>>> !cart.getOrderType().equals("PURCHASE_ORDER") to true, so that >>>> prices are always updated. But would this break any functionality? >>>> >>>> Si >>>> >>>> _______________________________________________ >>>> Dev mailing list >>>> [hidden email] >>>> http://lists.ofbiz.org/mailman/listinfo/dev >>>> >>> >>> >>> _______________________________________________ >>> Dev mailing list >>> [hidden email] >>> http://lists.ofbiz.org/mailman/listinfo/dev >>> >> >> _______________________________________________ >> Dev mailing list >> [hidden email] >> http://lists.ofbiz.org/mailman/listinfo/dev >> > > > _______________________________________________ > Dev mailing list > [hidden email] > http://lists.ofbiz.org/mailman/listinfo/dev > _______________________________________________ Dev mailing list [hidden email] http://lists.ofbiz.org/mailman/listinfo/dev |
Si,
thanks for fixing it. Now that I have more time, I'm reviewing the code I've committed in rev 5730 and yes, I agree with you that setting the updatePrice flag in that way was not a good idea... so I'd like to fix (as you've already suggested in your previous mail) the setQuantity(double quantity, LocalDispatcher dispatcher, ShoppingCart cart, boolean triggerExternalOps, boolean resetShipGroup) method to always set to true the updatePrice flag when calling the new setQuantity(double quantity, LocalDispatcher dispatcher, ShoppingCart cart, boolean triggerExternalOps, boolean resetShipGroup, boolean updateProductPrice) method (instead of setting it to true only for sales orders). The only thing I really don't understand is in which way (before rev 5730) the setQuantity methods could set the price in purchase orders since in the setQuantity method, when calling the updatePrice method, there was the following code: if (!cart.getOrderType().equals("PURCHASE_ORDER")) { this.updatePrice(dispatcher, cart); } Could you help me with this? Thanks so much for your time and sorry for the time you've spent fixing this issue. Jacopo Si Chen wrote: > Ok everybody - > > I can fix it by changing OrderServices.java line 3897 from > cartItem.setQuantity(qty.doubleValue(), dispatcher, > cart, true, false); > to > cartItem.setQuantity(qty.doubleValue(), dispatcher, > cart, true, false, true); > > So instead of calling your method which currently does not change prices > for new purchase orders, I'll just call the next method and force it to > update prices no matter what. Right now it's already updating prices > for sales orders, so this change will just fix purchase orders which > will take care of it... > > Si > > Jacopo Cappellato wrote: > >> Si, >> >> there are situations in which there is the need to create a shopping >> cart item (and set the quantity of it) passing to it a unit price: for >> example when a quote item is put in the cart. >> So I've added the new setQuantity(...) method with the new updatePrice >> flag. >> However, the default behaviour should be the same unless you >> explicitly call the new method. If not, please let me know and I'll >> fix it. >> >> Jacopo >> >> >> >> Si Chen wrote: >> >>> Jacopo, >>> >>> Yes. It is related to this. Why do you want the system not to run >>> updatePrice when calling setQuantity? Was it just because of the old >>> code that was inside setQuantity? Or did you notice a bug by having >>> it there? >>> >>> Si >>> >>> Jacopo Cappellato wrote: >>> >>>> Si, >>>> >>>> I wrote that code, and committed it just before you've committed a >>>> patch to add the isModifiedPrice to the shopping cart items: is the >>>> bug you are seeing related to this? >>>> Unfortunately, I cannot have a look at this right now (I'll be back >>>> on this the day after tomorrow) so I'm trying to explain why I did >>>> it in that way and how it should work. >>>> >>>> Before my modification, it was not possible, when calling the >>>> setQuantity() method, to tell the system to not run the updatePrice >>>> method; so I've added a new argument to the setQuantity method >>>> called updateProductPrice; for backward compatibility I've changed >>>> the original setQuantity methods to pass in a default value (true) >>>> for this argument. >>>> About the code snippet in your mail: I've hadded the code >>>> !cart.getOrderType().equals("PURCHASE_ORDER") because that code was >>>> originally inside the setQuantity method: >>>> >>>> if (!cart.getOrderType().equals("PURCHASE_ORDER")) { >>>> updatePrice(...); >>>> } >>>> >>>> (If I well remember the code was this). >>>> >>>> That said, I think that with this modification I've not changed the >>>> original behaviour... however I could be wrong so if you need to >>>> change smething in it, please do it :-) >>>> >>>> Thanks, >>>> >>>> Jacopo >>>> >>>> >>>> Si Chen wrote: >>>> >>>>> Hi everybody - >>>>> >>>>> I saw this in ShoppingCartItem: >>>>> +++ >>>>> applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java >>>>> (working copy) >>>>> /** Sets the quantity for the item and validates the change in >>>>> quantity, etc */ >>>>> public void setQuantity(double quantity, LocalDispatcher >>>>> dispatcher, ShoppingCart cart, boolean triggerExternalOps, boolean >>>>> resetShipGroup) throws CartItemModifyException { >>>>> this.setQuantity((int) quantity, dispatcher, cart, >>>>> triggerExternalOps, resetShipGroup, >>>>> !cart.getOrderType().equals("PURCHASE_ORDER")); >>>>> } >>>>> >>>>> Why is the last parameter, which is "updateProductPrice", disabled >>>>> when the cart is a purchase order? Was it because of the way >>>>> purchase order items' prices are set? >>>>> >>>>> Because it is causing a bug now when a purchase order's quantities >>>>> are modified: the price is being set to the old quantity, before >>>>> the modification. >>>>> >>>>> A simple solution would be to change >>>>> !cart.getOrderType().equals("PURCHASE_ORDER") to true, so that >>>>> prices are always updated. But would this break any functionality? >>>>> >>>>> Si _______________________________________________ Dev mailing list [hidden email] http://lists.ofbiz.org/mailman/listinfo/dev |
Jacopo,
No problem. I'm not sure why in older versions of the code, prices weren't updated for purchase orders either. It'd be nice in the future to have more comments that say why things are done. I think, though, it is safe to make the change you propose, since all the other methods of setQuantity automatically use updateProductPrice=true Si Jacopo Cappellato wrote: > Si, > > thanks for fixing it. > Now that I have more time, I'm reviewing the code I've committed in > rev 5730 and yes, I agree with you that setting the updatePrice flag > in that way was not a good idea... so I'd like to fix (as you've > already suggested in your previous mail) the > > setQuantity(double quantity, LocalDispatcher dispatcher, ShoppingCart > cart, boolean triggerExternalOps, boolean resetShipGroup) > > method to always set to true the updatePrice flag when calling the new > > setQuantity(double quantity, LocalDispatcher dispatcher, ShoppingCart > cart, boolean triggerExternalOps, boolean resetShipGroup, boolean > updateProductPrice) > > method (instead of setting it to true only for sales orders). > > The only thing I really don't understand is in which way (before rev > 5730) the setQuantity methods could set the price in purchase orders > since in the setQuantity method, when calling the updatePrice method, > there was the following code: > > if (!cart.getOrderType().equals("PURCHASE_ORDER")) { > this.updatePrice(dispatcher, cart); > } > > Could you help me with this? > > Thanks so much for your time and sorry for the time you've spent > fixing this issue. > > Jacopo > > > > Si Chen wrote: > >> Ok everybody - >> >> I can fix it by changing OrderServices.java line 3897 from >> cartItem.setQuantity(qty.doubleValue(), >> dispatcher, cart, true, false); >> to >> cartItem.setQuantity(qty.doubleValue(), >> dispatcher, cart, true, false, true); >> >> So instead of calling your method which currently does not change >> prices for new purchase orders, I'll just call the next method and >> force it to update prices no matter what. Right now it's already >> updating prices for sales orders, so this change will just fix >> purchase orders which will take care of it... >> >> Si >> >> Jacopo Cappellato wrote: >> >>> Si, >>> >>> there are situations in which there is the need to create a shopping >>> cart item (and set the quantity of it) passing to it a unit price: >>> for example when a quote item is put in the cart. >>> So I've added the new setQuantity(...) method with the new >>> updatePrice flag. >>> However, the default behaviour should be the same unless you >>> explicitly call the new method. If not, please let me know and I'll >>> fix it. >>> >>> Jacopo >>> >>> >>> >>> Si Chen wrote: >>> >>>> Jacopo, >>>> >>>> Yes. It is related to this. Why do you want the system not to run >>>> updatePrice when calling setQuantity? Was it just because of the >>>> old code that was inside setQuantity? Or did you notice a bug by >>>> having it there? >>>> >>>> Si >>>> >>>> Jacopo Cappellato wrote: >>>> >>>>> Si, >>>>> >>>>> I wrote that code, and committed it just before you've committed a >>>>> patch to add the isModifiedPrice to the shopping cart items: is >>>>> the bug you are seeing related to this? >>>>> Unfortunately, I cannot have a look at this right now (I'll be >>>>> back on this the day after tomorrow) so I'm trying to explain why >>>>> I did it in that way and how it should work. >>>>> >>>>> Before my modification, it was not possible, when calling the >>>>> setQuantity() method, to tell the system to not run the >>>>> updatePrice method; so I've added a new argument to the >>>>> setQuantity method called updateProductPrice; for backward >>>>> compatibility I've changed the original setQuantity methods to >>>>> pass in a default value (true) for this argument. >>>>> About the code snippet in your mail: I've hadded the code >>>>> !cart.getOrderType().equals("PURCHASE_ORDER") because that code >>>>> was originally inside the setQuantity method: >>>>> >>>>> if (!cart.getOrderType().equals("PURCHASE_ORDER")) { >>>>> updatePrice(...); >>>>> } >>>>> >>>>> (If I well remember the code was this). >>>>> >>>>> That said, I think that with this modification I've not changed >>>>> the original behaviour... however I could be wrong so if you need >>>>> to change smething in it, please do it :-) >>>>> >>>>> Thanks, >>>>> >>>>> Jacopo >>>>> >>>>> >>>>> Si Chen wrote: >>>>> >>>>>> Hi everybody - >>>>>> >>>>>> I saw this in ShoppingCartItem: >>>>>> +++ >>>>>> applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java >>>>>> (working copy) >>>>>> /** Sets the quantity for the item and validates the change >>>>>> in quantity, etc */ >>>>>> public void setQuantity(double quantity, LocalDispatcher >>>>>> dispatcher, ShoppingCart cart, boolean triggerExternalOps, >>>>>> boolean resetShipGroup) throws CartItemModifyException { >>>>>> this.setQuantity((int) quantity, dispatcher, cart, >>>>>> triggerExternalOps, resetShipGroup, >>>>>> !cart.getOrderType().equals("PURCHASE_ORDER")); >>>>>> } >>>>>> >>>>>> Why is the last parameter, which is "updateProductPrice", >>>>>> disabled when the cart is a purchase order? Was it because of >>>>>> the way purchase order items' prices are set? >>>>>> >>>>>> Because it is causing a bug now when a purchase order's >>>>>> quantities are modified: the price is being set to the old >>>>>> quantity, before the modification. >>>>>> >>>>>> A simple solution would be to change >>>>>> !cart.getOrderType().equals("PURCHASE_ORDER") to true, so that >>>>>> prices are always updated. But would this break any functionality? >>>>>> >>>>>> Si >>>>> > > _______________________________________________ > Dev mailing list > [hidden email] > http://lists.ofbiz.org/mailman/listinfo/dev > _______________________________________________ Dev mailing list [hidden email] http://lists.ofbiz.org/mailman/listinfo/dev |
Free forum by Nabble | Edit this page |