svn commit: r572172 - in /ofbiz/trunk/framework/common: src/org/ofbiz/common/CommonEve
Locked
svn commit: r572172 - in /ofbiz/trunk/framework/common: src/org/ofbiz/common/CommonEve
 
|
Author: jleroux
Date: Sun Sep 2 14:46:01 2007 New Revision: 572172 URL: http://svn.apache.org/viewvc?rev=572172&view=rev Log: A patch from Adrian Crum for an issue from Bilgin Ibryam"Changing the language in party manager is broken" (https://issues.apache.org/jira/browse/OFBIZ-1223) Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonEvents.java ofbiz/trunk/framework/common/webcommon/includes/listLocales.ftl Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonEvents.java URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonEvents.java?rev=572172&r1=572171&r2=572172&view=diff ============================================================================== --- ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonEvents.java (original) +++ ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonEvents.java Sun Sep 2 14:46:01 2007 @@ -166,7 +166,7 @@ /** Simple event to set the users per-session locale setting */ public static String setSessionLocale(HttpServletRequest request, HttpServletResponse response) { - String localeString = request.getParameter("locale"); + String localeString = request.getParameter("newLocale"); if (UtilValidate.isNotEmpty(localeString)) { UtilHttp.setLocale(request, localeString); @@ -243,6 +243,7 @@ return "success"; } } + Modified: ofbiz/trunk/framework/common/webcommon/includes/listLocales.ftl URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/webcommon/includes/listLocales.ftl?rev=572172&r1=572171&r2=572172&view=diff ============================================================================== --- ofbiz/trunk/framework/common/webcommon/includes/listLocales.ftl (original) +++ ofbiz/trunk/framework/common/webcommon/includes/listLocales.ftl Sun Sep 2 14:46:01 2007 @@ -50,7 +50,7 @@ </#if> <tr <#if altRow>class="alternate-row"</#if>> <td lang="${langAttr}" dir="${langDir}"> - <a href="<@ofbizUrl>setSessionLocale</@ofbizUrl>?locale=${availableLocale.toString()}">${availableLocale.getDisplayName(availableLocale)}</a> + <a href="<@ofbizUrl>setSessionLocale</@ofbizUrl>?newLocale=${availableLocale.toString()}">${availableLocale.getDisplayName(availableLocale)}</a> </td> </tr> </#list> |
Re: svn commit: r572172 - in /ofbiz/trunk/framework/common: src/org/ofbiz/common/CommonEve
|
|
This patch breaks stuff. It shouldn't necessarily be reverted, but the change is incomplete so if the rest of the effort is not finished soon then yes, this patch should be reverted. The pattern to find this is fairly simple: change something lots of stuff might use and only change one place that uses it, and chances are you have introduced a bug. In this case checking it was easy. Just search the whole project for "setSessionLocale" and make sure every one that represents a use of it now uses "newLocale" instead of "locale". Doing this search I found this has NOT been done and there ARE resources referring to the old name. When reviewing patches like this there are various acceptable things to do: 1. reject the patch describing the problem (more or less as above), and as the contributor to fix it 2. do the review and fix problems and commit it The alternative of committing without testing and doing some sanity checking is not very helpful for the project in general. Yeah, there is such a thing as CTR (Commit-Then-Review), but that is for group review of concepts and to catch things missed, it is NOT meant to be a replacement for testing and impact review to see if it breaks other stuff. Thanks for everyone's effort on general contribution review and commit, and thanks to contributors for contributing. Let's all just keep in mind that our lives in general will be a lot easier if we "First Do No Harm" as described on the committers page: http://docs.ofbiz.org/display/OFBADMIN/OFBiz+Committers+Roles+and+Responsibilities That page may seem to have a lot of useless crap on it, but please keep in mind that everything I wrote there was in reaction to a real world problem that has come up! Thanks, -David [hidden email] wrote: > Author: jleroux > Date: Sun Sep 2 14:46:01 2007 > New Revision: 572172 > > URL: http://svn.apache.org/viewvc?rev=572172&view=rev > Log: > A patch from Adrian Crum for an issue from Bilgin Ibryam"Changing the language in party manager is broken" (https://issues.apache.org/jira/browse/OFBIZ-1223) > > Modified: > ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonEvents.java > ofbiz/trunk/framework/common/webcommon/includes/listLocales.ftl > > Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonEvents.java > URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonEvents.java?rev=572172&r1=572171&r2=572172&view=diff > ============================================================================== > --- ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonEvents.java (original) > +++ ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonEvents.java Sun Sep 2 14:46:01 2007 > @@ -166,7 +166,7 @@ > > /** Simple event to set the users per-session locale setting */ > public static String setSessionLocale(HttpServletRequest request, HttpServletResponse response) { > - String localeString = request.getParameter("locale"); > + String localeString = request.getParameter("newLocale"); > if (UtilValidate.isNotEmpty(localeString)) { > UtilHttp.setLocale(request, localeString); > > @@ -243,6 +243,7 @@ > return "success"; > } > } > + > > > > > Modified: ofbiz/trunk/framework/common/webcommon/includes/listLocales.ftl > URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/webcommon/includes/listLocales.ftl?rev=572172&r1=572171&r2=572172&view=diff > ============================================================================== > --- ofbiz/trunk/framework/common/webcommon/includes/listLocales.ftl (original) > +++ ofbiz/trunk/framework/common/webcommon/includes/listLocales.ftl Sun Sep 2 14:46:01 2007 > @@ -50,7 +50,7 @@ > </#if> > <tr <#if altRow>class="alternate-row"</#if>> > <td lang="${langAttr}" dir="${langDir}"> > - <a href="<@ofbizUrl>setSessionLocale</@ofbizUrl>?locale=${availableLocale.toString()}">${availableLocale.getDisplayName(availableLocale)}</a> > + <a href="<@ofbizUrl>setSessionLocale</@ofbizUrl>?newLocale=${availableLocale.toString()}">${availableLocale.getDisplayName(availableLocale)}</a> > </td> > </tr> > </#list> > > |
«
Return to OFBiz - Commits
|
1 view|%1 views
| Free forum by Nabble | Edit this page |