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;