You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Stephen Rufle <sr...@salmonllc.com> on 2008/12/22 23:51:24 UTC

ShoppingCart.addOrIncreaseItem

I would like to add a parameter to ShoppingCart.addOrIncreaseItem(... ,
boolean checkItemsExists)

I see that the method signature is already pretty long. I would like to
know if instead would it be a better idea to pass along a Map object
around instead?

If there is already an existing effort to reduce the number of
parameters I would like to see how you are dealing with it.



Re: ShoppingCart.addOrIncreaseItem

Posted by Jacques Le Roux <ja...@les7arts.com>.
Yes, in such case 2 solutions (actually 3 but as you want 2 patches only 3. It's better to separate the 2 concerns anyway ;o) : 
* Just let us known as David suggested and we will take care of the spaces that should not be there
* Open a 1st issue with the spaces changes and a 2d with your specific changes

Obviously David is experienced !

Jacques

From: "Stephen Rufle" <sr...@salmonllc.com>
> In eclipse or using the svn command line client, how do I generate a
> first patch of just the white spaces changes and then the second one for
> what I wanted to show as my actual patch. When I have done it previously
> I used the trunk revision as the base of the diff. I see I need to use
> my whitespace patched file as the base, but do not know how because it
> is not yet on the trunk so when I do the patch generation the tools
> always generate at patch with both whitespace changes  AND  my actual
> code changes.
> 
> Jacques Le Roux wrote:
>> There are both pro and cons with any of these tools (Tortoise,
>> Eclipse, svn directly, patch, etc.). Only experience taught when to
>> use one of them. I prefer Tortoise in most cases but this implies
>> Windows.
>> In this case creating 2 patches is a good idea IMO
>>
>> Jacques
>>
>> From: "David E Jones" <da...@hotwaxmedia.com>
>>>
>>> It makes it easier to review if you avoid formatting changes.
>>> Usually  a note along with the patch that the file had tabs in it
>>> would help  the committer to know that it needs updating.
>>>
>>> BTW, the best way to generate patches is to use SVN itself,
>>> something  like "svn diff > mychanges.patch". I think various people
>>> use Eclipse  for patches, mainly because I see comments about
>>> problems they have  with it (seems mostly related to unexpected
>>> behavior).
>>>
>>> -David
>>>
>>>
>>> On Dec 23, 2008, at 10:19 AM, Stephen Rufle wrote:
>>>
>>>> Added checkItemExists to ShoppingCart.addOrIncreaseItem(... , boolean
>>>> checkItemsExists)
>>>>
>>>> I wanted to have the group review my patch before I created a JIRA. 
>>>> I am
>>>> new to patching so I see in my patch that it is changing tabs to 
>>>> spaces
>>>> and adding the new code and applies to multiple files all in one 
>>>> patch,
>>>> previously I have sent patches that were file specific.
>>>>
>>>> I am using eclipse to generate patches, but I would have liked to
>>>> generate two. One for the tab to spaces and another to show what I
>>>> added. Any advice would be appropriated.
>>>>
>>>> If the patch is good as is I will create a JIRA and attach.
>>>>
>>>> David E Jones wrote:
>>>>>
>>>>> My favorite current proposal is to super-simplify that method and 
>>>>> then
>>>>> on the ShoppingCartItem object that it returns just call additional
>>>>> methods to add additional data. That way now, and in the future, we
>>>>> only have to add setters to ShoppingCartItem to support new data 
>>>>> there.
>>>>>
>>>>> -David
>>>>>
>>>>>
>>>>> On Dec 22, 2008, at 3:51 PM, Stephen Rufle wrote:
>>>>>
>>>>>> I would like to add a parameter to 
>>>>>> ShoppingCart.addOrIncreaseItem(... ,
>>>>>> boolean checkItemsExists)
>>>>>>
>>>>>> I see that the method signature is already pretty long. I would 
>>>>>> like to
>>>>>> know if instead would it be a better idea to pass along a Map object
>>>>>> around instead?
>>>>>>
>>>>>> If there is already an existing effort to reduce the number of
>>>>>> parameters I would like to see how you are dealing with it.
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>> -- 
>>>> Stephen P Rufle
>>>> srufle@salmonllc.com
>>>> H1:480-626-8022
>>>> H2:480-802-7173
>>>> Yahoo IM: stephen_rufle
>>>> AOL IM: stephen1rufle
>>>>
>>>> ### Eclipse Workspace Patch 1.0
>>>> #P ofbiz
>>>> Index: applications/order/src/org/ofbiz/order/shoppingcart/
>>>> ShoppingCart.java
>>>> ===================================================================
>>>> --- applications/order/src/org/ofbiz/order/shoppingcart/
>>>> ShoppingCart.java (revision 729034)
>>>> +++ applications/order/src/org/ofbiz/order/shoppingcart/
>>>> ShoppingCart.java (working copy)
>>>> @@ -453,14 +453,14 @@
>>>>
>>>>        return  addOrIncreaseItem (productId ,selectedAmountDbl
>>>> ,quantity,reservStart,reservLengthDbl,reservPersonsDbl,
>>>>                         null
>>>> ,null,shipBeforeDate,shipAfterDate,features,attributes,prodCatalogId,
>>>> -                
>>>> configWrapper,itemType,itemGroupNumber,parentProductId,dispatcher);
>>>> +                
>>>> configWrapper,itemType,itemGroupNumber,parentProductId,dispatcher, 
>>>> true);
>>>>     }
>>>>
>>>>     /** add rental (with accommodation) item to cart  */
>>>>     public int addOrIncreaseItem(String productId, Double 
>>>> selectedAmountDbl, double quantity, Timestamp reservStart, Double
>>>> reservLengthDbl, Double reservPersonsDbl,
>>>>                String accommodationMapId, String accommodationSpotId,
>>>>             Timestamp shipBeforeDate, Timestamp shipAfterDate, Map 
>>>> features, Map attributes, String prodCatalogId,
>>>> -            ProductConfigWrapper configWrapper, String itemType, 
>>>> String itemGroupNumber, String parentProductId, LocalDispatcher 
>>>> dispatcher) throws CartItemModifyException, ItemNotFoundException {
>>>> +            ProductConfigWrapper configWrapper, String itemType, 
>>>> String itemGroupNumber, String parentProductId, LocalDispatcher 
>>>> dispatcher, boolean checkItemsExists) throws 
>>>> CartItemModifyException, ItemNotFoundException {
>>>>         if (isReadOnlyCart()) {
>>>>            throw new CartItemModifyException("Cart items cannot be 
>>>> changed");
>>>>         }
>>>> Index: applications/order/src/org/ofbiz/order/shoppingcart/
>>>> ShoppingCartEvents.java
>>>> ===================================================================
>>>> --- applications/order/src/org/ofbiz/order/shoppingcart/
>>>> ShoppingCartEvents.java (revision 729034)
>>>> +++ applications/order/src/org/ofbiz/order/shoppingcart/
>>>> ShoppingCartEvents.java (working copy)
>>>> @@ -156,12 +156,12 @@
>>>>         if (paramMap.containsKey("ADD_PRODUCT_ID")) {
>>>>             productId = (String) paramMap.remove("ADD_PRODUCT_ID");
>>>>         } else if (paramMap.containsKey("add_product_id")) {
>>>> -        Object object = paramMap.remove("add_product_id");
>>>> -        try{
>>>> -        productId = (String) object;
>>>> -        }catch(ClassCastException e){
>>>> -        productId = (String)((List)object).get(0);
>>>> -        }
>>>> +            Object object = paramMap.remove("add_product_id");
>>>> +            try{
>>>> +                productId = (String) object;
>>>> +            }catch(ClassCastException e){
>>>> +                productId = (String)((List)object).get(0);
>>>> +            }
>>>>         }
>>>>         if (paramMap.containsKey("PRODUCT_ID")) {
>>>>             parentProductId = (String) paramMap.remove("PRODUCT_ID");
>>>> @@ -241,23 +241,23 @@
>>>>         //Check for virtual products
>>>>         if (ProductWorker.isVirtual(delegator, productId)) {
>>>>
>>>> - if  ("VV_FEATURETREE
>>>> ".equals(ProductWorker.getProductvirtualVariantMethod(delegator, 
>>>> productId))) {
>>>> - // get the selected features.
>>>> - List <String> selectedFeatures = new LinkedList<String>();
>>>> -     java.util.Enumeration paramNames =  request.getParameterNames();
>>>> -     while(paramNames.hasMoreElements()) {
>>>> -     String paramName = (String)paramNames.nextElement();
>>>> -     if (paramName.startsWith("FT")) {
>>>> -     selectedFeatures.add(request.getParameterValues(paramName) [0]);
>>>> -     }
>>>> -     }
>>>> -     -     // check if features are selected
>>>> -     if (UtilValidate.isEmpty(selectedFeatures)) {
>>>> - request.setAttribute("product_id", productId);
>>>> - request .setAttribute ("_EVENT_MESSAGE_ ",UtilProperties
>>>> .getMessage
>>>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>>>> - return "product";
>>>> -     }
>>>> +            if  ("VV_FEATURETREE
>>>> ".equals(ProductWorker.getProductvirtualVariantMethod(delegator, 
>>>> productId))) {
>>>> +                // get the selected features.
>>>> +                List <String> selectedFeatures = new 
>>>> LinkedList<String>();
>>>> +                java.util.Enumeration paramNames = 
>>>> request.getParameterNames();
>>>> +                while(paramNames.hasMoreElements()) {
>>>> +                    String paramName = 
>>>> (String)paramNames.nextElement();
>>>> +                    if (paramName.startsWith("FT")) {
>>>> +                        
>>>> selectedFeatures.add(request.getParameterValues(paramName)[0]);
>>>> +                    }
>>>> +                }
>>>> +
>>>> +                // check if features are selected
>>>> +                if (UtilValidate.isEmpty(selectedFeatures)) {
>>>> +                    request.setAttribute("product_id", productId);
>>>> +                     request .setAttribute ("_EVENT_MESSAGE_
>>>> ",UtilProperties .getMessage
>>>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>>>> +                    return "product";
>>>> +                }
>>>>
>>>>                 String variantProductId = 
>>>> ProductWorker.getVariantFromFeatureTree(productId,
>>>> selectedFeatures,  delegator);
>>>>                 if (UtilValidate.isNotEmpty(variantProductId)) {
>>>> @@ -268,12 +268,12 @@
>>>>                     return "product";
>>>>                 }
>>>>
>>>> - } else {
>>>> - request.setAttribute("product_id", productId);
>>>> - request .setAttribute ("_EVENT_MESSAGE_ ",UtilProperties
>>>> .getMessage
>>>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>>>> - return "product";
>>>> - }
>>>> - }
>>>> +            } else {
>>>> +                request.setAttribute("product_id", productId);
>>>> +                 request .setAttribute ("_EVENT_MESSAGE_
>>>> ",UtilProperties .getMessage
>>>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>>>> +                return "product";
>>>> +            }
>>>> +        }
>>>>
>>>>         // get the override price
>>>>         if (paramMap.containsKey("PRICE")) {
>>>> @@ -323,7 +323,7 @@
>>>>             }
>>>>
>>>>             if (reservStart != null && reservEnd != null) {
>>>> -            reservLength = new 
>>>> Double(UtilDateTime.getInterval(reservStart,reservEnd)/86400000);
>>>> +                reservLength = new 
>>>> Double(UtilDateTime.getInterval(reservStart,reservEnd)/86400000);
>>>>             }
>>>>
>>>>             if (reservStart != null && 
>>>> paramMap.containsKey("reservLength")) {
>>>> @@ -362,8 +362,8 @@
>>>>
>>>>             //check accommodation for reservations
>>>>             if((paramMap.containsKey("accommodationMapId")) && 
>>>> (paramMap.containsKey("accommodationSpotId"))){
>>>> -            accommodationMapId = (String) 
>>>> paramMap.remove("accommodationMapId");
>>>> -            accommodationSpotId = (String) 
>>>> paramMap.remove("accommodationSpotId");
>>>> +                accommodationMapId = (String) 
>>>> paramMap.remove("accommodationMapId");
>>>> +                accommodationSpotId = (String) 
>>>> paramMap.remove("accommodationSpotId");
>>>>             }
>>>>         }
>>>>
>>>> @@ -541,7 +541,7 @@
>>>>         result = cartHelper.addToCart(catalogId, shoppingListId, 
>>>> shoppingListItemSeqId, productId, productCategoryId,
>>>>                 itemType, itemDescription, price, amount, quantity, 
>>>> reservStart, reservLength, reservPersons,
>>>>                 accommodationMapId, accommodationSpotId,
>>>> -                shipBeforeDate, shipAfterDate, configWrapper, 
>>>> itemGroupNumber, paramMap, parentProductId);
>>>> +                shipBeforeDate, shipAfterDate, configWrapper, 
>>>> itemGroupNumber, paramMap, parentProductId , true);
>>>>         controlDirective = processResult(result, request);
>>>>
>>>>         // Determine where to send the browser
>>>> @@ -549,9 +549,9 @@
>>>>             return "error";
>>>>         } else {
>>>>             if (cart.viewCartOnAdd()) {
>>>> -            return "viewcart";
>>>> +                return "viewcart";
>>>>             } else {
>>>> -            return "success";
>>>> +                return "success";
>>>>             }
>>>>         }
>>>>     }
>>>> @@ -940,11 +940,11 @@
>>>>         Locale locale = UtilHttp.getLocale(request);
>>>>
>>>>         if (UtilValidate.isEmpty(alternateGwpProductId)) {
>>>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>>> .getMessage (resource_error
>>>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpProductIdPassed", 
>>>> locale));
>>>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>>> .getMessage (resource_error
>>>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpProductIdPassed", 
>>>> locale));
>>>>             return "error";
>>>>         }
>>>>         if (UtilValidate.isEmpty(alternateGwpLineStr)) {
>>>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>>> .getMessage (resource_error
>>>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpLinePassed", locale));
>>>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>>> .getMessage (resource_error
>>>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpLinePassed", locale));
>>>>             return "error";
>>>>         }
>>>>
>>>> @@ -952,13 +952,13 @@
>>>>         try {
>>>>             alternateGwpLine = Integer.parseInt(alternateGwpLineStr);
>>>>         } catch (Exception e) {
>>>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>>> .getMessage (resource_error
>>>> ,"OrderCouldNotSelectAlternateGiftAlternateGwpLineIsNotAValidNumber
>>>> ", locale));
>>>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>>> .getMessage (resource_error
>>>> ,"OrderCouldNotSelectAlternateGiftAlternateGwpLineIsNotAValidNumber
>>>> ", locale));
>>>>             return "error";
>>>>         }
>>>>
>>>>         ShoppingCartItem cartLine = 
>>>> cart.findCartItem(alternateGwpLine);
>>>>         if (cartLine == null) {
>>>> -        request.setAttribute("_ERROR_MESSAGE_", "Could not select 
>>>> alternate gift, no cart line item found for #" + alternateGwpLine + 
>>>> ".");
>>>> +            request.setAttribute("_ERROR_MESSAGE_", "Could not 
>>>> select alternate gift, no cart line item found for #" +
>>>> alternateGwpLine + ".");
>>>>             return "error";
>>>>         }
>>>>
>>>> @@ -994,7 +994,7 @@
>>>>         int i;
>>>>
>>>>         if (UtilValidate.isEmpty(partyId) || roleTypeId.length < 1) {
>>>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined", 
>>>> locale));
>>>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined", 
>>>> locale));
>>>>             return "error";
>>>>         }
>>>>
>>>> @@ -1025,7 +1025,7 @@
>>>>         int i;
>>>>
>>>>         if (UtilValidate.isEmpty(partyId) || roleTypeId.length < 1) {
>>>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined", 
>>>> locale));
>>>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined", 
>>>> locale));
>>>>             return "error";
>>>>         }
>>>>
>>>> Index: applications/order/src/org/ofbiz/order/shoppingcart/
>>>> ShoppingCartHelper.java
>>>> ===================================================================
>>>> --- applications/order/src/org/ofbiz/order/shoppingcart/
>>>> ShoppingCartHelper.java (revision 729034)
>>>> +++ applications/order/src/org/ofbiz/order/shoppingcart/
>>>> ShoppingCartHelper.java (working copy)
>>>> @@ -107,7 +107,7 @@
>>>>         return 
>>>> addToCart(catalogId,shoppingListId,shoppingListItemSeqId,productId,
>>>>                 
>>>> productCategoryId,itemType,itemDescription,price,amount,quantity,
>>>>                  reservStart
>>>> ,reservLength,reservPersons,null,null,shipBeforeDate,shipAfterDate,
>>>> -                
>>>> configWrapper,itemGroupNumber,context,parentProductId);
>>>> +                
>>>> configWrapper,itemGroupNumber,context,parentProductId, true);
>>>>     }
>>>>
>>>>     /** Event to add an item to the shopping cart with 
>>>> accommodation. */
>>>> @@ -116,7 +116,7 @@
>>>>             Double price, Double amount, double quantity,
>>>>             java.sql.Timestamp reservStart, Double reservLength, 
>>>> Double reservPersons, String accommodationMapId,String
>>>> accommodationSpotId,
>>>>             java.sql.Timestamp shipBeforeDate, java.sql.Timestamp 
>>>> shipAfterDate,
>>>> -            ProductConfigWrapper configWrapper, String 
>>>> itemGroupNumber, Map context, String parentProductId) {
>>>> +            ProductConfigWrapper configWrapper, String 
>>>> itemGroupNumber, Map context, String parentProductId, boolean
>>>> checkItemsExists) {
>>>>         Map result = null;
>>>>         Map attributes = null;
>>>>         String pProductId = null;
>>>> @@ -234,7 +234,7 @@
>>>>
>>>>                        itemId = cart.addOrIncreaseItem(productId, 
>>>> amount, quantity, reservStart, reservLength,
>>>>                                                 reservPersons, 
>>>> accommodationMapId, accommodationSpotId, shipBeforeDate,
>>>> shipAfterDate, additionalFeaturesMap, attributes,
>>>> -                                                catalogId, 
>>>> configWrapper, itemType, itemGroupNumber, pProductId, dispatcher);
>>>> +                                                catalogId, 
>>>> configWrapper, itemType, itemGroupNumber, pProductId, dispatcher,
>>>> checkItemsExists);
>>>>
>>>>             } else {
>>>>                 itemId = cart.addNonProductItem(itemType, 
>>>> itemDescription, productCategoryId, price, quantity, attributes,
>>>> catalogId, itemGroupNumber, dispatcher);
>>>> @@ -678,16 +678,16 @@
>>>>                     } else if 
>>>> (parameterName.toUpperCase().startsWith("DESCRIPTION")) {
>>>>                         itemDescription = quantString;  // the 
>>>> quantString is actually the description if the field name starts
>>>> with DESCRIPTION
>>>>                     } else if 
>>>> (parameterName.startsWith("reservStart")) {
>>>> -                    if (quantString.length() ==0){
>>>> -                    // should have format: yyyy-mm-dd 
>>>> hh:mm:ss.fffffffff
>>>> -                    quantString += " 00:00:00.000000000";
>>>> -                    }
>>>> -                    if (item != null) {
>>>> -                    Timestamp reservStart = 
>>>> Timestamp.valueOf(quantString);
>>>> -                    item.setReservStart(reservStart);
>>>> -                    }
>>>> +                        if (quantString.length() ==0){
>>>> +                            // should have format: yyyy-mm-dd 
>>>> hh:mm:ss.fffffffff
>>>> +                            quantString += " 00:00:00.000000000";
>>>> +                        }
>>>> +                        if (item != null) {
>>>> +                            Timestamp reservStart = 
>>>> Timestamp.valueOf(quantString);
>>>> +                            item.setReservStart(reservStart);
>>>> +                        }
>>>>                     } else if 
>>>> (parameterName.startsWith("reservLength")) {
>>>> -                    if (item != null) {
>>>> +                        if (item != null) {
>>>>                             double reservLength = 
>>>> nf.parse(quantString).doubleValue();
>>>>                             item.setReservLength(reservLength);
>>>>                         }
>>>> Index: applications/order/src/org/ofbiz/order/shoppinglist/
>>>> ShoppingListEvents.java
>>>> ===================================================================
>>>> --- applications/order/src/org/ofbiz/order/shoppinglist/
>>>> ShoppingListEvents.java (revision 729034)
>>>> +++ applications/order/src/org/ofbiz/order/shoppinglist/
>>>> ShoppingListEvents.java (working copy)
>>>> @@ -138,7 +138,7 @@
>>>>             try {
>>>>                 cartIdInt = new Integer(items[i]);
>>>>             } catch (Exception e) {
>>>> -            Debug.logWarning(e,  UtilProperties .getMessage
>>>> (resource_error,"OrderIllegalCharacterInSelectedItemField",
>>>> cart.getLocale()), module);
>>>> +                Debug.logWarning(e,  UtilProperties .getMessage
>>>> (resource_error,"OrderIllegalCharacterInSelectedItemField",
>>>> cart.getLocale()), module);
>>>>             }
>>>>             if (cartIdInt != null) {
>>>>                 ShoppingCartItem item = 
>>>> cart.findCartItem(cartIdInt.intValue());
>>>> @@ -292,18 +292,18 @@
>>>>                 if (reservStart == null) {
>>>>                        cart.addOrIncreaseItem(productId, null, 
>>>> quantity.doubleValue(), null, null, null, null, null, null,
>>>> attributes, prodCatalogId, configWrapper, null, null, null, 
>>>> dispatcher);
>>>>                 }else{
>>>> -                    cart.addOrIncreaseItem(productId, null, 
>>>> quantity.doubleValue(), reservStart, reservLength, reservPersons,
>>>> null, null, null, null, null, attributes, prodCatalogId, 
>>>> configWrapper, null, null, null, dispatcher);
>>>> +                    cart.addOrIncreaseItem(productId, null, 
>>>> quantity.doubleValue(), reservStart, reservLength, reservPersons,
>>>> null, null, null, null, null, attributes, prodCatalogId, 
>>>> configWrapper, null, null, null, dispatcher, true);
>>>>                 }
>>>>                 Map messageMap = UtilMisc.toMap("productId", 
>>>> productId);
>>>>                 errMsg =  UtilProperties
>>>> .getMessage(resource,"shoppinglistevents.added_product_to_cart", 
>>>> messageMap, cart.getLocale());
>>>>                 eventMessage.append(errMsg + "\n");
>>>>             } catch (CartItemModifyException e) {
>>>> -            Debug.logWarning(e,  UtilProperties
>>>> .getMessage(resource_error,"OrderProblemsAddingItemFromListToCart",
>>>> cart.getLocale()));
>>>> +                Debug.logWarning(e,  UtilProperties
>>>> .getMessage(resource_error,"OrderProblemsAddingItemFromListToCart",
>>>> cart.getLocale()));
>>>>                 Map messageMap = UtilMisc.toMap("productId", 
>>>> productId);
>>>>                 errMsg =  UtilProperties .getMessage
>>>> (resource,"shoppinglistevents.problem_adding_product_to_cart", 
>>>> messageMap, cart.getLocale());
>>>>                 eventMessage.append(errMsg + "\n");
>>>>             } catch (ItemNotFoundException e) {
>>>> -            Debug.logWarning(e, 
>>>> UtilProperties.getMessage(resource_error,"OrderProductNotFound", 
>>>> cart.getLocale()));
>>>> +                Debug.logWarning(e, 
>>>> UtilProperties.getMessage(resource_error,"OrderProductNotFound", 
>>>> cart.getLocale()));
>>>>                 Map messageMap = UtilMisc.toMap("productId", 
>>>> productId);
>>>>                 errMsg =  UtilProperties .getMessage
>>>> (resource,"shoppinglistevents.problem_adding_product_to_cart", 
>>>> messageMap, cart.getLocale());
>>>>                 eventMessage.append(errMsg + "\n");
>>>> @@ -343,7 +343,7 @@
>>>>         try {
>>>>             result = dispatcher.runSync("updateShoppingListItem", 
>>>> serviceInMap);
>>>>         } catch (GenericServiceException e) {
>>>> -        String errMsg =  UtilProperties .getMessage
>>>> (ShoppingListEvents
>>>> .err_resource,"shoppingListEvents.error_calling_update", locale) + 
>>>> ": "  + e.toString();
>>>> +            String errMsg =  UtilProperties .getMessage
>>>> (ShoppingListEvents
>>>> .err_resource,"shoppingListEvents.error_calling_update", locale) + 
>>>> ": "  + e.toString();
>>>>             request.setAttribute("_ERROR_MESSAGE_", errMsg);
>>>>             String errorMsg = "Error calling the 
>>>> updateShoppingListItem in handleShoppingListItemVariant: " + 
>>>> e.toString();
>>>>             Debug.logError(e, errorMsg, module);
>>>> Index: applications/order/src/org/ofbiz/order/shoppinglist/
>>>> ShoppingListServices.java
>>>> ===================================================================
>>>> --- applications/order/src/org/ofbiz/order/shoppinglist/
>>>> ShoppingListServices.java (revision 729034)
>>>> +++ applications/order/src/org/ofbiz/order/shoppinglist/
>>>> ShoppingListServices.java (working copy)
>>>> @@ -416,7 +416,7 @@
>>>>      * @return
>>>>      */
>>>>     public static ShoppingCart makeShoppingListCart(LocalDispatcher 
>>>> dispatcher, GenericValue shoppingList, Locale locale) {
>>>> -    return makeShoppingListCart(null, dispatcher, shoppingList, 
>>>> locale); }
>>>> +        return makeShoppingListCart(null, dispatcher,
>>>> shoppingList,  locale); }
>>>>
>>>>     /**
>>>>      * Add a shoppinglist to an existing shoppingcart
>>>> @@ -451,20 +451,20 @@
>>>>             }
>>>>
>>>>             if (UtilValidate.isNotEmpty(items)) {
>>>> -            if (listCart == null) {
>>>> -            listCart = new ShoppingCart(delegator,  productStoreId,
>>>> locale, currencyUom);
>>>> -           
>>>> listCart.setOrderPartyId(shoppingList.getString("partyId"));
>>>> -            listCart
>>>> .setAutoOrderShoppingListId(shoppingList.getString("shoppingListId"));
>>>> -            } else {
>>>> -            if (!
>>>> listCart.getPartyId().equals(shoppingList.getString("partyId"))){
>>>> -            Debug.logError("CANNOT add shoppingList: " + 
>>>> shoppingList.getString("shoppingListId")
>>>> -            + " of partyId: " +  shoppingList.getString("partyId")
>>>> -            + " to a shoppingcart with a different  orderPartyId: "
>>>> -            + listCart.getPartyId(), module);
>>>> -                return listCart;
>>>> -            }
>>>> -            }
>>>> -            +                if (listCart == null) {
>>>> +                    listCart = new ShoppingCart(delegator, 
>>>> productStoreId, locale, currencyUom);
>>>> +                    
>>>> listCart.setOrderPartyId(shoppingList.getString("partyId"));
>>>> +                     listCart
>>>> .setAutoOrderShoppingListId(shoppingList.getString("shoppingListId"));
>>>> +                } else {
>>>> +                    if (!
>>>> listCart.getPartyId().equals(shoppingList.getString("partyId"))){
>>>> +                        Debug.logError("CANNOT add shoppingList: " 
>>>> + shoppingList.getString("shoppingListId")
>>>> +                                + " of partyId: " + 
>>>> shoppingList.getString("partyId")
>>>> +                                + " to a shoppingcart with a 
>>>> different orderPartyId: "
>>>> +                                + listCart.getPartyId(), module);
>>>> +                        return listCart;
>>>> +                    }
>>>> +                }
>>>> +
>>>>
>>>>                 Iterator i = items.iterator();
>>>>                 ProductConfigWrapper configWrapper = null;
>>>> @@ -502,7 +502,7 @@
>>>>                         Map attributes = 
>>>> UtilMisc.toMap("shoppingListId", listId, "shoppingListItemSeqId", 
>>>> itemId);
>>>>
>>>>                         try {
>>>> -                            listCart.addOrIncreaseItem(productId, 
>>>> null, quantity.doubleValue(), reservStart, reservLength,
>>>> reservPersons, null, null, null, null, null, attributes, null, 
>>>> configWrapper, null, null, null, dispatcher);
>>>> +                            listCart.addOrIncreaseItem(productId, 
>>>> null, quantity.doubleValue(), reservStart, reservLength,
>>>> reservPersons, null, null, null, null, null, attributes, null, 
>>>> configWrapper, null, null, null, dispatcher, true);
>>>>                         } catch (CartItemModifyException e) {
>>>>                             Debug.logError(e, "Unable to add
>>>> product  to List Cart - " + productId, module);
>>>>                         } catch (ItemNotFoundException e) {
>>>
>>
>>
>>
> 
> -- 
> Stephen P Rufle
> srufle@salmonllc.com
> H1:480-626-8022
> H2:480-802-7173
> Yahoo IM: stephen_rufle
> AOL IM: stephen1rufle
>

Re: ShoppingCart.addOrIncreaseItem

Posted by Stephen Rufle <sr...@salmonllc.com>.
In eclipse or using the svn command line client, how do I generate a
first patch of just the white spaces changes and then the second one for
what I wanted to show as my actual patch. When I have done it previously
I used the trunk revision as the base of the diff. I see I need to use
my whitespace patched file as the base, but do not know how because it
is not yet on the trunk so when I do the patch generation the tools
always generate at patch with both whitespace changes  AND  my actual
code changes.

Jacques Le Roux wrote:
> There are both pro and cons with any of these tools (Tortoise,
> Eclipse, svn directly, patch, etc.). Only experience taught when to
> use one of them. I prefer Tortoise in most cases but this implies
> Windows.
> In this case creating 2 patches is a good idea IMO
>
> Jacques
>
> From: "David E Jones" <da...@hotwaxmedia.com>
>>
>> It makes it easier to review if you avoid formatting changes.
>> Usually  a note along with the patch that the file had tabs in it
>> would help  the committer to know that it needs updating.
>>
>> BTW, the best way to generate patches is to use SVN itself,
>> something  like "svn diff > mychanges.patch". I think various people
>> use Eclipse  for patches, mainly because I see comments about
>> problems they have  with it (seems mostly related to unexpected
>> behavior).
>>
>> -David
>>
>>
>> On Dec 23, 2008, at 10:19 AM, Stephen Rufle wrote:
>>
>>> Added checkItemExists to ShoppingCart.addOrIncreaseItem(... , boolean
>>> checkItemsExists)
>>>
>>> I wanted to have the group review my patch before I created a JIRA. 
>>> I am
>>> new to patching so I see in my patch that it is changing tabs to 
>>> spaces
>>> and adding the new code and applies to multiple files all in one 
>>> patch,
>>> previously I have sent patches that were file specific.
>>>
>>> I am using eclipse to generate patches, but I would have liked to
>>> generate two. One for the tab to spaces and another to show what I
>>> added. Any advice would be appropriated.
>>>
>>> If the patch is good as is I will create a JIRA and attach.
>>>
>>> David E Jones wrote:
>>>>
>>>> My favorite current proposal is to super-simplify that method and 
>>>> then
>>>> on the ShoppingCartItem object that it returns just call additional
>>>> methods to add additional data. That way now, and in the future, we
>>>> only have to add setters to ShoppingCartItem to support new data 
>>>> there.
>>>>
>>>> -David
>>>>
>>>>
>>>> On Dec 22, 2008, at 3:51 PM, Stephen Rufle wrote:
>>>>
>>>>> I would like to add a parameter to 
>>>>> ShoppingCart.addOrIncreaseItem(... ,
>>>>> boolean checkItemsExists)
>>>>>
>>>>> I see that the method signature is already pretty long. I would 
>>>>> like to
>>>>> know if instead would it be a better idea to pass along a Map object
>>>>> around instead?
>>>>>
>>>>> If there is already an existing effort to reduce the number of
>>>>> parameters I would like to see how you are dealing with it.
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>> -- 
>>> Stephen P Rufle
>>> srufle@salmonllc.com
>>> H1:480-626-8022
>>> H2:480-802-7173
>>> Yahoo IM: stephen_rufle
>>> AOL IM: stephen1rufle
>>>
>>> ### Eclipse Workspace Patch 1.0
>>> #P ofbiz
>>> Index: applications/order/src/org/ofbiz/order/shoppingcart/
>>> ShoppingCart.java
>>> ===================================================================
>>> --- applications/order/src/org/ofbiz/order/shoppingcart/
>>> ShoppingCart.java (revision 729034)
>>> +++ applications/order/src/org/ofbiz/order/shoppingcart/
>>> ShoppingCart.java (working copy)
>>> @@ -453,14 +453,14 @@
>>>
>>>        return  addOrIncreaseItem (productId ,selectedAmountDbl
>>> ,quantity,reservStart,reservLengthDbl,reservPersonsDbl,
>>>                         null
>>> ,null,shipBeforeDate,shipAfterDate,features,attributes,prodCatalogId,
>>> -                
>>> configWrapper,itemType,itemGroupNumber,parentProductId,dispatcher);
>>> +                
>>> configWrapper,itemType,itemGroupNumber,parentProductId,dispatcher, 
>>> true);
>>>     }
>>>
>>>     /** add rental (with accommodation) item to cart  */
>>>     public int addOrIncreaseItem(String productId, Double 
>>> selectedAmountDbl, double quantity, Timestamp reservStart, Double
>>> reservLengthDbl, Double reservPersonsDbl,
>>>                String accommodationMapId, String accommodationSpotId,
>>>             Timestamp shipBeforeDate, Timestamp shipAfterDate, Map 
>>> features, Map attributes, String prodCatalogId,
>>> -            ProductConfigWrapper configWrapper, String itemType, 
>>> String itemGroupNumber, String parentProductId, LocalDispatcher 
>>> dispatcher) throws CartItemModifyException, ItemNotFoundException {
>>> +            ProductConfigWrapper configWrapper, String itemType, 
>>> String itemGroupNumber, String parentProductId, LocalDispatcher 
>>> dispatcher, boolean checkItemsExists) throws 
>>> CartItemModifyException, ItemNotFoundException {
>>>         if (isReadOnlyCart()) {
>>>            throw new CartItemModifyException("Cart items cannot be 
>>> changed");
>>>         }
>>> Index: applications/order/src/org/ofbiz/order/shoppingcart/
>>> ShoppingCartEvents.java
>>> ===================================================================
>>> --- applications/order/src/org/ofbiz/order/shoppingcart/
>>> ShoppingCartEvents.java (revision 729034)
>>> +++ applications/order/src/org/ofbiz/order/shoppingcart/
>>> ShoppingCartEvents.java (working copy)
>>> @@ -156,12 +156,12 @@
>>>         if (paramMap.containsKey("ADD_PRODUCT_ID")) {
>>>             productId = (String) paramMap.remove("ADD_PRODUCT_ID");
>>>         } else if (paramMap.containsKey("add_product_id")) {
>>> -        Object object = paramMap.remove("add_product_id");
>>> -        try{
>>> -        productId = (String) object;
>>> -        }catch(ClassCastException e){
>>> -        productId = (String)((List)object).get(0);
>>> -        }
>>> +            Object object = paramMap.remove("add_product_id");
>>> +            try{
>>> +                productId = (String) object;
>>> +            }catch(ClassCastException e){
>>> +                productId = (String)((List)object).get(0);
>>> +            }
>>>         }
>>>         if (paramMap.containsKey("PRODUCT_ID")) {
>>>             parentProductId = (String) paramMap.remove("PRODUCT_ID");
>>> @@ -241,23 +241,23 @@
>>>         //Check for virtual products
>>>         if (ProductWorker.isVirtual(delegator, productId)) {
>>>
>>> - if  ("VV_FEATURETREE
>>> ".equals(ProductWorker.getProductvirtualVariantMethod(delegator, 
>>> productId))) {
>>> - // get the selected features.
>>> - List <String> selectedFeatures = new LinkedList<String>();
>>> -     java.util.Enumeration paramNames =  request.getParameterNames();
>>> -     while(paramNames.hasMoreElements()) {
>>> -     String paramName = (String)paramNames.nextElement();
>>> -     if (paramName.startsWith("FT")) {
>>> -     selectedFeatures.add(request.getParameterValues(paramName) [0]);
>>> -     }
>>> -     }
>>> -     -     // check if features are selected
>>> -     if (UtilValidate.isEmpty(selectedFeatures)) {
>>> - request.setAttribute("product_id", productId);
>>> - request .setAttribute ("_EVENT_MESSAGE_ ",UtilProperties
>>> .getMessage
>>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>>> - return "product";
>>> -     }
>>> +            if  ("VV_FEATURETREE
>>> ".equals(ProductWorker.getProductvirtualVariantMethod(delegator, 
>>> productId))) {
>>> +                // get the selected features.
>>> +                List <String> selectedFeatures = new 
>>> LinkedList<String>();
>>> +                java.util.Enumeration paramNames = 
>>> request.getParameterNames();
>>> +                while(paramNames.hasMoreElements()) {
>>> +                    String paramName = 
>>> (String)paramNames.nextElement();
>>> +                    if (paramName.startsWith("FT")) {
>>> +                        
>>> selectedFeatures.add(request.getParameterValues(paramName)[0]);
>>> +                    }
>>> +                }
>>> +
>>> +                // check if features are selected
>>> +                if (UtilValidate.isEmpty(selectedFeatures)) {
>>> +                    request.setAttribute("product_id", productId);
>>> +                     request .setAttribute ("_EVENT_MESSAGE_
>>> ",UtilProperties .getMessage
>>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>>> +                    return "product";
>>> +                }
>>>
>>>                 String variantProductId = 
>>> ProductWorker.getVariantFromFeatureTree(productId,
>>> selectedFeatures,  delegator);
>>>                 if (UtilValidate.isNotEmpty(variantProductId)) {
>>> @@ -268,12 +268,12 @@
>>>                     return "product";
>>>                 }
>>>
>>> - } else {
>>> - request.setAttribute("product_id", productId);
>>> - request .setAttribute ("_EVENT_MESSAGE_ ",UtilProperties
>>> .getMessage
>>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>>> - return "product";
>>> - }
>>> - }
>>> +            } else {
>>> +                request.setAttribute("product_id", productId);
>>> +                 request .setAttribute ("_EVENT_MESSAGE_
>>> ",UtilProperties .getMessage
>>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>>> +                return "product";
>>> +            }
>>> +        }
>>>
>>>         // get the override price
>>>         if (paramMap.containsKey("PRICE")) {
>>> @@ -323,7 +323,7 @@
>>>             }
>>>
>>>             if (reservStart != null && reservEnd != null) {
>>> -            reservLength = new 
>>> Double(UtilDateTime.getInterval(reservStart,reservEnd)/86400000);
>>> +                reservLength = new 
>>> Double(UtilDateTime.getInterval(reservStart,reservEnd)/86400000);
>>>             }
>>>
>>>             if (reservStart != null && 
>>> paramMap.containsKey("reservLength")) {
>>> @@ -362,8 +362,8 @@
>>>
>>>             //check accommodation for reservations
>>>             if((paramMap.containsKey("accommodationMapId")) && 
>>> (paramMap.containsKey("accommodationSpotId"))){
>>> -            accommodationMapId = (String) 
>>> paramMap.remove("accommodationMapId");
>>> -            accommodationSpotId = (String) 
>>> paramMap.remove("accommodationSpotId");
>>> +                accommodationMapId = (String) 
>>> paramMap.remove("accommodationMapId");
>>> +                accommodationSpotId = (String) 
>>> paramMap.remove("accommodationSpotId");
>>>             }
>>>         }
>>>
>>> @@ -541,7 +541,7 @@
>>>         result = cartHelper.addToCart(catalogId, shoppingListId, 
>>> shoppingListItemSeqId, productId, productCategoryId,
>>>                 itemType, itemDescription, price, amount, quantity, 
>>> reservStart, reservLength, reservPersons,
>>>                 accommodationMapId, accommodationSpotId,
>>> -                shipBeforeDate, shipAfterDate, configWrapper, 
>>> itemGroupNumber, paramMap, parentProductId);
>>> +                shipBeforeDate, shipAfterDate, configWrapper, 
>>> itemGroupNumber, paramMap, parentProductId , true);
>>>         controlDirective = processResult(result, request);
>>>
>>>         // Determine where to send the browser
>>> @@ -549,9 +549,9 @@
>>>             return "error";
>>>         } else {
>>>             if (cart.viewCartOnAdd()) {
>>> -            return "viewcart";
>>> +                return "viewcart";
>>>             } else {
>>> -            return "success";
>>> +                return "success";
>>>             }
>>>         }
>>>     }
>>> @@ -940,11 +940,11 @@
>>>         Locale locale = UtilHttp.getLocale(request);
>>>
>>>         if (UtilValidate.isEmpty(alternateGwpProductId)) {
>>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>> .getMessage (resource_error
>>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpProductIdPassed", 
>>> locale));
>>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>> .getMessage (resource_error
>>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpProductIdPassed", 
>>> locale));
>>>             return "error";
>>>         }
>>>         if (UtilValidate.isEmpty(alternateGwpLineStr)) {
>>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>> .getMessage (resource_error
>>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpLinePassed", locale));
>>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>> .getMessage (resource_error
>>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpLinePassed", locale));
>>>             return "error";
>>>         }
>>>
>>> @@ -952,13 +952,13 @@
>>>         try {
>>>             alternateGwpLine = Integer.parseInt(alternateGwpLineStr);
>>>         } catch (Exception e) {
>>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>> .getMessage (resource_error
>>> ,"OrderCouldNotSelectAlternateGiftAlternateGwpLineIsNotAValidNumber
>>> ", locale));
>>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>> .getMessage (resource_error
>>> ,"OrderCouldNotSelectAlternateGiftAlternateGwpLineIsNotAValidNumber
>>> ", locale));
>>>             return "error";
>>>         }
>>>
>>>         ShoppingCartItem cartLine = 
>>> cart.findCartItem(alternateGwpLine);
>>>         if (cartLine == null) {
>>> -        request.setAttribute("_ERROR_MESSAGE_", "Could not select 
>>> alternate gift, no cart line item found for #" + alternateGwpLine + 
>>> ".");
>>> +            request.setAttribute("_ERROR_MESSAGE_", "Could not 
>>> select alternate gift, no cart line item found for #" +
>>> alternateGwpLine + ".");
>>>             return "error";
>>>         }
>>>
>>> @@ -994,7 +994,7 @@
>>>         int i;
>>>
>>>         if (UtilValidate.isEmpty(partyId) || roleTypeId.length < 1) {
>>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined", 
>>> locale));
>>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined", 
>>> locale));
>>>             return "error";
>>>         }
>>>
>>> @@ -1025,7 +1025,7 @@
>>>         int i;
>>>
>>>         if (UtilValidate.isEmpty(partyId) || roleTypeId.length < 1) {
>>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined", 
>>> locale));
>>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties
>>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined", 
>>> locale));
>>>             return "error";
>>>         }
>>>
>>> Index: applications/order/src/org/ofbiz/order/shoppingcart/
>>> ShoppingCartHelper.java
>>> ===================================================================
>>> --- applications/order/src/org/ofbiz/order/shoppingcart/
>>> ShoppingCartHelper.java (revision 729034)
>>> +++ applications/order/src/org/ofbiz/order/shoppingcart/
>>> ShoppingCartHelper.java (working copy)
>>> @@ -107,7 +107,7 @@
>>>         return 
>>> addToCart(catalogId,shoppingListId,shoppingListItemSeqId,productId,
>>>                 
>>> productCategoryId,itemType,itemDescription,price,amount,quantity,
>>>                  reservStart
>>> ,reservLength,reservPersons,null,null,shipBeforeDate,shipAfterDate,
>>> -                
>>> configWrapper,itemGroupNumber,context,parentProductId);
>>> +                
>>> configWrapper,itemGroupNumber,context,parentProductId, true);
>>>     }
>>>
>>>     /** Event to add an item to the shopping cart with 
>>> accommodation. */
>>> @@ -116,7 +116,7 @@
>>>             Double price, Double amount, double quantity,
>>>             java.sql.Timestamp reservStart, Double reservLength, 
>>> Double reservPersons, String accommodationMapId,String
>>> accommodationSpotId,
>>>             java.sql.Timestamp shipBeforeDate, java.sql.Timestamp 
>>> shipAfterDate,
>>> -            ProductConfigWrapper configWrapper, String 
>>> itemGroupNumber, Map context, String parentProductId) {
>>> +            ProductConfigWrapper configWrapper, String 
>>> itemGroupNumber, Map context, String parentProductId, boolean
>>> checkItemsExists) {
>>>         Map result = null;
>>>         Map attributes = null;
>>>         String pProductId = null;
>>> @@ -234,7 +234,7 @@
>>>
>>>                        itemId = cart.addOrIncreaseItem(productId, 
>>> amount, quantity, reservStart, reservLength,
>>>                                                 reservPersons, 
>>> accommodationMapId, accommodationSpotId, shipBeforeDate,
>>> shipAfterDate, additionalFeaturesMap, attributes,
>>> -                                                catalogId, 
>>> configWrapper, itemType, itemGroupNumber, pProductId, dispatcher);
>>> +                                                catalogId, 
>>> configWrapper, itemType, itemGroupNumber, pProductId, dispatcher,
>>> checkItemsExists);
>>>
>>>             } else {
>>>                 itemId = cart.addNonProductItem(itemType, 
>>> itemDescription, productCategoryId, price, quantity, attributes,
>>> catalogId, itemGroupNumber, dispatcher);
>>> @@ -678,16 +678,16 @@
>>>                     } else if 
>>> (parameterName.toUpperCase().startsWith("DESCRIPTION")) {
>>>                         itemDescription = quantString;  // the 
>>> quantString is actually the description if the field name starts
>>> with DESCRIPTION
>>>                     } else if 
>>> (parameterName.startsWith("reservStart")) {
>>> -                    if (quantString.length() ==0){
>>> -                    // should have format: yyyy-mm-dd 
>>> hh:mm:ss.fffffffff
>>> -                    quantString += " 00:00:00.000000000";
>>> -                    }
>>> -                    if (item != null) {
>>> -                    Timestamp reservStart = 
>>> Timestamp.valueOf(quantString);
>>> -                    item.setReservStart(reservStart);
>>> -                    }
>>> +                        if (quantString.length() ==0){
>>> +                            // should have format: yyyy-mm-dd 
>>> hh:mm:ss.fffffffff
>>> +                            quantString += " 00:00:00.000000000";
>>> +                        }
>>> +                        if (item != null) {
>>> +                            Timestamp reservStart = 
>>> Timestamp.valueOf(quantString);
>>> +                            item.setReservStart(reservStart);
>>> +                        }
>>>                     } else if 
>>> (parameterName.startsWith("reservLength")) {
>>> -                    if (item != null) {
>>> +                        if (item != null) {
>>>                             double reservLength = 
>>> nf.parse(quantString).doubleValue();
>>>                             item.setReservLength(reservLength);
>>>                         }
>>> Index: applications/order/src/org/ofbiz/order/shoppinglist/
>>> ShoppingListEvents.java
>>> ===================================================================
>>> --- applications/order/src/org/ofbiz/order/shoppinglist/
>>> ShoppingListEvents.java (revision 729034)
>>> +++ applications/order/src/org/ofbiz/order/shoppinglist/
>>> ShoppingListEvents.java (working copy)
>>> @@ -138,7 +138,7 @@
>>>             try {
>>>                 cartIdInt = new Integer(items[i]);
>>>             } catch (Exception e) {
>>> -            Debug.logWarning(e,  UtilProperties .getMessage
>>> (resource_error,"OrderIllegalCharacterInSelectedItemField",
>>> cart.getLocale()), module);
>>> +                Debug.logWarning(e,  UtilProperties .getMessage
>>> (resource_error,"OrderIllegalCharacterInSelectedItemField",
>>> cart.getLocale()), module);
>>>             }
>>>             if (cartIdInt != null) {
>>>                 ShoppingCartItem item = 
>>> cart.findCartItem(cartIdInt.intValue());
>>> @@ -292,18 +292,18 @@
>>>                 if (reservStart == null) {
>>>                        cart.addOrIncreaseItem(productId, null, 
>>> quantity.doubleValue(), null, null, null, null, null, null,
>>> attributes, prodCatalogId, configWrapper, null, null, null, 
>>> dispatcher);
>>>                 }else{
>>> -                    cart.addOrIncreaseItem(productId, null, 
>>> quantity.doubleValue(), reservStart, reservLength, reservPersons,
>>> null, null, null, null, null, attributes, prodCatalogId, 
>>> configWrapper, null, null, null, dispatcher);
>>> +                    cart.addOrIncreaseItem(productId, null, 
>>> quantity.doubleValue(), reservStart, reservLength, reservPersons,
>>> null, null, null, null, null, attributes, prodCatalogId, 
>>> configWrapper, null, null, null, dispatcher, true);
>>>                 }
>>>                 Map messageMap = UtilMisc.toMap("productId", 
>>> productId);
>>>                 errMsg =  UtilProperties
>>> .getMessage(resource,"shoppinglistevents.added_product_to_cart", 
>>> messageMap, cart.getLocale());
>>>                 eventMessage.append(errMsg + "\n");
>>>             } catch (CartItemModifyException e) {
>>> -            Debug.logWarning(e,  UtilProperties
>>> .getMessage(resource_error,"OrderProblemsAddingItemFromListToCart",
>>> cart.getLocale()));
>>> +                Debug.logWarning(e,  UtilProperties
>>> .getMessage(resource_error,"OrderProblemsAddingItemFromListToCart",
>>> cart.getLocale()));
>>>                 Map messageMap = UtilMisc.toMap("productId", 
>>> productId);
>>>                 errMsg =  UtilProperties .getMessage
>>> (resource,"shoppinglistevents.problem_adding_product_to_cart", 
>>> messageMap, cart.getLocale());
>>>                 eventMessage.append(errMsg + "\n");
>>>             } catch (ItemNotFoundException e) {
>>> -            Debug.logWarning(e, 
>>> UtilProperties.getMessage(resource_error,"OrderProductNotFound", 
>>> cart.getLocale()));
>>> +                Debug.logWarning(e, 
>>> UtilProperties.getMessage(resource_error,"OrderProductNotFound", 
>>> cart.getLocale()));
>>>                 Map messageMap = UtilMisc.toMap("productId", 
>>> productId);
>>>                 errMsg =  UtilProperties .getMessage
>>> (resource,"shoppinglistevents.problem_adding_product_to_cart", 
>>> messageMap, cart.getLocale());
>>>                 eventMessage.append(errMsg + "\n");
>>> @@ -343,7 +343,7 @@
>>>         try {
>>>             result = dispatcher.runSync("updateShoppingListItem", 
>>> serviceInMap);
>>>         } catch (GenericServiceException e) {
>>> -        String errMsg =  UtilProperties .getMessage
>>> (ShoppingListEvents
>>> .err_resource,"shoppingListEvents.error_calling_update", locale) + 
>>> ": "  + e.toString();
>>> +            String errMsg =  UtilProperties .getMessage
>>> (ShoppingListEvents
>>> .err_resource,"shoppingListEvents.error_calling_update", locale) + 
>>> ": "  + e.toString();
>>>             request.setAttribute("_ERROR_MESSAGE_", errMsg);
>>>             String errorMsg = "Error calling the 
>>> updateShoppingListItem in handleShoppingListItemVariant: " + 
>>> e.toString();
>>>             Debug.logError(e, errorMsg, module);
>>> Index: applications/order/src/org/ofbiz/order/shoppinglist/
>>> ShoppingListServices.java
>>> ===================================================================
>>> --- applications/order/src/org/ofbiz/order/shoppinglist/
>>> ShoppingListServices.java (revision 729034)
>>> +++ applications/order/src/org/ofbiz/order/shoppinglist/
>>> ShoppingListServices.java (working copy)
>>> @@ -416,7 +416,7 @@
>>>      * @return
>>>      */
>>>     public static ShoppingCart makeShoppingListCart(LocalDispatcher 
>>> dispatcher, GenericValue shoppingList, Locale locale) {
>>> -    return makeShoppingListCart(null, dispatcher, shoppingList, 
>>> locale); }
>>> +        return makeShoppingListCart(null, dispatcher,
>>> shoppingList,  locale); }
>>>
>>>     /**
>>>      * Add a shoppinglist to an existing shoppingcart
>>> @@ -451,20 +451,20 @@
>>>             }
>>>
>>>             if (UtilValidate.isNotEmpty(items)) {
>>> -            if (listCart == null) {
>>> -            listCart = new ShoppingCart(delegator,  productStoreId,
>>> locale, currencyUom);
>>> -           
>>> listCart.setOrderPartyId(shoppingList.getString("partyId"));
>>> -            listCart
>>> .setAutoOrderShoppingListId(shoppingList.getString("shoppingListId"));
>>> -            } else {
>>> -            if (!
>>> listCart.getPartyId().equals(shoppingList.getString("partyId"))){
>>> -            Debug.logError("CANNOT add shoppingList: " + 
>>> shoppingList.getString("shoppingListId")
>>> -            + " of partyId: " +  shoppingList.getString("partyId")
>>> -            + " to a shoppingcart with a different  orderPartyId: "
>>> -            + listCart.getPartyId(), module);
>>> -                return listCart;
>>> -            }
>>> -            }
>>> -            +                if (listCart == null) {
>>> +                    listCart = new ShoppingCart(delegator, 
>>> productStoreId, locale, currencyUom);
>>> +                    
>>> listCart.setOrderPartyId(shoppingList.getString("partyId"));
>>> +                     listCart
>>> .setAutoOrderShoppingListId(shoppingList.getString("shoppingListId"));
>>> +                } else {
>>> +                    if (!
>>> listCart.getPartyId().equals(shoppingList.getString("partyId"))){
>>> +                        Debug.logError("CANNOT add shoppingList: " 
>>> + shoppingList.getString("shoppingListId")
>>> +                                + " of partyId: " + 
>>> shoppingList.getString("partyId")
>>> +                                + " to a shoppingcart with a 
>>> different orderPartyId: "
>>> +                                + listCart.getPartyId(), module);
>>> +                        return listCart;
>>> +                    }
>>> +                }
>>> +
>>>
>>>                 Iterator i = items.iterator();
>>>                 ProductConfigWrapper configWrapper = null;
>>> @@ -502,7 +502,7 @@
>>>                         Map attributes = 
>>> UtilMisc.toMap("shoppingListId", listId, "shoppingListItemSeqId", 
>>> itemId);
>>>
>>>                         try {
>>> -                            listCart.addOrIncreaseItem(productId, 
>>> null, quantity.doubleValue(), reservStart, reservLength,
>>> reservPersons, null, null, null, null, null, attributes, null, 
>>> configWrapper, null, null, null, dispatcher);
>>> +                            listCart.addOrIncreaseItem(productId, 
>>> null, quantity.doubleValue(), reservStart, reservLength,
>>> reservPersons, null, null, null, null, null, attributes, null, 
>>> configWrapper, null, null, null, dispatcher, true);
>>>                         } catch (CartItemModifyException e) {
>>>                             Debug.logError(e, "Unable to add
>>> product  to List Cart - " + productId, module);
>>>                         } catch (ItemNotFoundException e) {
>>
>
>
>

-- 
Stephen P Rufle
srufle@salmonllc.com
H1:480-626-8022
H2:480-802-7173
Yahoo IM: stephen_rufle
AOL IM: stephen1rufle


Re: ShoppingCart.addOrIncreaseItem

Posted by Jacques Le Roux <ja...@les7arts.com>.
There are both pro and cons with any of these tools (Tortoise, Eclipse, svn directly, patch, etc.). Only experience taught when to 
use one of them. I prefer Tortoise in most cases but this implies Windows.
In this case creating 2 patches is a good idea IMO

Jacques

From: "David E Jones" <da...@hotwaxmedia.com>
>
> It makes it easier to review if you avoid formatting changes. Usually  a note along with the patch that the file had tabs in it 
> would help  the committer to know that it needs updating.
>
> BTW, the best way to generate patches is to use SVN itself, something  like "svn diff > mychanges.patch". I think various people 
> use Eclipse  for patches, mainly because I see comments about problems they have  with it (seems mostly related to unexpected 
> behavior).
>
> -David
>
>
> On Dec 23, 2008, at 10:19 AM, Stephen Rufle wrote:
>
>> Added checkItemExists to ShoppingCart.addOrIncreaseItem(... , boolean
>> checkItemsExists)
>>
>> I wanted to have the group review my patch before I created a JIRA.  I am
>> new to patching so I see in my patch that it is changing tabs to  spaces
>> and adding the new code and applies to multiple files all in one  patch,
>> previously I have sent patches that were file specific.
>>
>> I am using eclipse to generate patches, but I would have liked to
>> generate two. One for the tab to spaces and another to show what I
>> added. Any advice would be appropriated.
>>
>> If the patch is good as is I will create a JIRA and attach.
>>
>> David E Jones wrote:
>>>
>>> My favorite current proposal is to super-simplify that method and  then
>>> on the ShoppingCartItem object that it returns just call additional
>>> methods to add additional data. That way now, and in the future, we
>>> only have to add setters to ShoppingCartItem to support new data  there.
>>>
>>> -David
>>>
>>>
>>> On Dec 22, 2008, at 3:51 PM, Stephen Rufle wrote:
>>>
>>>> I would like to add a parameter to  ShoppingCart.addOrIncreaseItem(... ,
>>>> boolean checkItemsExists)
>>>>
>>>> I see that the method signature is already pretty long. I would  like to
>>>> know if instead would it be a better idea to pass along a Map object
>>>> around instead?
>>>>
>>>> If there is already an existing effort to reduce the number of
>>>> parameters I would like to see how you are dealing with it.
>>>>
>>>>
>>>
>>>
>>>
>>
>> -- 
>> Stephen P Rufle
>> srufle@salmonllc.com
>> H1:480-626-8022
>> H2:480-802-7173
>> Yahoo IM: stephen_rufle
>> AOL IM: stephen1rufle
>>
>> ### Eclipse Workspace Patch 1.0
>> #P ofbiz
>> Index: applications/order/src/org/ofbiz/order/shoppingcart/ ShoppingCart.java
>> ===================================================================
>> --- applications/order/src/org/ofbiz/order/shoppingcart/ ShoppingCart.java (revision 729034)
>> +++ applications/order/src/org/ofbiz/order/shoppingcart/ ShoppingCart.java (working copy)
>> @@ -453,14 +453,14 @@
>>
>>        return  addOrIncreaseItem (productId ,selectedAmountDbl ,quantity,reservStart,reservLengthDbl,reservPersonsDbl,
>>                         null ,null,shipBeforeDate,shipAfterDate,features,attributes,prodCatalogId,
>> -                 configWrapper,itemType,itemGroupNumber,parentProductId,dispatcher);
>> +                 configWrapper,itemType,itemGroupNumber,parentProductId,dispatcher,  true);
>>     }
>>
>>     /** add rental (with accommodation) item to cart  */
>>     public int addOrIncreaseItem(String productId, Double  selectedAmountDbl, double quantity, Timestamp reservStart, Double 
>> reservLengthDbl, Double reservPersonsDbl,
>>                String accommodationMapId, String accommodationSpotId,
>>             Timestamp shipBeforeDate, Timestamp shipAfterDate, Map  features, Map attributes, String prodCatalogId,
>> -            ProductConfigWrapper configWrapper, String itemType,  String itemGroupNumber, String parentProductId, 
>> LocalDispatcher  dispatcher) throws CartItemModifyException, ItemNotFoundException {
>> +            ProductConfigWrapper configWrapper, String itemType,  String itemGroupNumber, String parentProductId, 
>> LocalDispatcher  dispatcher, boolean checkItemsExists) throws  CartItemModifyException, ItemNotFoundException {
>>         if (isReadOnlyCart()) {
>>            throw new CartItemModifyException("Cart items cannot be  changed");
>>         }
>> Index: applications/order/src/org/ofbiz/order/shoppingcart/ ShoppingCartEvents.java
>> ===================================================================
>> --- applications/order/src/org/ofbiz/order/shoppingcart/ ShoppingCartEvents.java (revision 729034)
>> +++ applications/order/src/org/ofbiz/order/shoppingcart/ ShoppingCartEvents.java (working copy)
>> @@ -156,12 +156,12 @@
>>         if (paramMap.containsKey("ADD_PRODUCT_ID")) {
>>             productId = (String) paramMap.remove("ADD_PRODUCT_ID");
>>         } else if (paramMap.containsKey("add_product_id")) {
>> -        Object object = paramMap.remove("add_product_id");
>> -        try{
>> -        productId = (String) object;
>> -        }catch(ClassCastException e){
>> -        productId = (String)((List)object).get(0);
>> -        }
>> +            Object object = paramMap.remove("add_product_id");
>> +            try{
>> +                productId = (String) object;
>> +            }catch(ClassCastException e){
>> +                productId = (String)((List)object).get(0);
>> +            }
>>         }
>>         if (paramMap.containsKey("PRODUCT_ID")) {
>>             parentProductId = (String) paramMap.remove("PRODUCT_ID");
>> @@ -241,23 +241,23 @@
>>         //Check for virtual products
>>         if (ProductWorker.isVirtual(delegator, productId)) {
>>
>> - if  ("VV_FEATURETREE ".equals(ProductWorker.getProductvirtualVariantMethod(delegator,  productId))) {
>> - // get the selected features.
>> - List <String> selectedFeatures = new LinkedList<String>();
>> -     java.util.Enumeration paramNames =  request.getParameterNames();
>> -     while(paramNames.hasMoreElements()) {
>> -     String paramName = (String)paramNames.nextElement();
>> -     if (paramName.startsWith("FT")) {
>> -     selectedFeatures.add(request.getParameterValues(paramName) [0]);
>> -     }
>> -     }
>> -     -     // check if features are selected
>> -     if (UtilValidate.isEmpty(selectedFeatures)) {
>> - request.setAttribute("product_id", productId);
>> - request .setAttribute ("_EVENT_MESSAGE_ ",UtilProperties .getMessage 
>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>> - return "product";
>> -     }
>> +            if  ("VV_FEATURETREE ".equals(ProductWorker.getProductvirtualVariantMethod(delegator,  productId))) {
>> +                // get the selected features.
>> +                List <String> selectedFeatures = new  LinkedList<String>();
>> +                java.util.Enumeration paramNames =  request.getParameterNames();
>> +                while(paramNames.hasMoreElements()) {
>> +                    String paramName =  (String)paramNames.nextElement();
>> +                    if (paramName.startsWith("FT")) {
>> +                         selectedFeatures.add(request.getParameterValues(paramName)[0]);
>> +                    }
>> +                }
>> +
>> +                // check if features are selected
>> +                if (UtilValidate.isEmpty(selectedFeatures)) {
>> +                    request.setAttribute("product_id", productId);
>> +                     request .setAttribute ("_EVENT_MESSAGE_ ",UtilProperties .getMessage 
>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>> +                    return "product";
>> +                }
>>
>>                 String variantProductId =  ProductWorker.getVariantFromFeatureTree(productId, selectedFeatures,  delegator);
>>                 if (UtilValidate.isNotEmpty(variantProductId)) {
>> @@ -268,12 +268,12 @@
>>                     return "product";
>>                 }
>>
>> - } else {
>> - request.setAttribute("product_id", productId);
>> - request .setAttribute ("_EVENT_MESSAGE_ ",UtilProperties .getMessage 
>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>> - return "product";
>> - }
>> - }
>> +            } else {
>> +                request.setAttribute("product_id", productId);
>> +                 request .setAttribute ("_EVENT_MESSAGE_ ",UtilProperties .getMessage 
>> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
>> +                return "product";
>> +            }
>> +        }
>>
>>         // get the override price
>>         if (paramMap.containsKey("PRICE")) {
>> @@ -323,7 +323,7 @@
>>             }
>>
>>             if (reservStart != null && reservEnd != null) {
>> -            reservLength = new  Double(UtilDateTime.getInterval(reservStart,reservEnd)/86400000);
>> +                reservLength = new  Double(UtilDateTime.getInterval(reservStart,reservEnd)/86400000);
>>             }
>>
>>             if (reservStart != null &&  paramMap.containsKey("reservLength")) {
>> @@ -362,8 +362,8 @@
>>
>>             //check accommodation for reservations
>>             if((paramMap.containsKey("accommodationMapId")) &&  (paramMap.containsKey("accommodationSpotId"))){
>> -            accommodationMapId = (String)  paramMap.remove("accommodationMapId");
>> -            accommodationSpotId = (String)  paramMap.remove("accommodationSpotId");
>> +                accommodationMapId = (String)  paramMap.remove("accommodationMapId");
>> +                accommodationSpotId = (String)  paramMap.remove("accommodationSpotId");
>>             }
>>         }
>>
>> @@ -541,7 +541,7 @@
>>         result = cartHelper.addToCart(catalogId, shoppingListId,  shoppingListItemSeqId, productId, productCategoryId,
>>                 itemType, itemDescription, price, amount, quantity,  reservStart, reservLength, reservPersons,
>>                 accommodationMapId, accommodationSpotId,
>> -                shipBeforeDate, shipAfterDate, configWrapper,  itemGroupNumber, paramMap, parentProductId);
>> +                shipBeforeDate, shipAfterDate, configWrapper,  itemGroupNumber, paramMap, parentProductId , true);
>>         controlDirective = processResult(result, request);
>>
>>         // Determine where to send the browser
>> @@ -549,9 +549,9 @@
>>             return "error";
>>         } else {
>>             if (cart.viewCartOnAdd()) {
>> -            return "viewcart";
>> +                return "viewcart";
>>             } else {
>> -            return "success";
>> +                return "success";
>>             }
>>         }
>>     }
>> @@ -940,11 +940,11 @@
>>         Locale locale = UtilHttp.getLocale(request);
>>
>>         if (UtilValidate.isEmpty(alternateGwpProductId)) {
>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties .getMessage (resource_error 
>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpProductIdPassed",  locale));
>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties .getMessage (resource_error 
>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpProductIdPassed",  locale));
>>             return "error";
>>         }
>>         if (UtilValidate.isEmpty(alternateGwpLineStr)) {
>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties .getMessage (resource_error 
>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpLinePassed", locale));
>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties .getMessage (resource_error 
>> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpLinePassed", locale));
>>             return "error";
>>         }
>>
>> @@ -952,13 +952,13 @@
>>         try {
>>             alternateGwpLine = Integer.parseInt(alternateGwpLineStr);
>>         } catch (Exception e) {
>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties .getMessage (resource_error 
>> ,"OrderCouldNotSelectAlternateGiftAlternateGwpLineIsNotAValidNumber ", locale));
>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties .getMessage (resource_error 
>> ,"OrderCouldNotSelectAlternateGiftAlternateGwpLineIsNotAValidNumber ", locale));
>>             return "error";
>>         }
>>
>>         ShoppingCartItem cartLine =  cart.findCartItem(alternateGwpLine);
>>         if (cartLine == null) {
>> -        request.setAttribute("_ERROR_MESSAGE_", "Could not select  alternate gift, no cart line item found for #" + 
>> alternateGwpLine +  ".");
>> +            request.setAttribute("_ERROR_MESSAGE_", "Could not  select alternate gift, no cart line item found for #" + 
>> alternateGwpLine + ".");
>>             return "error";
>>         }
>>
>> @@ -994,7 +994,7 @@
>>         int i;
>>
>>         if (UtilValidate.isEmpty(partyId) || roleTypeId.length < 1) {
>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties 
>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined",  locale));
>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties 
>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined",  locale));
>>             return "error";
>>         }
>>
>> @@ -1025,7 +1025,7 @@
>>         int i;
>>
>>         if (UtilValidate.isEmpty(partyId) || roleTypeId.length < 1) {
>> -        request.setAttribute("_ERROR_MESSAGE_",  UtilProperties 
>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined",  locale));
>> +            request.setAttribute("_ERROR_MESSAGE_",  UtilProperties 
>> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined",  locale));
>>             return "error";
>>         }
>>
>> Index: applications/order/src/org/ofbiz/order/shoppingcart/ ShoppingCartHelper.java
>> ===================================================================
>> --- applications/order/src/org/ofbiz/order/shoppingcart/ ShoppingCartHelper.java (revision 729034)
>> +++ applications/order/src/org/ofbiz/order/shoppingcart/ ShoppingCartHelper.java (working copy)
>> @@ -107,7 +107,7 @@
>>         return  addToCart(catalogId,shoppingListId,shoppingListItemSeqId,productId,
>>                  productCategoryId,itemType,itemDescription,price,amount,quantity,
>>                  reservStart ,reservLength,reservPersons,null,null,shipBeforeDate,shipAfterDate,
>> -                 configWrapper,itemGroupNumber,context,parentProductId);
>> +                 configWrapper,itemGroupNumber,context,parentProductId, true);
>>     }
>>
>>     /** Event to add an item to the shopping cart with  accommodation. */
>> @@ -116,7 +116,7 @@
>>             Double price, Double amount, double quantity,
>>             java.sql.Timestamp reservStart, Double reservLength,  Double reservPersons, String accommodationMapId,String 
>> accommodationSpotId,
>>             java.sql.Timestamp shipBeforeDate, java.sql.Timestamp  shipAfterDate,
>> -            ProductConfigWrapper configWrapper, String  itemGroupNumber, Map context, String parentProductId) {
>> +            ProductConfigWrapper configWrapper, String  itemGroupNumber, Map context, String parentProductId, boolean 
>> checkItemsExists) {
>>         Map result = null;
>>         Map attributes = null;
>>         String pProductId = null;
>> @@ -234,7 +234,7 @@
>>
>>                        itemId = cart.addOrIncreaseItem(productId,  amount, quantity, reservStart, reservLength,
>>                                                 reservPersons,  accommodationMapId, accommodationSpotId, shipBeforeDate, 
>> shipAfterDate, additionalFeaturesMap, attributes,
>> -                                                catalogId,  configWrapper, itemType, itemGroupNumber, pProductId, dispatcher);
>> +                                                catalogId,  configWrapper, itemType, itemGroupNumber, pProductId, dispatcher, 
>> checkItemsExists);
>>
>>             } else {
>>                 itemId = cart.addNonProductItem(itemType,  itemDescription, productCategoryId, price, quantity, attributes, 
>> catalogId, itemGroupNumber, dispatcher);
>> @@ -678,16 +678,16 @@
>>                     } else if  (parameterName.toUpperCase().startsWith("DESCRIPTION")) {
>>                         itemDescription = quantString;  // the  quantString is actually the description if the field name starts 
>> with DESCRIPTION
>>                     } else if  (parameterName.startsWith("reservStart")) {
>> -                    if (quantString.length() ==0){
>> -                    // should have format: yyyy-mm-dd  hh:mm:ss.fffffffff
>> -                    quantString += " 00:00:00.000000000";
>> -                    }
>> -                    if (item != null) {
>> -                    Timestamp reservStart =  Timestamp.valueOf(quantString);
>> -                    item.setReservStart(reservStart);
>> -                    }
>> +                        if (quantString.length() ==0){
>> +                            // should have format: yyyy-mm-dd  hh:mm:ss.fffffffff
>> +                            quantString += " 00:00:00.000000000";
>> +                        }
>> +                        if (item != null) {
>> +                            Timestamp reservStart =  Timestamp.valueOf(quantString);
>> +                            item.setReservStart(reservStart);
>> +                        }
>>                     } else if  (parameterName.startsWith("reservLength")) {
>> -                    if (item != null) {
>> +                        if (item != null) {
>>                             double reservLength =  nf.parse(quantString).doubleValue();
>>                             item.setReservLength(reservLength);
>>                         }
>> Index: applications/order/src/org/ofbiz/order/shoppinglist/ ShoppingListEvents.java
>> ===================================================================
>> --- applications/order/src/org/ofbiz/order/shoppinglist/ ShoppingListEvents.java (revision 729034)
>> +++ applications/order/src/org/ofbiz/order/shoppinglist/ ShoppingListEvents.java (working copy)
>> @@ -138,7 +138,7 @@
>>             try {
>>                 cartIdInt = new Integer(items[i]);
>>             } catch (Exception e) {
>> -            Debug.logWarning(e,  UtilProperties .getMessage (resource_error,"OrderIllegalCharacterInSelectedItemField", 
>> cart.getLocale()), module);
>> +                Debug.logWarning(e,  UtilProperties .getMessage (resource_error,"OrderIllegalCharacterInSelectedItemField", 
>> cart.getLocale()), module);
>>             }
>>             if (cartIdInt != null) {
>>                 ShoppingCartItem item =  cart.findCartItem(cartIdInt.intValue());
>> @@ -292,18 +292,18 @@
>>                 if (reservStart == null) {
>>                        cart.addOrIncreaseItem(productId, null,  quantity.doubleValue(), null, null, null, null, null, null, 
>> attributes, prodCatalogId, configWrapper, null, null, null,  dispatcher);
>>                 }else{
>> -                    cart.addOrIncreaseItem(productId, null,  quantity.doubleValue(), reservStart, reservLength, reservPersons, 
>> null, null, null, null, null, attributes, prodCatalogId,  configWrapper, null, null, null, dispatcher);
>> +                    cart.addOrIncreaseItem(productId, null,  quantity.doubleValue(), reservStart, reservLength, reservPersons, 
>> null, null, null, null, null, attributes, prodCatalogId,  configWrapper, null, null, null, dispatcher, true);
>>                 }
>>                 Map messageMap = UtilMisc.toMap("productId",  productId);
>>                 errMsg =  UtilProperties .getMessage(resource,"shoppinglistevents.added_product_to_cart",  messageMap, 
>> cart.getLocale());
>>                 eventMessage.append(errMsg + "\n");
>>             } catch (CartItemModifyException e) {
>> -            Debug.logWarning(e,  UtilProperties .getMessage(resource_error,"OrderProblemsAddingItemFromListToCart", 
>> cart.getLocale()));
>> +                Debug.logWarning(e,  UtilProperties .getMessage(resource_error,"OrderProblemsAddingItemFromListToCart", 
>> cart.getLocale()));
>>                 Map messageMap = UtilMisc.toMap("productId",  productId);
>>                 errMsg =  UtilProperties .getMessage (resource,"shoppinglistevents.problem_adding_product_to_cart",  messageMap, 
>> cart.getLocale());
>>                 eventMessage.append(errMsg + "\n");
>>             } catch (ItemNotFoundException e) {
>> -            Debug.logWarning(e,  UtilProperties.getMessage(resource_error,"OrderProductNotFound",  cart.getLocale()));
>> +                Debug.logWarning(e,  UtilProperties.getMessage(resource_error,"OrderProductNotFound",  cart.getLocale()));
>>                 Map messageMap = UtilMisc.toMap("productId",  productId);
>>                 errMsg =  UtilProperties .getMessage (resource,"shoppinglistevents.problem_adding_product_to_cart",  messageMap, 
>> cart.getLocale());
>>                 eventMessage.append(errMsg + "\n");
>> @@ -343,7 +343,7 @@
>>         try {
>>             result = dispatcher.runSync("updateShoppingListItem",  serviceInMap);
>>         } catch (GenericServiceException e) {
>> -        String errMsg =  UtilProperties .getMessage (ShoppingListEvents .err_resource,"shoppingListEvents.error_calling_update", 
>> locale) +  ": "  + e.toString();
>> +            String errMsg =  UtilProperties .getMessage (ShoppingListEvents 
>> .err_resource,"shoppingListEvents.error_calling_update", locale) +  ": "  + e.toString();
>>             request.setAttribute("_ERROR_MESSAGE_", errMsg);
>>             String errorMsg = "Error calling the  updateShoppingListItem in handleShoppingListItemVariant: " +  e.toString();
>>             Debug.logError(e, errorMsg, module);
>> Index: applications/order/src/org/ofbiz/order/shoppinglist/ ShoppingListServices.java
>> ===================================================================
>> --- applications/order/src/org/ofbiz/order/shoppinglist/ ShoppingListServices.java (revision 729034)
>> +++ applications/order/src/org/ofbiz/order/shoppinglist/ ShoppingListServices.java (working copy)
>> @@ -416,7 +416,7 @@
>>      * @return
>>      */
>>     public static ShoppingCart makeShoppingListCart(LocalDispatcher  dispatcher, GenericValue shoppingList, Locale locale) {
>> -    return makeShoppingListCart(null, dispatcher, shoppingList,  locale); }
>> +        return makeShoppingListCart(null, dispatcher, shoppingList,  locale); }
>>
>>     /**
>>      * Add a shoppinglist to an existing shoppingcart
>> @@ -451,20 +451,20 @@
>>             }
>>
>>             if (UtilValidate.isNotEmpty(items)) {
>> -            if (listCart == null) {
>> -            listCart = new ShoppingCart(delegator,  productStoreId, locale, currencyUom);
>> -            listCart.setOrderPartyId(shoppingList.getString("partyId"));
>> -            listCart .setAutoOrderShoppingListId(shoppingList.getString("shoppingListId"));
>> -            } else {
>> -            if (! listCart.getPartyId().equals(shoppingList.getString("partyId"))){
>> -            Debug.logError("CANNOT add shoppingList: " +  shoppingList.getString("shoppingListId")
>> -            + " of partyId: " +  shoppingList.getString("partyId")
>> -            + " to a shoppingcart with a different  orderPartyId: "
>> -            + listCart.getPartyId(), module);
>> -                return listCart;
>> -            }
>> -            }
>> -            +                if (listCart == null) {
>> +                    listCart = new ShoppingCart(delegator,  productStoreId, locale, currencyUom);
>> +                     listCart.setOrderPartyId(shoppingList.getString("partyId"));
>> +                     listCart .setAutoOrderShoppingListId(shoppingList.getString("shoppingListId"));
>> +                } else {
>> +                    if (! listCart.getPartyId().equals(shoppingList.getString("partyId"))){
>> +                        Debug.logError("CANNOT add shoppingList: "  + shoppingList.getString("shoppingListId")
>> +                                + " of partyId: " +  shoppingList.getString("partyId")
>> +                                + " to a shoppingcart with a  different orderPartyId: "
>> +                                + listCart.getPartyId(), module);
>> +                        return listCart;
>> +                    }
>> +                }
>> +
>>
>>                 Iterator i = items.iterator();
>>                 ProductConfigWrapper configWrapper = null;
>> @@ -502,7 +502,7 @@
>>                         Map attributes =  UtilMisc.toMap("shoppingListId", listId, "shoppingListItemSeqId",  itemId);
>>
>>                         try {
>> -                            listCart.addOrIncreaseItem(productId,  null, quantity.doubleValue(), reservStart, reservLength, 
>> reservPersons, null, null, null, null, null, attributes, null,  configWrapper, null, null, null, dispatcher);
>> +                            listCart.addOrIncreaseItem(productId,  null, quantity.doubleValue(), reservStart, reservLength, 
>> reservPersons, null, null, null, null, null, attributes, null,  configWrapper, null, null, null, dispatcher, true);
>>                         } catch (CartItemModifyException e) {
>>                             Debug.logError(e, "Unable to add product  to List Cart - " + productId, module);
>>                         } catch (ItemNotFoundException e) {
> 


Re: ShoppingCart.addOrIncreaseItem

Posted by David E Jones <da...@hotwaxmedia.com>.
It makes it easier to review if you avoid formatting changes. Usually  
a note along with the patch that the file had tabs in it would help  
the committer to know that it needs updating.

BTW, the best way to generate patches is to use SVN itself, something  
like "svn diff > mychanges.patch". I think various people use Eclipse  
for patches, mainly because I see comments about problems they have  
with it (seems mostly related to unexpected behavior).

-David


On Dec 23, 2008, at 10:19 AM, Stephen Rufle wrote:

> Added checkItemExists to ShoppingCart.addOrIncreaseItem(... , boolean
> checkItemsExists)
>
> I wanted to have the group review my patch before I created a JIRA.  
> I am
> new to patching so I see in my patch that it is changing tabs to  
> spaces
> and adding the new code and applies to multiple files all in one  
> patch,
> previously I have sent patches that were file specific.
>
> I am using eclipse to generate patches, but I would have liked to
> generate two. One for the tab to spaces and another to show what I
> added. Any advice would be appropriated.
>
> If the patch is good as is I will create a JIRA and attach.
>
> David E Jones wrote:
>>
>> My favorite current proposal is to super-simplify that method and  
>> then
>> on the ShoppingCartItem object that it returns just call additional
>> methods to add additional data. That way now, and in the future, we
>> only have to add setters to ShoppingCartItem to support new data  
>> there.
>>
>> -David
>>
>>
>> On Dec 22, 2008, at 3:51 PM, Stephen Rufle wrote:
>>
>>> I would like to add a parameter to  
>>> ShoppingCart.addOrIncreaseItem(... ,
>>> boolean checkItemsExists)
>>>
>>> I see that the method signature is already pretty long. I would  
>>> like to
>>> know if instead would it be a better idea to pass along a Map object
>>> around instead?
>>>
>>> If there is already an existing effort to reduce the number of
>>> parameters I would like to see how you are dealing with it.
>>>
>>>
>>
>>
>>
>
> -- 
> Stephen P Rufle
> srufle@salmonllc.com
> H1:480-626-8022
> H2:480-802-7173
> Yahoo IM: stephen_rufle
> AOL IM: stephen1rufle
>
> ### Eclipse Workspace Patch 1.0
> #P ofbiz
> Index: applications/order/src/org/ofbiz/order/shoppingcart/ 
> ShoppingCart.java
> ===================================================================
> --- applications/order/src/org/ofbiz/order/shoppingcart/ 
> ShoppingCart.java	(revision 729034)
> +++ applications/order/src/org/ofbiz/order/shoppingcart/ 
> ShoppingCart.java	(working copy)
> @@ -453,14 +453,14 @@
>
>        return  
> addOrIncreaseItem 
> (productId 
> ,selectedAmountDbl 
> ,quantity,reservStart,reservLengthDbl,reservPersonsDbl,
>                         
> null 
> ,null,shipBeforeDate,shipAfterDate,features,attributes,prodCatalogId,
> -                 
> configWrapper,itemType,itemGroupNumber,parentProductId,dispatcher);
> +                 
> configWrapper,itemType,itemGroupNumber,parentProductId,dispatcher,  
> true);
>     }
>
>     /** add rental (with accommodation) item to cart  */
>     public int addOrIncreaseItem(String productId, Double  
> selectedAmountDbl, double quantity, Timestamp reservStart, Double  
> reservLengthDbl, Double reservPersonsDbl,
>                String accommodationMapId, String accommodationSpotId,
>             Timestamp shipBeforeDate, Timestamp shipAfterDate, Map  
> features, Map attributes, String prodCatalogId,
> -            ProductConfigWrapper configWrapper, String itemType,  
> String itemGroupNumber, String parentProductId, LocalDispatcher  
> dispatcher) throws CartItemModifyException, ItemNotFoundException {
> +            ProductConfigWrapper configWrapper, String itemType,  
> String itemGroupNumber, String parentProductId, LocalDispatcher  
> dispatcher, boolean checkItemsExists) throws  
> CartItemModifyException, ItemNotFoundException {
>         if (isReadOnlyCart()) {
>            throw new CartItemModifyException("Cart items cannot be  
> changed");
>         }
> Index: applications/order/src/org/ofbiz/order/shoppingcart/ 
> ShoppingCartEvents.java
> ===================================================================
> --- applications/order/src/org/ofbiz/order/shoppingcart/ 
> ShoppingCartEvents.java	(revision 729034)
> +++ applications/order/src/org/ofbiz/order/shoppingcart/ 
> ShoppingCartEvents.java	(working copy)
> @@ -156,12 +156,12 @@
>         if (paramMap.containsKey("ADD_PRODUCT_ID")) {
>             productId = (String) paramMap.remove("ADD_PRODUCT_ID");
>         } else if (paramMap.containsKey("add_product_id")) {
> -        	Object object = paramMap.remove("add_product_id");
> -        	try{
> -        		productId = (String) object;
> -        	}catch(ClassCastException e){
> -        		productId = (String)((List)object).get(0);
> -        	}
> +            Object object = paramMap.remove("add_product_id");
> +            try{
> +                productId = (String) object;
> +            }catch(ClassCastException e){
> +                productId = (String)((List)object).get(0);
> +            }
>         }
>         if (paramMap.containsKey("PRODUCT_ID")) {
>             parentProductId = (String) paramMap.remove("PRODUCT_ID");
> @@ -241,23 +241,23 @@
>         //Check for virtual products
>         if (ProductWorker.isVirtual(delegator, productId)) {
>
> -			if  
> ("VV_FEATURETREE 
> ".equals(ProductWorker.getProductvirtualVariantMethod(delegator,  
> productId))) {
> -				// get the selected features.
> -				List <String> selectedFeatures = new LinkedList<String>();
> -		    	java.util.Enumeration paramNames =  
> request.getParameterNames();
> -		    	while(paramNames.hasMoreElements()) {
> -		    		String paramName = (String)paramNames.nextElement();
> -		    		if (paramName.startsWith("FT")) {
> -		    			selectedFeatures.add(request.getParameterValues(paramName) 
> [0]);
> -		    		}
> -		    	}
> -		    	
> -		    	// check if features are selected
> -		    	if (UtilValidate.isEmpty(selectedFeatures)) {
> -					request.setAttribute("product_id", productId);
> -					 
> request 
> .setAttribute 
> ("_EVENT_MESSAGE_ 
> ",UtilProperties 
> .getMessage 
> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
> -					return "product";
> -		    	}
> +            if  
> ("VV_FEATURETREE 
> ".equals(ProductWorker.getProductvirtualVariantMethod(delegator,  
> productId))) {
> +                // get the selected features.
> +                List <String> selectedFeatures = new  
> LinkedList<String>();
> +                java.util.Enumeration paramNames =  
> request.getParameterNames();
> +                while(paramNames.hasMoreElements()) {
> +                    String paramName =  
> (String)paramNames.nextElement();
> +                    if (paramName.startsWith("FT")) {
> +                         
> selectedFeatures.add(request.getParameterValues(paramName)[0]);
> +                    }
> +                }
> +
> +                // check if features are selected
> +                if (UtilValidate.isEmpty(selectedFeatures)) {
> +                    request.setAttribute("product_id", productId);
> +                     
> request 
> .setAttribute 
> ("_EVENT_MESSAGE_ 
> ",UtilProperties 
> .getMessage 
> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
> +                    return "product";
> +                }
>
>                 String variantProductId =  
> ProductWorker.getVariantFromFeatureTree(productId, selectedFeatures,  
> delegator);
>                 if (UtilValidate.isNotEmpty(variantProductId)) {
> @@ -268,12 +268,12 @@
>                     return "product";
>                 }
>
> -			} else {
> -				request.setAttribute("product_id", productId);
> -				 
> request 
> .setAttribute 
> ("_EVENT_MESSAGE_ 
> ",UtilProperties 
> .getMessage 
> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
> -				return "product";
> -			}
> -		}
> +            } else {
> +                request.setAttribute("product_id", productId);
> +                 
> request 
> .setAttribute 
> ("_EVENT_MESSAGE_ 
> ",UtilProperties 
> .getMessage 
> (resource,"cart.addToCart.chooseVariationBeforeAddingToCart",locale));
> +                return "product";
> +            }
> +        }
>
>         // get the override price
>         if (paramMap.containsKey("PRICE")) {
> @@ -323,7 +323,7 @@
>             }
>
>             if (reservStart != null && reservEnd != null) {
> -            	reservLength = new  
> Double(UtilDateTime.getInterval(reservStart,reservEnd)/86400000);
> +                reservLength = new  
> Double(UtilDateTime.getInterval(reservStart,reservEnd)/86400000);
>             }
>
>             if (reservStart != null &&  
> paramMap.containsKey("reservLength")) {
> @@ -362,8 +362,8 @@
>
>             //check accommodation for reservations
>             if((paramMap.containsKey("accommodationMapId")) &&  
> (paramMap.containsKey("accommodationSpotId"))){
> -            	accommodationMapId = (String)  
> paramMap.remove("accommodationMapId");
> -            	accommodationSpotId = (String)  
> paramMap.remove("accommodationSpotId");
> +                accommodationMapId = (String)  
> paramMap.remove("accommodationMapId");
> +                accommodationSpotId = (String)  
> paramMap.remove("accommodationSpotId");
>             }
>         }
>
> @@ -541,7 +541,7 @@
>         result = cartHelper.addToCart(catalogId, shoppingListId,  
> shoppingListItemSeqId, productId, productCategoryId,
>                 itemType, itemDescription, price, amount, quantity,  
> reservStart, reservLength, reservPersons,
>                 accommodationMapId, accommodationSpotId,
> -                shipBeforeDate, shipAfterDate, configWrapper,  
> itemGroupNumber, paramMap, parentProductId);
> +                shipBeforeDate, shipAfterDate, configWrapper,  
> itemGroupNumber, paramMap, parentProductId , true);
>         controlDirective = processResult(result, request);
>
>         // Determine where to send the browser
> @@ -549,9 +549,9 @@
>             return "error";
>         } else {
>             if (cart.viewCartOnAdd()) {
> -            	return "viewcart";
> +                return "viewcart";
>             } else {
> -            	return "success";
> +                return "success";
>             }
>         }
>     }
> @@ -940,11 +940,11 @@
>         Locale locale = UtilHttp.getLocale(request);
>
>         if (UtilValidate.isEmpty(alternateGwpProductId)) {
> -        	request.setAttribute("_ERROR_MESSAGE_",  
> UtilProperties 
> .getMessage 
> (resource_error 
> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpProductIdPassed",  
> locale));
> +            request.setAttribute("_ERROR_MESSAGE_",  
> UtilProperties 
> .getMessage 
> (resource_error 
> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpProductIdPassed",  
> locale));
>             return "error";
>         }
>         if (UtilValidate.isEmpty(alternateGwpLineStr)) {
> -        	request.setAttribute("_ERROR_MESSAGE_",  
> UtilProperties 
> .getMessage 
> (resource_error 
> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpLinePassed", locale));
> +            request.setAttribute("_ERROR_MESSAGE_",  
> UtilProperties 
> .getMessage 
> (resource_error 
> ,"OrderCouldNotSelectAlternateGiftNoAlternateGwpLinePassed", locale));
>             return "error";
>         }
>
> @@ -952,13 +952,13 @@
>         try {
>             alternateGwpLine = Integer.parseInt(alternateGwpLineStr);
>         } catch (Exception e) {
> -        	request.setAttribute("_ERROR_MESSAGE_",  
> UtilProperties 
> .getMessage 
> (resource_error 
> ,"OrderCouldNotSelectAlternateGiftAlternateGwpLineIsNotAValidNumber 
> ", locale));
> +            request.setAttribute("_ERROR_MESSAGE_",  
> UtilProperties 
> .getMessage 
> (resource_error 
> ,"OrderCouldNotSelectAlternateGiftAlternateGwpLineIsNotAValidNumber 
> ", locale));
>             return "error";
>         }
>
>         ShoppingCartItem cartLine =  
> cart.findCartItem(alternateGwpLine);
>         if (cartLine == null) {
> -        	request.setAttribute("_ERROR_MESSAGE_", "Could not select  
> alternate gift, no cart line item found for #" + alternateGwpLine +  
> ".");
> +            request.setAttribute("_ERROR_MESSAGE_", "Could not  
> select alternate gift, no cart line item found for #" +  
> alternateGwpLine + ".");
>             return "error";
>         }
>
> @@ -994,7 +994,7 @@
>         int i;
>
>         if (UtilValidate.isEmpty(partyId) || roleTypeId.length < 1) {
> -        	request.setAttribute("_ERROR_MESSAGE_",  
> UtilProperties 
> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined",  
> locale));
> +            request.setAttribute("_ERROR_MESSAGE_",  
> UtilProperties 
> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined",  
> locale));
>             return "error";
>         }
>
> @@ -1025,7 +1025,7 @@
>         int i;
>
>         if (UtilValidate.isEmpty(partyId) || roleTypeId.length < 1) {
> -        	request.setAttribute("_ERROR_MESSAGE_",  
> UtilProperties 
> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined",  
> locale));
> +            request.setAttribute("_ERROR_MESSAGE_",  
> UtilProperties 
> .getMessage(resource_error,"OrderPartyIdAndOrRoleTypeIdNotDefined",  
> locale));
>             return "error";
>         }
>
> Index: applications/order/src/org/ofbiz/order/shoppingcart/ 
> ShoppingCartHelper.java
> ===================================================================
> --- applications/order/src/org/ofbiz/order/shoppingcart/ 
> ShoppingCartHelper.java	(revision 729034)
> +++ applications/order/src/org/ofbiz/order/shoppingcart/ 
> ShoppingCartHelper.java	(working copy)
> @@ -107,7 +107,7 @@
>         return  
> addToCart(catalogId,shoppingListId,shoppingListItemSeqId,productId,
>                  
> productCategoryId,itemType,itemDescription,price,amount,quantity,
>                  
> reservStart 
> ,reservLength,reservPersons,null,null,shipBeforeDate,shipAfterDate,
> -                 
> configWrapper,itemGroupNumber,context,parentProductId);
> +                 
> configWrapper,itemGroupNumber,context,parentProductId, true);
>     }
>
>     /** Event to add an item to the shopping cart with  
> accommodation. */
> @@ -116,7 +116,7 @@
>             Double price, Double amount, double quantity,
>             java.sql.Timestamp reservStart, Double reservLength,  
> Double reservPersons, String accommodationMapId,String  
> accommodationSpotId,
>             java.sql.Timestamp shipBeforeDate, java.sql.Timestamp  
> shipAfterDate,
> -            ProductConfigWrapper configWrapper, String  
> itemGroupNumber, Map context, String parentProductId) {
> +            ProductConfigWrapper configWrapper, String  
> itemGroupNumber, Map context, String parentProductId, boolean  
> checkItemsExists) {
>         Map result = null;
>         Map attributes = null;
>         String pProductId = null;
> @@ -234,7 +234,7 @@
>
>                        itemId = cart.addOrIncreaseItem(productId,  
> amount, quantity, reservStart, reservLength,
>                                                 reservPersons,  
> accommodationMapId, accommodationSpotId, shipBeforeDate,  
> shipAfterDate, additionalFeaturesMap, attributes,
> -                                                catalogId,  
> configWrapper, itemType, itemGroupNumber, pProductId, dispatcher);
> +                                                catalogId,  
> configWrapper, itemType, itemGroupNumber, pProductId, dispatcher,  
> checkItemsExists);
>
>             } else {
>                 itemId = cart.addNonProductItem(itemType,  
> itemDescription, productCategoryId, price, quantity, attributes,  
> catalogId, itemGroupNumber, dispatcher);
> @@ -678,16 +678,16 @@
>                     } else if  
> (parameterName.toUpperCase().startsWith("DESCRIPTION")) {
>                         itemDescription = quantString;  // the  
> quantString is actually the description if the field name starts  
> with DESCRIPTION
>                     } else if  
> (parameterName.startsWith("reservStart")) {
> -                    	if (quantString.length() ==0){
> -                    		// should have format: yyyy-mm-dd  
> hh:mm:ss.fffffffff
> -                    		quantString += " 00:00:00.000000000";
> -                    	}
> -                    	if (item != null) {
> -                    		Timestamp reservStart =  
> Timestamp.valueOf(quantString);
> -                    		item.setReservStart(reservStart);
> -                    	}
> +                        if (quantString.length() ==0){
> +                            // should have format: yyyy-mm-dd  
> hh:mm:ss.fffffffff
> +                            quantString += " 00:00:00.000000000";
> +                        }
> +                        if (item != null) {
> +                            Timestamp reservStart =  
> Timestamp.valueOf(quantString);
> +                            item.setReservStart(reservStart);
> +                        }
>                     } else if  
> (parameterName.startsWith("reservLength")) {
> -                    	if (item != null) {
> +                        if (item != null) {
>                             double reservLength =  
> nf.parse(quantString).doubleValue();
>                             item.setReservLength(reservLength);
>                         }
> Index: applications/order/src/org/ofbiz/order/shoppinglist/ 
> ShoppingListEvents.java
> ===================================================================
> --- applications/order/src/org/ofbiz/order/shoppinglist/ 
> ShoppingListEvents.java	(revision 729034)
> +++ applications/order/src/org/ofbiz/order/shoppinglist/ 
> ShoppingListEvents.java	(working copy)
> @@ -138,7 +138,7 @@
>             try {
>                 cartIdInt = new Integer(items[i]);
>             } catch (Exception e) {
> -            	Debug.logWarning(e,  
> UtilProperties 
> .getMessage 
> (resource_error,"OrderIllegalCharacterInSelectedItemField",  
> cart.getLocale()), module);
> +                Debug.logWarning(e,  
> UtilProperties 
> .getMessage 
> (resource_error,"OrderIllegalCharacterInSelectedItemField",  
> cart.getLocale()), module);
>             }
>             if (cartIdInt != null) {
>                 ShoppingCartItem item =  
> cart.findCartItem(cartIdInt.intValue());
> @@ -292,18 +292,18 @@
>                 if (reservStart == null) {
>                        cart.addOrIncreaseItem(productId, null,  
> quantity.doubleValue(), null, null, null, null, null, null,  
> attributes, prodCatalogId, configWrapper, null, null, null,  
> dispatcher);
>                 }else{
> -                    cart.addOrIncreaseItem(productId, null,  
> quantity.doubleValue(), reservStart, reservLength, reservPersons,  
> null, null, null, null, null, attributes, prodCatalogId,  
> configWrapper, null, null, null, dispatcher);
> +                    cart.addOrIncreaseItem(productId, null,  
> quantity.doubleValue(), reservStart, reservLength, reservPersons,  
> null, null, null, null, null, attributes, prodCatalogId,  
> configWrapper, null, null, null, dispatcher, true);
>                 }
>                 Map messageMap = UtilMisc.toMap("productId",  
> productId);
>                 errMsg =  
> UtilProperties 
> .getMessage(resource,"shoppinglistevents.added_product_to_cart",  
> messageMap, cart.getLocale());
>                 eventMessage.append(errMsg + "\n");
>             } catch (CartItemModifyException e) {
> -            	Debug.logWarning(e,  
> UtilProperties 
> .getMessage(resource_error,"OrderProblemsAddingItemFromListToCart",  
> cart.getLocale()));
> +                Debug.logWarning(e,  
> UtilProperties 
> .getMessage(resource_error,"OrderProblemsAddingItemFromListToCart",  
> cart.getLocale()));
>                 Map messageMap = UtilMisc.toMap("productId",  
> productId);
>                 errMsg =  
> UtilProperties 
> .getMessage 
> (resource,"shoppinglistevents.problem_adding_product_to_cart",  
> messageMap, cart.getLocale());
>                 eventMessage.append(errMsg + "\n");
>             } catch (ItemNotFoundException e) {
> -            	Debug.logWarning(e,  
> UtilProperties.getMessage(resource_error,"OrderProductNotFound",  
> cart.getLocale()));
> +                Debug.logWarning(e,  
> UtilProperties.getMessage(resource_error,"OrderProductNotFound",  
> cart.getLocale()));
>                 Map messageMap = UtilMisc.toMap("productId",  
> productId);
>                 errMsg =  
> UtilProperties 
> .getMessage 
> (resource,"shoppinglistevents.problem_adding_product_to_cart",  
> messageMap, cart.getLocale());
>                 eventMessage.append(errMsg + "\n");
> @@ -343,7 +343,7 @@
>         try {
>             result = dispatcher.runSync("updateShoppingListItem",  
> serviceInMap);
>         } catch (GenericServiceException e) {
> -        	String errMsg =  
> UtilProperties 
> .getMessage 
> (ShoppingListEvents 
> .err_resource,"shoppingListEvents.error_calling_update", locale) +  
> ": "  + e.toString();
> +            String errMsg =  
> UtilProperties 
> .getMessage 
> (ShoppingListEvents 
> .err_resource,"shoppingListEvents.error_calling_update", locale) +  
> ": "  + e.toString();
>             request.setAttribute("_ERROR_MESSAGE_", errMsg);
>             String errorMsg = "Error calling the  
> updateShoppingListItem in handleShoppingListItemVariant: " +  
> e.toString();
>             Debug.logError(e, errorMsg, module);
> Index: applications/order/src/org/ofbiz/order/shoppinglist/ 
> ShoppingListServices.java
> ===================================================================
> --- applications/order/src/org/ofbiz/order/shoppinglist/ 
> ShoppingListServices.java	(revision 729034)
> +++ applications/order/src/org/ofbiz/order/shoppinglist/ 
> ShoppingListServices.java	(working copy)
> @@ -416,7 +416,7 @@
>      * @return
>      */
>     public static ShoppingCart makeShoppingListCart(LocalDispatcher  
> dispatcher, GenericValue shoppingList, Locale locale) {
> -    	return makeShoppingListCart(null, dispatcher, shoppingList,  
> locale); }
> +        return makeShoppingListCart(null, dispatcher, shoppingList,  
> locale); }
>
>     /**
>      * Add a shoppinglist to an existing shoppingcart
> @@ -451,20 +451,20 @@
>             }
>
>             if (UtilValidate.isNotEmpty(items)) {
> -            	if (listCart == null) {
> -            		listCart = new ShoppingCart(delegator,  
> productStoreId, locale, currencyUom);
> -            		 
> listCart.setOrderPartyId(shoppingList.getString("partyId"));
> -            		 
> listCart 
> .setAutoOrderShoppingListId(shoppingList.getString("shoppingListId"));
> -            	} else {
> -            		if (! 
> listCart.getPartyId().equals(shoppingList.getString("partyId"))){
> -            			Debug.logError("CANNOT add shoppingList: " +  
> shoppingList.getString("shoppingListId")
> -            					+ " of partyId: " +  
> shoppingList.getString("partyId")
> -            					+ " to a shoppingcart with a different  
> orderPartyId: "
> -            					+ listCart.getPartyId(), module);
> -                		return listCart;
> -            		}
> -            	}
> -            	
> +                if (listCart == null) {
> +                    listCart = new ShoppingCart(delegator,  
> productStoreId, locale, currencyUom);
> +                     
> listCart.setOrderPartyId(shoppingList.getString("partyId"));
> +                     
> listCart 
> .setAutoOrderShoppingListId(shoppingList.getString("shoppingListId"));
> +                } else {
> +                    if (! 
> listCart.getPartyId().equals(shoppingList.getString("partyId"))){
> +                        Debug.logError("CANNOT add shoppingList: "  
> + shoppingList.getString("shoppingListId")
> +                                + " of partyId: " +  
> shoppingList.getString("partyId")
> +                                + " to a shoppingcart with a  
> different orderPartyId: "
> +                                + listCart.getPartyId(), module);
> +                        return listCart;
> +                    }
> +                }
> +
>
>                 Iterator i = items.iterator();
>                 ProductConfigWrapper configWrapper = null;
> @@ -502,7 +502,7 @@
>                         Map attributes =  
> UtilMisc.toMap("shoppingListId", listId, "shoppingListItemSeqId",  
> itemId);
>
>                         try {
> -                            listCart.addOrIncreaseItem(productId,  
> null, quantity.doubleValue(), reservStart, reservLength,  
> reservPersons, null, null, null, null, null, attributes, null,  
> configWrapper, null, null, null, dispatcher);
> +                            listCart.addOrIncreaseItem(productId,  
> null, quantity.doubleValue(), reservStart, reservLength,  
> reservPersons, null, null, null, null, null, attributes, null,  
> configWrapper, null, null, null, dispatcher, true);
>                         } catch (CartItemModifyException e) {
>                             Debug.logError(e, "Unable to add product  
> to List Cart - " + productId, module);
>                         } catch (ItemNotFoundException e) {


Re: ShoppingCart.addOrIncreaseItem

Posted by Stephen Rufle <sr...@salmonllc.com>.
Added checkItemExists to ShoppingCart.addOrIncreaseItem(... , boolean
checkItemsExists)

I wanted to have the group review my patch before I created a JIRA. I am
new to patching so I see in my patch that it is changing tabs to spaces
and adding the new code and applies to multiple files all in one patch,
previously I have sent patches that were file specific.

I am using eclipse to generate patches, but I would have liked to
generate two. One for the tab to spaces and another to show what I
added. Any advice would be appropriated.

If the patch is good as is I will create a JIRA and attach.

David E Jones wrote:
>
> My favorite current proposal is to super-simplify that method and then
> on the ShoppingCartItem object that it returns just call additional
> methods to add additional data. That way now, and in the future, we
> only have to add setters to ShoppingCartItem to support new data there.
>
> -David
>
>
> On Dec 22, 2008, at 3:51 PM, Stephen Rufle wrote:
>
>> I would like to add a parameter to ShoppingCart.addOrIncreaseItem(... ,
>> boolean checkItemsExists)
>>
>> I see that the method signature is already pretty long. I would like to
>> know if instead would it be a better idea to pass along a Map object
>> around instead?
>>
>> If there is already an existing effort to reduce the number of
>> parameters I would like to see how you are dealing with it.
>>
>>
>
>
>

-- 
Stephen P Rufle
srufle@salmonllc.com
H1:480-626-8022
H2:480-802-7173
Yahoo IM: stephen_rufle
AOL IM: stephen1rufle


Re: ShoppingCart.addOrIncreaseItem

Posted by David E Jones <da...@hotwaxmedia.com>.
My favorite current proposal is to super-simplify that method and then  
on the ShoppingCartItem object that it returns just call additional  
methods to add additional data. That way now, and in the future, we  
only have to add setters to ShoppingCartItem to support new data there.

-David


On Dec 22, 2008, at 3:51 PM, Stephen Rufle wrote:

> I would like to add a parameter to  
> ShoppingCart.addOrIncreaseItem(... ,
> boolean checkItemsExists)
>
> I see that the method signature is already pretty long. I would like  
> to
> know if instead would it be a better idea to pass along a Map object
> around instead?
>
> If there is already an existing effort to reduce the number of
> parameters I would like to see how you are dealing with it.
>
>