I stumbled upon this while working on https://issues.apache.org/jira/browse/OFBIZ-5338
Quoting Jacopo: >>> I don't like the assumption about the string "control": this is a >>> configurable value (in web.xml) and we should not have it >>> hardcoded in our code. I know there are already few examples of this bad >>> pattern and in fact we should work and fix them as I suggest to consider, review and hopefully commit https://issues.apache.org/jira/browse/OFBIZ-5312 Jacques Jacopo Cappellato wrote: > Hans, > > please provide more details about the error you are seeing, how to recreate > it and also how the applications are currently using > the "urlPrefix" env variable and me or someone else will help you to find a > good way to solve the problem. > > Thanks, > > Jacopo > > On Mar 8, 2012, at 4:28 AM, Hans Bakker wrote: > >> Jacopo, >> >> Ok I reverted this change and we try to meet your request. The name of the >> 'control' servlet is not available where this code is >> used. Now we have clean code but there is an error in the system (not sure >> what is better) >> >> Do you have a suggestion how we could change this patch so it meets your >> requirements? >> >> Regards, >> Hans >> >> On 02/29/2012 04:40 PM, Jacopo Cappellato wrote: >>> Hi Hans, >>> >>> I don't like the assumption about the string "control": this is a >>> configurable value (in web.xml) and we should not have it >>> hardcoded in our code. I know there are already few examples of this bad >>> pattern and in fact we should work and fix them as >>> well. >>> >>> Jacopo >>> >>> >>> On Feb 29, 2012, at 10:23 AM, [email protected] wrote: >>> >>>> Author: hansbak >>>> Date: Wed Feb 29 09:23:36 2012 >>>> New Revision: 1295029 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=1295029&view=rev >>>> Log: >>>> correct url generation for seo friendly url's >>>> >>>> Modified: >>>> >>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/category/OfbizCatalogAltUrlTransform.java >>>> ofbiz/trunk/specialpurpose/ecommerce/widget/EmailOrderScreens.xml >>>> >>>> Modified: >>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/category/OfbizCatalogAltUrlTransform.java >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/category/OfbizCatalogAltUrlTransform.java?rev=1295029&r1=1295028&r2=1295029&view=diff >>>> ============================================================================== >>>> --- >>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/category/OfbizCatalogAltUrlTransform.java >>>> (original) +++ >>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/category/OfbizCatalogAltUrlTransform.java >>>> Wed Feb 29 09:23:36 2012 @@ >>>> -102,6 +102,7 @@ public class >>>> OfbizCatalogAltUrlTransform String productCategoryId = getStringArg(args, >>>> "productCategoryId"); String productId = >>>> getStringArg(args, "productId"); >>>> String url = ""; >>>> + String CONTROL_MOUNT_POINT = "control"; >>>> >>>> Object prefix = env.getVariable("urlPrefix"); >>>> String viewSize = getStringArg(args, "viewSize"); >>>> @@ -127,14 +128,19 @@ public class OfbizCatalogAltUrlTransform >>>> Delegator delegator = >>>> FreeMarkerWorker.getWrappedObject("delegator", env); >>>> LocalDispatcher dispatcher = >>>> FreeMarkerWorker.getWrappedObject("dispatcher", env); >>>> Locale locale = (Locale) args.get("locale"); >>>> + String prefixUrl = prefix.toString(); >>>> + // remove control path from the prefix URL >>>> + if(prefixUrl.contains(CONTROL_MOUNT_POINT)){ >>>> + prefixUrl = >>>> prefixUrl.replaceAll("/"+CONTROL_MOUNT_POINT, ""); >>>> + } >>>> if (UtilValidate.isNotEmpty(productId)) { >>>> GenericValue product = >>>> delegator.findOne("Product", UtilMisc.toMap("productId", productId), >>>> false); >>>> ProductContentWrapper wrapper = new >>>> ProductContentWrapper(dispatcher, product, locale, >>>> "text/html"); - url = >>>> CatalogUrlFilter.makeProductUrl(delegator, wrapper, null, ((StringModel) >>>> prefix).getAsString(), previousCategoryId, productCategoryId, productId); >>>> + url = >>>> CatalogUrlFilter.makeProductUrl(delegator, >>>> wrapper, null, prefixUrl, previousCategoryId, >>>> productCategoryId, productId); } else { >>>> GenericValue productCategory = >>>> delegator.findOne("ProductCategory", >>>> UtilMisc.toMap("productCategoryId", productCategoryId), >>>> false); CategoryContentWrapper wrapper = new >>>> CategoryContentWrapper(dispatcher, productCategory, locale, "text/html"); >>>> - >>>> url = CatalogUrlFilter.makeCategoryUrl(delegator, wrapper, null, >>>> ((StringModel) prefix).getAsString(), previousCategoryId, >>>> productCategoryId, productId, viewSize, viewIndex, >>>> viewSort, searchString); + >>>> url = CatalogUrlFilter.makeCategoryUrl(delegator, >>>> wrapper, null, prefixUrl, previousCategoryId, >>>> productCategoryId, productId, viewSize, viewIndex, >>>> viewSort, searchString); } out.write(url.toString()); } >>>> else { >>>> >>>> Modified: ofbiz/trunk/specialpurpose/ecommerce/widget/EmailOrderScreens.xml >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/ecommerce/widget/EmailOrderScreens.xml?rev=1295029&r1=1295028&r2=1295029&view=diff >>>> ============================================================================== >>>> --- >>>> ofbiz/trunk/specialpurpose/ecommerce/widget/EmailOrderScreens.xml >>>> (original) +++ >>>> ofbiz/trunk/specialpurpose/ecommerce/widget/EmailOrderScreens.xml Wed Feb >>>> 29 09:23:36 2012 @@ -85,7 +85,7 @@ under the License. >>>> <property-map resource="PartyUiLabels" >>>> map-name="uiLabelMap" global="true"/> >>>> <property-map resource="CommonUiLabels" >>>> map-name="uiLabelMap" global="true"/> >>>> <set field="title" >>>> value="${uiLabelMap.PageTitleOrderConfirmationNotice}"/> >>>> -<set field="baseEcommerceSecureUrl" value="${baseSecureUrl}/ecommerce"/> >>>> +<set field="baseEcommerceSecureUrl" >>>> value="${baseSecureUrl}/ecommerce/control"/> >>>> <set field="allowAnonymousView" value="Y"/> <!-- this >>>> field will instruction OrderStatus.groovy to allow an >>>> anonymous order to be viewed by anybody, so the email >>>> confirmation screen will work --> <script >>>> >>>> location="component://ecommerce/webapp/ecommerce/WEB-INF/actions/order/OrderStatus.groovy"/> >>>> </actions> >>>> @@ -105,7 +105,7 @@ under the License. >>>> <property-map resource="PartyUiLabels" >>>> map-name="uiLabelMap" global="true"/> >>>> <property-map resource="OrderUiLabels" >>>> map-name="uiLabelMap" global="true"/> >>>> <set field="title" >>>> value="${uiLabelMap.PageTitleOrderCompleteNotice}"/> >>>> -<set field="baseEcommerceSecureUrl" >>>> value="${baseSecureUrl}/ecommerce/control/"/> >>>> +<set field="baseEcommerceSecureUrl" >>>> value="${baseSecureUrl}/ecommerce/control"/> >>>> <script >>>> location="component://ecommerce/webapp/ecommerce/WEB-INF/actions/order/OrderStatus.groovy"/> >>>> </actions> >>>> <widgets>
