[ 
https://issues.apache.org/jira/browse/OFBIZ-6988?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17415415#comment-17415415
 ] 

ASF subversion and git services commented on OFBIZ-6988:
--------------------------------------------------------

Commit fcd2c0cec8d78dad16cebe349b7d8b573894028a in ofbiz-framework's branch 
refs/heads/trunk from Nicolas Malin
[ https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=fcd2c0c ]

Improved: Estimated shipping cost resolution with breaks on price and quantity 
(OFBIZ-6988)

On the service calcShipmentCostEstimate, each estimated shipment cost is 
analysed to resolve which ones should be applied on the order.
During the breakQtys block analysis, OFBiz checks if the estimate match the 
quantity with their breaks and validates if one is good.
If a rule contains multiple break type (ex: weight and price) this will be 
ignored or use as partial, only one break would be analysed.

To solve it we extend the analyse on the break type combinaison.

After a global code review, we reformat the function to down less code quantity 
for help the reader on:
 * use an Util function to resolve BigDecimal values
 * dedicate function to resolve the shipping address
 * dedicate function to validate a break type
 * move multiple loop to stream

 Thanks to Gil Portenseigne and Jacques Leroux for their look


> Estimated shipping cost resolution with breaks on price and quantity
> --------------------------------------------------------------------
>
>                 Key: OFBIZ-6988
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-6988
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: product
>    Affects Versions: Trunk
>            Reporter: Nicolas Malin
>            Assignee: Nicolas Malin
>            Priority: Minor
>              Labels: shipping
>         Attachments: OFBIZ-6988-1.patch, OFBIZ-6988.patch
>
>
> On the service *calcShipmentCostEstimate*, each estimated shipment cost is 
> analysed to resolve which ones should be applied on the order.
> During the breakQtys block analysis, OFBiz checks if the estimate match the 
> quantity  with their breaks and validates if one is good
> {code}
>                     if (qv != null) {
>                         useQty = true;
>                         BigDecimal min = BigDecimal.ONE.movePointLeft(4);
>                         BigDecimal max = BigDecimal.ONE.movePointLeft(4);
>                         try {
>                             min = qv.getBigDecimal("fromQuantity");
>                             max = qv.getBigDecimal("thruQuantity");
>                         } catch (Exception e) {
>                         }
>                         if (shippableQuantity.compareTo(min) >= 0 && 
> (max.compareTo(BigDecimal.ZERO) == 0 || shippableQuantity.compareTo(max) <= 
> 0)) {
>                             qtyValid = true;
>                         }
>                         if (Debug.infoOn()) Debug.logInfo(" # QUANTITY SHIP 
> min : " + min + ", max : " + max + ", value " + shippableQuantity + " 
> qtyValid " + qtyValid, module);
>                     }
>                     if (pv != null) {
>                         usePrice = true;
>                         BigDecimal min = BigDecimal.ONE.movePointLeft(4);
>                         BigDecimal max = BigDecimal.ONE.movePointLeft(4);
>                         try {
>                             min = pv.getBigDecimal("fromQuantity");
>                             max = pv.getBigDecimal("thruQuantity");
>                         } catch (Exception e) {
>                         }
>                         if (shippableTotal.compareTo(min) >= 0 && 
> (max.compareTo(BigDecimal.ZERO) == 0 || shippableTotal.compareTo(max) <= 0)) {
>                             priceValid = true;
>                         }
>                         if (Debug.infoOn()) Debug.logInfo(" # PRICE TOT SHIP 
> min : " + min + ", max : " + max + ", value " + shippableTotal+ " qtyValid " 
> + priceValid, module);
>                     }
>                     // Now check the tests.
>                     if ((useWeight && weightValid)  || (useQty && qtyValid) 
> || (usePrice && priceValid)) {
>                         estimateList.add(thisEstimate);
>                     }
> {code}
> I didn't understand why an estimated shippping cost that contains a no valid 
> break can be applied on the order.
> On a customer project I have these rules:
> ||    ||Quantity Break Id||   Price Break Id|| Flat Price ||  Order Price 
> Percent ||
> |FR000|       |       0 - 1000 [FRP4]         | 25    |0|
> |FR001|       0 - 500,000 [FRB01]|    1000 - 0 [FRP3]         |       |15|
> |FR004|       2,000,000 - 0 [FRB04]|  1000 - 0 [FRP3]         |       |2|
> |FR003|       1,000,001 - 1,999,999 [FRB03]|  1000 - 0 [FRP3]         |       
>  |5|
> |FR002|       500,001 - 1,000,000 [FRB02]|    1000 - 0 [FRP3]         |       
>   |8|
> The problem with the previous code is that for a total price more than 300€ 
> OFBiz gives me a random rule between FR00[1-4] and it's wrong because I have 
> also a break on total quantity shipped
> I propose to change the check like this
> {code}
> @@ -406,7 +410,9 @@
>                          }
>                      }
>                      // Now check the tests.
> -                    if ((useWeight && weightValid) || (useQty && qtyValid) 
> || (usePrice && priceValid)) {
> +                    if ((!useWeight || useWeight && weightValid)
> +                            && (!useQty || useQty && qtyValid)
> +                            && (!usePrice || usePrice && priceValid)) {
>                          estimateList.add(thisEstimate);
>                      }
>                  }
> {code}
> To ensure that if a break is defined on the estimated shipping cost, we 
> enable it only if all defined breaks are valid.
> Any suggestion ?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to