You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacopo Cappellato <ja...@hotwaxmedia.com> on 2010/02/04 16:52:51 UTC

Re: svn commit: r896855 - in /ofbiz/trunk/applications/order/src/org/ofbiz/order: order/OrderServices.java shoppingcart/ShoppingCart.java shoppingcart/ShoppingCartItem.java shoppingcart/ShoppingCartServices.java

Hi David,

when you create a purchase order the modification below is preventing the OrderItemShipGroupAssoc records to be created (and they are now required by the new shipment screens I implemented a few weeks ago).
I guess that you did it because it was causing the ship groups to be destroyed when an order is edited, am I right?
What is the best way to fix it?
We could add the following code:
            cart.setItemShipGroupQty(newItem, quantity, 0);
right after the call newItem.setQuantity or we could modify the setQuantity method to always call cart.setItemShipGroupQty

Could you please help? I don't want to change something that will have an impact of what you fixed.

Thank you,

Jacopo

On Jan 7, 2010, at 1:01 PM, jonesde@apache.org wrote:

> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java?rev=896855&r1=896854&r2=896855&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java (original)
> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java Thu Jan  7 11:59:48 2010
> @@ -239,7 +239,7 @@
>         newItem.setCancelBackOrderDate(cancelBackOrderDate != null ? cancelBackOrderDate : cart.getCancelBackOrderDate());
> 
>         try {
> -            newItem.setQuantity(quantity, dispatcher, cart, true);
> +            newItem.setQuantity(quantity, dispatcher, cart, true, false);
>         } catch (CartItemModifyException e) {
>             cart.removeCartItem(cart.getItemIndex(newItem), dispatcher);
>             cart.clearItemShipInfo(newItem);


Re: svn commit: r896855 - in /ofbiz/trunk/applications/order/src/org/ofbiz/order: order/OrderServices.java shoppingcart/ShoppingCart.java shoppingcart/ShoppingCartItem.java shoppingcart/ShoppingCartServices.java

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
On Feb 4, 2010, at 5:30 PM, David E Jones wrote:

> 
> Jacopo,
> 
> I'm not sure what to say without looking into it more.
> 
> I must say that when I found out that there was code that explicitly destroyed data with no way of getting it back I was pretty surprised! It seems to be based on an assumption that once ship group data is entered then there will be no more changing of any item quantities.
> 
> Is that a safe assumption, even in the checkout?
> 
> This code is really messy... and looking at it now I'm not sure why there the OrderItemShipGroup records need to be destroyed in order to get the OrderItemShipGroupAssoc records in place. I guess one way or another that needs to change, and perhaps what you proposed with the cart.setItemShipGroupQty is the way to go (and leaving the false on the setQuantity, or actually I think it might be nice to get rid of the data killing code altogether and do it in a better way).

I think that for now I will leave the false flag and I will add an explicit cart.setItemShipGroupQty right after the call to setQuantity in order to create the ship group assoc.
But I agree that the code is a bit messed up.

Thank you, David

Jacopo

> 
> -David
> 
> 
> On Feb 4, 2010, at 9:52 AM, Jacopo Cappellato wrote:
> 
>> Hi David,
>> 
>> when you create a purchase order the modification below is preventing the OrderItemShipGroupAssoc records to be created (and they are now required by the new shipment screens I implemented a few weeks ago).
>> I guess that you did it because it was causing the ship groups to be destroyed when an order is edited, am I right?
>> What is the best way to fix it?
>> We could add the following code:
>>           cart.setItemShipGroupQty(newItem, quantity, 0);
>> right after the call newItem.setQuantity or we could modify the setQuantity method to always call cart.setItemShipGroupQty
>> 
>> Could you please help? I don't want to change something that will have an impact of what you fixed.
>> 
>> Thank you,
>> 
>> Jacopo
>> 
>> On Jan 7, 2010, at 1:01 PM, jonesde@apache.org wrote:
>> 
>>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java?rev=896855&r1=896854&r2=896855&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java (original)
>>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java Thu Jan  7 11:59:48 2010
>>> @@ -239,7 +239,7 @@
>>>       newItem.setCancelBackOrderDate(cancelBackOrderDate != null ? cancelBackOrderDate : cart.getCancelBackOrderDate());
>>> 
>>>       try {
>>> -            newItem.setQuantity(quantity, dispatcher, cart, true);
>>> +            newItem.setQuantity(quantity, dispatcher, cart, true, false);
>>>       } catch (CartItemModifyException e) {
>>>           cart.removeCartItem(cart.getItemIndex(newItem), dispatcher);
>>>           cart.clearItemShipInfo(newItem);
>> 
> 


Re: svn commit: r896855 - in /ofbiz/trunk/applications/order/src/org/ofbiz/order: order/OrderServices.java shoppingcart/ShoppingCart.java shoppingcart/ShoppingCartItem.java shoppingcart/ShoppingCartServices.java

Posted by David E Jones <de...@me.com>.
Jacopo,

I'm not sure what to say without looking into it more.

I must say that when I found out that there was code that explicitly destroyed data with no way of getting it back I was pretty surprised! It seems to be based on an assumption that once ship group data is entered then there will be no more changing of any item quantities.

Is that a safe assumption, even in the checkout?

This code is really messy... and looking at it now I'm not sure why there the OrderItemShipGroup records need to be destroyed in order to get the OrderItemShipGroupAssoc records in place. I guess one way or another that needs to change, and perhaps what you proposed with the cart.setItemShipGroupQty is the way to go (and leaving the false on the setQuantity, or actually I think it might be nice to get rid of the data killing code altogether and do it in a better way).

-David


On Feb 4, 2010, at 9:52 AM, Jacopo Cappellato wrote:

> Hi David,
> 
> when you create a purchase order the modification below is preventing the OrderItemShipGroupAssoc records to be created (and they are now required by the new shipment screens I implemented a few weeks ago).
> I guess that you did it because it was causing the ship groups to be destroyed when an order is edited, am I right?
> What is the best way to fix it?
> We could add the following code:
>            cart.setItemShipGroupQty(newItem, quantity, 0);
> right after the call newItem.setQuantity or we could modify the setQuantity method to always call cart.setItemShipGroupQty
> 
> Could you please help? I don't want to change something that will have an impact of what you fixed.
> 
> Thank you,
> 
> Jacopo
> 
> On Jan 7, 2010, at 1:01 PM, jonesde@apache.org wrote:
> 
>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java?rev=896855&r1=896854&r2=896855&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java (original)
>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java Thu Jan  7 11:59:48 2010
>> @@ -239,7 +239,7 @@
>>        newItem.setCancelBackOrderDate(cancelBackOrderDate != null ? cancelBackOrderDate : cart.getCancelBackOrderDate());
>> 
>>        try {
>> -            newItem.setQuantity(quantity, dispatcher, cart, true);
>> +            newItem.setQuantity(quantity, dispatcher, cart, true, false);
>>        } catch (CartItemModifyException e) {
>>            cart.removeCartItem(cart.getItemIndex(newItem), dispatcher);
>>            cart.clearItemShipInfo(newItem);
>