Re: svn commit: r1295029 - in /ofbiz/trunk: applications/product/src/org/ofbiz/product/category/OfbizCatalogAltUrlTransform.java specialpurpose/ecommerce/widget/EmailOrderScreens.xml

[Jacques Le Roux]

Maybe then sending back a word saying that it's a WIP would help ;o)

Jacques

From: "Hans Bakker" <mailingl...@antwebsystems.com>

Jacopo,

in general i honor all comments although it can take some time, also this one 
it is comming....

Regards,
Hans

On 03/05/2012 05:57 PM, Jacopo Cappellato wrote:

Hey Hans, (and all OFBiz committers in general)

I would like to stress the importance to address reviews on commits (especially if they come from committers, like me, because we have "veto" on commits) asap by improving the code or reverting it.

Actually the etiquette when a committer complains about a commit and motivates 
the concerns (as I did) should be:

1) immediately improve the code according to the committer's review

2) or immediately provide additional information if you think that the code is good and that, with the additional details, the reviewer will be satisfied; if the discussion takes longer then the commit should be reverted until the agreement is found

3) or immediately revert it

The fact that we use a "commit then review" approach doesn't mean that every committers has the power to commit what he wants ignoring the others... the opposite is actually true: every time we do a commit we should be ready to receive feedback and improve the code; only at that point the code will become "official".

I hope this is the last time I have to state this fundamental rule,

Jacopo

On Feb 29, 2012, at 10:40 AM, 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, hans...@apache.org 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>