svn commit: r1802681 - in /ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config: ProductConfigWorker.java ProductConfigWrapper.java

[ashish-18]

Author: ashish
Date: Sat Jul 22 12:10:06 2017
New Revision: 1802681

URL: http://svn.apache.org/viewvc?rev=1802681&view=rev
Log:
Improved: Code Improvement on Product Config.(OFBIZ-9281)
Additional Details:
I noticed a couple of code improvements on product config current code. Here is the reference:

- In ProductConfigWorker.fillProductConfigWrapper() method, currently, we only check parameters map, while it is possible in a business environment that user can submit required fields values in request attributes as well. So here getCombinedMap method can be used to make it more efficient.

- While calling constructor of ConfigOption (ConfigOption(ConfigOption co)), some private members should also be copied, like componentOptions etc.

Thanks Suraj for the contribution

Modified:
    ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWorker.java
    ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWrapper.java

Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWorker.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWorker.java?rev=1802681&r1=1802680&r2=1802681&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWorker.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWorker.java Sat Jul 22 12:10:06 2017
@@ -89,8 +89,16 @@ public final class ProductConfigWorker {
 
     public static void fillProductConfigWrapper(ProductConfigWrapper configWrapper, HttpServletRequest request) {
         int numOfQuestions = configWrapper.getQuestions().size();
+        Map<String, Object> combinedMap = UtilHttp.getCombinedMap(request);
         for (int k = 0; k < numOfQuestions; k++) {
-            String[] opts = request.getParameterValues(Integer.toString(k));
+            String[] opts = new String[0];
+            Object o = combinedMap.get(Integer.toString(k));;
+            if (o instanceof String) {
+                opts = new String[]{(String)o};
+            } else if(o instanceof List) {
+                List list = (List)o;
+                opts = (String[])((String[])list.toArray(new String[list.size()]));
+            }
             if (opts == null) {
 
                 //  check for standard item comments
@@ -98,7 +106,7 @@ public final class ProductConfigWorker {
                 if (question.isStandard()) {
                     int i = 0;
                     while (i <= (question.getOptions().size() -1)) {
-                        String comments = request.getParameter("comments_" + k + "_" + i);
+                        String comments = (String) combinedMap.get("comments_" + k + "_" + i);
                         if (UtilValidate.isNotEmpty(comments)) {
                             try {
                                 configWrapper.setSelected(k, i, comments);
@@ -118,9 +126,9 @@ public final class ProductConfigWorker {
                     String comments = null;
                     ProductConfigWrapper.ConfigItem question = configWrapper.getQuestions().get(k);
                     if (question.isSingleChoice()) {
-                        comments = request.getParameter("comments_" + k + "_" + "0");
+                        comments = (String) combinedMap.get("comments_" + k + "_" + "0");
                     } else {
-                        comments = request.getParameter("comments_" + k + "_" + cnt);
+                        comments = (String) combinedMap.get("comments_" + k + "_" + cnt);
                     }
 
                     configWrapper.setSelected(k, cnt, comments);
@@ -134,7 +142,7 @@ public final class ProductConfigWorker {
                             GenericValue component = components.get(i);
                             if (option.isVirtualComponent(component)) {
                                 String productParamName = "add_product_id" + k + "_" + cnt + "_" + variantIndex;
-                                String selectedProductId = request.getParameter(productParamName);
+                                String selectedProductId = (String) combinedMap.get(productParamName);
                                 if (UtilValidate.isEmpty(selectedProductId)) {
                                     Debug.logWarning("ERROR: Request param [" + productParamName + "] not found!", module);
                                 } else {
@@ -264,7 +272,10 @@ public final class ProductConfigWorker {
                                             if (anOption.isVirtualComponent(aComponent)) {
                                                 Map<String, String> componentOptions = anOption.getComponentOptions();
                                                 String optionProductId = aComponent.getString("productId");
-                                                String optionProductOptionId = componentOptions.get(optionProductId);
+                                                String optionProductOptionId = null;
+                                                if(UtilValidate.isNotEmpty(componentOptions)) {
+                                                    optionProductOptionId = componentOptions.get(optionProductId);
+                                                }
                                                 String configOptionId = anOption.configOption.getString("configOptionId");
                                                 configItemId = ci.getConfigItemAssoc().getString("configItemId");
                                                 sequenceNum = ci.getConfigItemAssoc().getLong("sequenceNum");

Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWrapper.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWrapper.java?rev=1802681&r1=1802680&r2=1802681&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWrapper.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/config/ProductConfigWrapper.java Sat Jul 22 12:10:06 2017
@@ -603,6 +603,8 @@ public class ProductConfigWrapper implem
             for (GenericValue component: co.componentList) {
                 componentList.add(GenericValue.create(component));
             }
+            parentConfigItem = co.parentConfigItem;
+            componentOptions = co.componentOptions;
             optionListPrice = co.optionListPrice;
             optionPrice = co.optionPrice;
             available = co.available;