You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2014/01/03 14:30:38 UTC

Re: svn commit: r1554381 - in /ofbiz/branches/release13.07: ./ applications/order/src/org/ofbiz/order/shoppingcart/ applications/order/src/org/ofbiz/order/shoppingcart/product/

Thanks Jacopo for the clear explanation!

Jacques

On Tuesday, December 31, 2013 8:44 AM jacopoc@apache.org wrote
> Author: jacopoc
> Date: Tue Dec 31 07:44:19 2013
> New Revision: 1554381
> 
> URL: http://svn.apache.org/r1554381
> Log:
> Applied fix from trunk for revision: 1554265
> ===
> 
> This is a refactoring of the product promotion engine in order to overcome some limitations that prevented it to select and apply
> the best set of promotions under special conditions. 
> 
> Example: Consider two promotions:
> * consider two products: Product A, with price $20, and Product B, with price $40
> * Promotion 1: 20% discount on all the products in a category containing Product A and Product B
> * Promotion 2: 40% discount on Product A
> 
> When Product A and Product B are both in the cart:
> * Expected behavior: on Product A the Promotion 2 should be applied i.e. 40% discount, and on Product B Promotion 1 should be
> applied i.e. 20% discount. 
> ** Summary
> Product Price Discount Subtotal
> A $20 $8 (40% discount) $12
> B $40 $8 (20% discount) $32
> Total Adjustment: $16
> 
> * OFBiz behavior (before this fix): Promotion 1 is applied to Product A and Product B; this happens because the total discount of
> Promotion 1 is greater than the total discount of Promotion 2 and OFBiz applies promotions sorted by discount (desc) 
> ** Summary
> Product Price Discount Subtotal
> A $20 $4 (20% discount) $16
> B $40 $8 (20% discount) $32
> Total Adjustment: $12
> 
> The new solution fixes this issue and similar ones.
> 
> Here are some details about the new algorithm.
> 
> Overview of the flow:
> 1) run the promotions one by one in a test run
> 2) collect the ProductPromoUse information
> 3) sort them by weight (i.e. the ratio between the discount and the value of the products discounted)
> 4) execute the ProductPromoUse in the given order
> 
> In order to understand this solution, and specifically the changes to ProductPromoWorker.java, there is an important concept to
> consider: 
> one Promotion can generate more than one ProductPromoUseInfo objects.
> For example if I have 2 units of WG-1111 in the cart (in one cart item) and I have the promotion “20% discount on WG-1111 and
> GZ-1000” then the system will create TWO ProductPromoUseInfo objects both associated to the same promotion one for each of the 2
> units discounted.  
> Similarly if I had two lines: 2 units of WG-1111 and 1 unit of GZ-1000 I would get 3 ProductPromoUseInfo objects 2 objects for
> WG-1111 and 1 object for GZ-1000 
> 
> We can sort these ProductPromoUseInfo objects based on their weight (i.e. the ratio between the discount and the value of the
> products discounted) in desc order 
> and now we have a sorted list of ProductPromoUseInfo objects ready to be executed
> However we only want to execute each of them once and for this reason we set (in memory, not in the DB) the useLimitPerOrder to 1
> in the first ProductPromoUseInfo of a given promotion and then to 2 if the same ProductPromoUseInfo is associated to the same
> promotion etc...  
> in this way the end result is that the system will generate, as we desire, ONE ProductPromoUseInfo only for each of the
> ProductPromoUseInfo in the list. 
> 
> Here is an example:
> we have 2 promotions:
> PROMO A
> PROMO B
> 
> After test run:
> 
> ProductPromoUseInfo - PROMO A - #1 - weight 0.3
> ProductPromoUseInfo - PROMO A - #2 - weight 0.3
> ProductPromoUseInfo - PROMO B - #1 - weight 0.4
> 
> After sorting:
> 
> ProductPromoUseInfo - PROMO B - #1 - weight 0.4
> ProductPromoUseInfo - PROMO A - #1 - weight 0.3
> ProductPromoUseInfo - PROMO A - #2 - weight 0.3
> 
> Based on this we create a list (sortedExplodedProductPromoList) of ProductPromo:
> 
> PROMO B - with useLimitPerOrder=1
> PROMO A - with useLimitPerOrder=1
> PROMO A - with useLimitPerOrder=2
> 
> When we apply these to the cart we get the following results:
> 
> PROMO B - with useLimitPerOrder=1 APPLIED
> PROMO A - with useLimitPerOrder=1 APPLIED
> PROMO A - with useLimitPerOrder=2 NOT APPLIED (because PROMO B used the item)
> 
> 
> 
> Modified:
>    ofbiz/branches/release13.07/   (props changed)
>    ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java
>    ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java
>    ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java
> 
> Propchange: ofbiz/branches/release13.07/
> ------------------------------------------------------------------------------
>  Merged /ofbiz/trunk:r1554265
> 
> Modified: ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java?rev=1554381&r1=1554380&r2=1554381&view=diff
> ============================================================================== ---
> ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java (original) +++
> ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java Tue Dec 31 07:44:19 2013 @@
>             -2714,7 +2714,7 @@ public class ShoppingCart implements Ite }
>             itemsTotal = itemsTotal.add(cartItem.getItemSubTotal());
>         }
> -        return itemsTotal;
> +        return itemsTotal.add(this.getOrderOtherAdjustmentTotal());
>     }
> 
>     /**
> @@ -3142,12 +3142,12 @@ public class ShoppingCart implements Ite
>         return new HashMap<GenericPK, String>(this.desiredAlternateGiftByAction);
>     }
> 
> -    public void addProductPromoUse(String productPromoId, String productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal
> quantityLeftInActions) { +    public void addProductPromoUse(String productPromoId, String productPromoCodeId, BigDecimal
>         totalDiscountAmount, BigDecimal quantityLeftInActions, Map<ShoppingCartItem,BigDecimal> usageInfoMap) { if
>             (UtilValidate.isNotEmpty(productPromoCodeId) && !this.productPromoCodes.contains(productPromoCodeId)) { throw new
>         IllegalStateException("Cannot add a use to a promo code use for a code that has not been entered."); }
>         if (Debug.verboseOn()) Debug.logVerbose("Used promotion [" + productPromoId + "] with code [" + productPromoCodeId + "]
> for total discount [" + totalDiscountAmount + "] and quantity left in actions [" + quantityLeftInActions + "]", module); -       
> this.productPromoUseInfoList.add(new ProductPromoUseInfo(productPromoId, productPromoCodeId, totalDiscountAmount,
>     quantityLeftInActions)); +        this.productPromoUseInfoList.add(new ProductPromoUseInfo(productPromoId,
> productPromoCodeId, totalDiscountAmount, quantityLeftInActions, usageInfoMap)); } 
> 
>     public void removeProductPromoUse(String productPromoId) {
> @@ -4385,23 +4385,43 @@ public class ShoppingCart implements Ite
>         }
>     }
> 
> -    public static class ProductPromoUseInfo implements Serializable {
> +    public static class ProductPromoUseInfo implements Serializable, Comparable<ProductPromoUseInfo> {
>         public String productPromoId = null;
>         public String productPromoCodeId = null;
>         public BigDecimal totalDiscountAmount = BigDecimal.ZERO;
>         public BigDecimal quantityLeftInActions = BigDecimal.ZERO;
> +        private Map<ShoppingCartItem,BigDecimal> usageInfoMap = null;
> 
> -        public ProductPromoUseInfo(String productPromoId, String productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal
> quantityLeftInActions) { +        public ProductPromoUseInfo(String productPromoId, String productPromoCodeId, BigDecimal
>             totalDiscountAmount, BigDecimal quantityLeftInActions, Map<ShoppingCartItem,BigDecimal> usageInfoMap) {
>             this.productPromoId = productPromoId; this.productPromoCodeId = productPromoCodeId;
>             this.totalDiscountAmount = totalDiscountAmount;
>             this.quantityLeftInActions = quantityLeftInActions;
> +            this.usageInfoMap = usageInfoMap;
>         }
> 
>         public String getProductPromoId() { return this.productPromoId; }
>         public String getProductPromoCodeId() { return this.productPromoCodeId; }
>         public BigDecimal getTotalDiscountAmount() { return this.totalDiscountAmount; }
>         public BigDecimal getQuantityLeftInActions() { return this.quantityLeftInActions; }
> +        public Map<ShoppingCartItem,BigDecimal> getUsageInfoMap() { return this.usageInfoMap; }
> +        public BigDecimal getUsageWeight() {
> +            Iterator<ShoppingCartItem> lineItems = this.usageInfoMap.keySet().iterator();
> +            BigDecimal totalAmount = BigDecimal.ZERO;
> +            while (lineItems.hasNext()) {
> +                ShoppingCartItem lineItem = lineItems.next();
> +                totalAmount = totalAmount.add(lineItem.getBasePrice().multiply(usageInfoMap.get(lineItem)));
> +            }
> +            if (totalAmount.compareTo(BigDecimal.ZERO) == 0) {
> +                return BigDecimal.ZERO;
> +            } else {
> +                return getTotalDiscountAmount().negate().divide(totalAmount);
> +            }
> +        }
> +
> +        public int compareTo(ProductPromoUseInfo other) {
> +            return other.getUsageWeight().compareTo(getUsageWeight());
> +        }
>     }
> 
>     public static class CartShipInfo implements Serializable {
> 
> Modified: ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java?rev=1554381&r1=1554380&r2=1554381&view=diff
> ============================================================================== ---
> ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java (original) +++
> ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java Tue Dec 31 07:44:19
> 2013 @@ -21,6 +21,7 @@ package org.ofbiz.order.shoppingcart; 
> import java.math.BigDecimal;
> import java.math.MathContext;
> import java.sql.Timestamp;
> +import java.util.HashMap;
> import java.util.Iterator;
> import java.util.List;
> import java.util.Locale;
> @@ -629,7 +630,7 @@ public class ShoppingCartServices {
>                 cart.addProductPromoCode(productPromoCode, dispatcher);
>             }
>             for (GenericValue productPromoUse: orh.getProductPromoUse()) {
> -                cart.addProductPromoUse(productPromoUse.getString("productPromoId"),
> productPromoUse.getString("productPromoCodeId"), productPromoUse.getBigDecimal("totalDiscountAmount"),
>             productPromoUse.getBigDecimal("quantityLeftInActions")); +               
>         cart.addProductPromoUse(productPromoUse.getString("productPromoId"), productPromoUse.getString("productPromoCodeId"),
> productPromoUse.getBigDecimal("totalDiscountAmount"), productPromoUse.getBigDecimal("quantityLeftInActions"), new
> HashMap<ShoppingCartItem, BigDecimal>()); } }  
> 
> 
> Modified: ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java?rev=1554381&r1=1554380&r2=1554381&view=diff
> ============================================================================== ---
> ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java (original) +++
> ofbiz/branches/release13.07/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java Tue Dec 31
> 07:44:19 2013 @@ -22,6 +22,8 @@ import java.math.BigDecimal; 
> import java.math.MathContext;
> import java.sql.Timestamp;
> import java.util.ArrayList;
> +import java.util.Collections;
> +import java.util.HashMap;
> import java.util.Iterator;
> import java.util.List;
> import java.util.Locale;
> @@ -51,6 +53,7 @@ import org.ofbiz.entity.condition.Entity
> import org.ofbiz.entity.util.EntityUtil;
> import org.ofbiz.order.shoppingcart.CartItemModifyException;
> import org.ofbiz.order.shoppingcart.ShoppingCart;
> +import org.ofbiz.order.shoppingcart.ShoppingCart.ProductPromoUseInfo;
> import org.ofbiz.order.shoppingcart.ShoppingCartEvents;
> import org.ofbiz.order.shoppingcart.ShoppingCartItem;
> import org.ofbiz.product.product.ProductContentWrapper;
> @@ -318,44 +321,62 @@ public class ProductPromoWorker {
>             // do a calculate only run through the promotions, then order by descending totalDiscountAmount for each promotion
>             // NOTE: on this run, with isolatedTestRun passed as false it should not apply any adjustments
>             //  or track which cart items are used for which promotions, but it will track ProductPromoUseInfo and
> -            //  useLimits; we are basicly just trying to run each promo "independently" to see how much each is worth
> +            //  useLimits; we are basically just trying to run each promo "independently" to see how much each is worth
>             runProductPromos(productPromoList, cart, delegator, dispatcher, nowTimestamp, true);
> 
> -            // NOTE: after that first pass we could remove any that have a 0 totalDiscountAmount from the run list, but we won't
> because by the time they are run the cart may have changed enough to get them to go; also, certain actions like free shipping
> should always be run even though we won't know what the totalDiscountAmount is at the time the promotion is run  
> -            // each ProductPromoUseInfo on the shopping cart will contain it's total value, so add up all totals for each
> promoId and put them in a List of Maps 
> -            // create a List of Maps with productPromo and totalDiscountAmount, use the Map sorter to sort them descending by
> totalDiscountAmount -
> -            // before sorting split into two lists and sort each list; one list for promos that have a order total condition,
> and the other list for all promos that don't; then we'll always run the ones that have no condition on the order total first 
> -            List<Map<Object, Object>> productPromoDiscountMapList = FastList.newInstance();
> -            List<Map<Object, Object>> productPromoDiscountMapListOrderTotal = FastList.newInstance();
> +            // NOTE: we can easily recognize the promos for the order total: they are the ones with usage set to 0
> +            Iterator<ProductPromoUseInfo> promoUses = cart.getProductPromoUseInfoIter();
> +            List<ProductPromoUseInfo> sortedPromoUses = new ArrayList<ProductPromoUseInfo>();
> +            while (promoUses.hasNext()) {
> +                ProductPromoUseInfo promoUse = promoUses.next();
> +                sortedPromoUses.add(promoUse);
> +            }
> +            Collections.sort(sortedPromoUses);
> +            List<GenericValue> sortedExplodedProductPromoList = new ArrayList<GenericValue>(sortedPromoUses.size());
> +            Map<String, Long> usesPerPromo = new HashMap<String, Long>();
> +            int indexOfFirstOrderTotalPromo = -1;
> +            for (ProductPromoUseInfo promoUse: sortedPromoUses) {
> +                GenericValue productPromo = delegator.findOne("ProductPromo", UtilMisc.toMap("productPromoId",
> promoUse.getProductPromoId()), true); +                GenericValue newProductPromo = (GenericValue)productPromo.clone();
> +                if (!usesPerPromo.containsKey(promoUse.getProductPromoId())) {
> +                    usesPerPromo.put(promoUse.getProductPromoId(), 0l);
> +                }
> +                long uses = usesPerPromo.get(promoUse.getProductPromoId());
> +                uses = uses + 1;
> +                long useLimitPerOrder = (newProductPromo.get("useLimitPerOrder") != null?
> newProductPromo.getLong("useLimitPerOrder"): -1); +                if (useLimitPerOrder == -1 || uses < useLimitPerOrder) {
> +                    newProductPromo.set("useLimitPerOrder", uses);
> +                }
> +                usesPerPromo.put(promoUse.getProductPromoId(), uses);
> +                sortedExplodedProductPromoList.add(newProductPromo);
> +                if (indexOfFirstOrderTotalPromo == -1 && BigDecimal.ZERO.equals(promoUse.getUsageWeight())) {
> +                    indexOfFirstOrderTotalPromo = sortedExplodedProductPromoList.size() - 1;
> +                }
> +            }
> +            if (indexOfFirstOrderTotalPromo == -1) {
> +                indexOfFirstOrderTotalPromo = sortedExplodedProductPromoList.size() - 1;
> +            }
> +
>             for(GenericValue productPromo : productPromoList) {
>                 Map<Object, Object> productPromoDiscountMap = UtilGenerics.checkMap(UtilMisc.toMap("productPromo", productPromo,
>                 "totalDiscountAmount", cart.getProductPromoUseTotalDiscount(productPromo.getString("productPromoId")))); if
> (hasOrderTotalCondition(productPromo, delegator)) { -                   
> productPromoDiscountMapListOrderTotal.add(productPromoDiscountMap); +                    if
> (!usesPerPromo.containsKey(productPromo.getString("productPromoId"))) { +                       
> sortedExplodedProductPromoList.add(productPromo); +                    }
>                 } else {
> -                    productPromoDiscountMapList.add(productPromoDiscountMap);
> +                    if (!usesPerPromo.containsKey(productPromo.getString("productPromoId"))) {
> +                        if (indexOfFirstOrderTotalPromo != -1) {
> +                            sortedExplodedProductPromoList.add(indexOfFirstOrderTotalPromo, productPromo);
> +                        } else {
> +                            sortedExplodedProductPromoList.add(0, productPromo);
> +                        }
> +                    }
>                 }
>             }
> 
> -
> -            // sort the Map List, do it ascending because the discount amounts will be negative, so the lowest number is really
> the highest discount 
> -            productPromoDiscountMapList = UtilMisc.sortMaps(productPromoDiscountMapList,
> UtilMisc.toList("+totalDiscountAmount")); 
> -            productPromoDiscountMapListOrderTotal = UtilMisc.sortMaps(productPromoDiscountMapListOrderTotal,
> UtilMisc.toList("+totalDiscountAmount")); -
> -            productPromoDiscountMapList.addAll(productPromoDiscountMapListOrderTotal);
> -
> -            List<GenericValue> sortedProductPromoList = new ArrayList<GenericValue>(productPromoDiscountMapList.size());
> -            Iterator<Map<Object, Object>> productPromoDiscountMapIter = productPromoDiscountMapList.iterator();
> -            while (productPromoDiscountMapIter.hasNext()) {
> -                Map<Object, Object> productPromoDiscountMap = UtilGenerics.checkMap(productPromoDiscountMapIter.next());
> -                GenericValue productPromo = (GenericValue) productPromoDiscountMap.get("productPromo");
> -                sortedProductPromoList.add(productPromo);
> -                if (Debug.verboseOn()) Debug.logVerbose("Sorted Promo [" + productPromo.getString("productPromoId") + "] with
> total discount: " + productPromoDiscountMap.get("totalDiscountAmount"), module); 
> -            }
> -
>             // okay, all ready, do the real run, clearing the temporary result first...
>             cart.clearAllPromotionInformation();
> -            runProductPromos(sortedProductPromoList, cart, delegator, dispatcher, nowTimestamp, false);
> +            runProductPromos(sortedExplodedProductPromoList, cart, delegator, dispatcher, nowTimestamp, false);
>         } catch (NumberFormatException e) {
>             Debug.logError(e, "Number not formatted correctly in promotion rules, not completed...", module);
>         } catch (GenericEntityException e) {
> @@ -436,7 +457,7 @@ public class ProductPromoWorker {
>                                     GenericValue productPromoCode = productPromoCodeIter.next();
>                                     String productPromoCodeId = productPromoCode.getString("productPromoCodeId");
>                                     Long codeUseLimit = getProductPromoCodeUseLimit(productPromoCode, partyId, delegator);
> -                                    if (runProductPromoRules(cart, cartChanged, useLimit, true, productPromoCodeId,
> codeUseLimit, maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) { +                            
>                                         if (runProductPromoRules(cart, useLimit, true, productPromoCodeId, codeUseLimit,
>                                     maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) {
> cartChanged = true; } 
> 
> @@ -448,7 +469,7 @@ public class ProductPromoWorker {
>                             }
>                         } else {
>                             try {
> -                                if (runProductPromoRules(cart, cartChanged, useLimit, false, null, null, maxUseLimit,
> productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) { +                                if
>                                     (runProductPromoRules(cart, useLimit, false, null, null, maxUseLimit, productPromo,
>                                 productPromoRules, dispatcher, delegator, nowTimestamp)) { cartChanged = true; }
>                             } catch (RuntimeException e) {
> @@ -735,8 +756,10 @@ public class ProductPromoWorker {
>         return promoDescBuf.toString();
>     }
> 
> -    protected static boolean runProductPromoRules(ShoppingCart cart, boolean cartChanged, Long useLimit, boolean requireCode,
> String productPromoCodeId, Long codeUseLimit, long maxUseLimit, +    protected static boolean runProductPromoRules(ShoppingCart
>         cart, Long useLimit, boolean requireCode, String productPromoCodeId, Long codeUseLimit, long maxUseLimit, GenericValue
> productPromo, List<GenericValue> productPromoRules, LocalDispatcher dispatcher, Delegator delegator, Timestamp nowTimestamp)
> throws GenericEntityException, UseLimitException { +        boolean cartChanged = false; +       
>         Map<ShoppingCartItem,BigDecimal> usageInfoMap = prepareProductUsageInfoMap(cart); String productPromoId =
>         productPromo.getString("productPromoId"); while ((useLimit == null || useLimit.longValue() >
>                 cart.getProductPromoUseCount(productPromoId)) && (!requireCode || UtilValidate.isNotEmpty(productPromoCodeId)) &&
> @@ -755,17 +778,17 @@ public class ProductPromoWorker {
>                 // loop through conditions for rule, if any false, set allConditionsTrue to false
>                 List<GenericValue> productPromoConds = delegator.findByAnd("ProductPromoCond", UtilMisc.toMap("productPromoId",
>                 productPromo.get("productPromoId")), UtilMisc.toList("productPromoCondSeqId"), true); productPromoConds =
> EntityUtil.filterByAnd(productPromoConds, UtilMisc.toMap("productPromoRuleId", productPromoRule.get("productPromoRuleId"))); -   
> // using the other method to consolodate cache entries because the same cache is used elsewhere: List productPromoConds =
>                 productPromoRule.getRelated("ProductPromoCond", null, UtilMisc.toList("productPromoCondSeqId"), true); +         
> // using the other method to consolidate cache entries because the same cache is used elsewhere: List productPromoConds =
> productPromoRule.getRelated("ProductPromoCond", null, UtilMisc.toList("productPromoCondSeqId"), true); if (Debug.verboseOn())
> Debug.logVerbose("Checking " + productPromoConds.size() + " conditions for rule " + productPromoRule, module);   
> 
>                 Iterator<GenericValue> productPromoCondIter = UtilMisc.toIterator(productPromoConds);
>                 while (productPromoCondIter != null && productPromoCondIter.hasNext()) {
>                     GenericValue productPromoCond = productPromoCondIter.next();
> 
> -                    boolean condResult = checkCondition(productPromoCond, cart, delegator, dispatcher, nowTimestamp);
> +                    boolean conditionSatisfied = checkCondition(productPromoCond, cart, delegator, dispatcher, nowTimestamp);
> 
>                     // any false condition will cause it to NOT perform the action
> -                    if (condResult == false) {
> +                    if (!conditionSatisfied) {
>                         performActions = false;
>                         break;
>                     }
> @@ -797,13 +820,16 @@ public class ProductPromoWorker {
>             }
> 
>             if (promoUsed) {
> -                cart.addProductPromoUse(productPromo.getString("productPromoId"), productPromoCodeId, totalDiscountAmount,
> quantityLeftInActions); +                // Get product use information from the cart
> +                Map<ShoppingCartItem,BigDecimal> newUsageInfoMap = prepareProductUsageInfoMap(cart);
> +                Map<ShoppingCartItem,BigDecimal> deltaUsageInfoMap = prepareDeltaProductUsageInfoMap(usageInfoMap,
> newUsageInfoMap); +                usageInfoMap = newUsageInfoMap;
> +                cart.addProductPromoUse(productPromo.getString("productPromoId"), productPromoCodeId, totalDiscountAmount,
>             quantityLeftInActions, deltaUsageInfoMap); } else {
>                 // the promotion was not used, don't try again until we finish a full pass and come back to see the promo
>                 conditions are now satisfied based on changes to the cart break;
>             }
> 
> -
>             if (cart.getProductPromoUseCount(productPromoId) > maxUseLimit) {
>                 throw new UseLimitException("ERROR: While calculating promotions the promotion [" + productPromoId + "] action
>             was applied more than " + maxUseLimit + " times, so the calculation has been ended. This should generally never
> happen unless you have bad rule definitions."); } @@ -812,6 +838,34 @@ public class ProductPromoWorker {
>         return cartChanged;
>     }
> 
> +    private static Map<ShoppingCartItem,BigDecimal> prepareProductUsageInfoMap(ShoppingCart cart) {
> +        Map<ShoppingCartItem,BigDecimal> usageInfoMap = new HashMap<ShoppingCartItem, BigDecimal>();
> +        List<ShoppingCartItem> lineOrderedByBasePriceList = cart.getLineListOrderedByBasePrice(false);
> +        for (ShoppingCartItem cartItem : lineOrderedByBasePriceList) {
> +            BigDecimal used = cartItem.getPromoQuantityUsed();
> +            if (used.compareTo(BigDecimal.ZERO) != 0) {
> +                usageInfoMap.put(cartItem, used);
> +            }
> +        }
> +        return usageInfoMap;
> +    }
> +
> +    private static Map<ShoppingCartItem,BigDecimal> prepareDeltaProductUsageInfoMap(Map<ShoppingCartItem,BigDecimal> oldMap,
> Map<ShoppingCartItem,BigDecimal> newMap) { +        Map<ShoppingCartItem,BigDecimal> deltaUsageInfoMap = new
> HashMap<ShoppingCartItem, BigDecimal>(newMap); +        Iterator<ShoppingCartItem> cartLines = oldMap.keySet().iterator();
> +        while (cartLines.hasNext()) {
> +            ShoppingCartItem cartLine = cartLines.next();
> +            BigDecimal oldUsed = oldMap.get(cartLine);
> +            BigDecimal newUsed = newMap.get(cartLine);
> +            if (newUsed.compareTo(oldUsed) > 0) {
> +                deltaUsageInfoMap.put(cartLine, newUsed.add(oldUsed.negate()));
> +            } else {
> +                deltaUsageInfoMap.remove(cartLine);
> +            }
> +        }
> +        return deltaUsageInfoMap;
> +    }
> +
>     protected static boolean checkCondition(GenericValue productPromoCond, ShoppingCart cart, Delegator delegator,
>         LocalDispatcher dispatcher, Timestamp nowTimestamp) throws GenericEntityException { String condValue =
>         productPromoCond.getString("condValue"); String otherValue = productPromoCond.getString("otherValue");
> @@ -1772,8 +1826,8 @@ public class ProductPromoWorker {
>             actionResultInfo.ranAction = false;
>         }
> 
> +        // in action, if doesn't have enough quantity to use the promo at all, remove candidate promo uses and increment
>         promoQuantityUsed; this should go for all actions, if any action runs we confirm if (actionResultInfo.ranAction) {
> -            // in action, if doesn't have enough quantity to use the promo at all, remove candidate promo uses and increment
>             promoQuantityUsed; this should go for all actions, if any action runs we confirm
>         cart.confirmPromoRuleUse(productPromoAction.getString("productPromoId"),
>             productPromoAction.getString("productPromoRuleId")); } else {
> cart.resetPromoRuleUse(productPromoAction.getString("productPromoId"), productPromoAction.getString("productPromoRuleId"));