svn commit: r1342983 - in /ofbiz/branches/release10.04: ./ applications/order/webapp/ordermgr/WEB-INF/actions/order/FilterOrderList.groovy

Author: jleroux
Date: Sat May 26 22:19:26 2012
New Revision: 1342983

URL: http://svn.apache.org/viewvc?rev=1342983&view=rev
Log:
"Applied fix from trunk for revision: 1342980  "  (actually not the point 2 which was not there yet)
------------------------------------------------------------------------
r1342980 | jleroux | 2012-05-27 00:13:01 +0200 (dim., 27 mai 2012) | 26 lines

Closes
1) "Deleted row in FilterOrderList.groovy"  https://issues.apache.org/jira/browse/OFBIZ-3431
2) "getOrders() in OrderListState does not work" https://issues.apache.org/jira/browse/OFBIZ-4883

As I had some doubt about the origin of the issues and wanted to check if they were real and would not hide something else or have side-effect, I did a complete historical review.

1) Could have been hidden using groovy safe navigation
ie
facilityId = orderHeader.originFacilityId ?: productStore?.inventoryFacilityId;
instead of
facilityId = orderHeader.originFacilityId ?: productStore.inventoryFacilityId;

But anyway it was wrong from the start when I introduced the feature  http://svn.apache.org/viewvc/incubator/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/order/filterorderlist.bsh?view=markup&pathrev=465376
(related to https://issues.apache.org/jira/browse/OFBIZ-644)
The facilityId  was useless. I remember I extracted it from a client specific feature and certainly forgot  to remove this part (useless at least in OFBiz code) when I merged with Leon's code.

2) Was Erwan wrong C/P or typo at http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderListState.java?r1=1305309&r2=1305308&pathrev=1305309

Thanks to Chatree Srichart and Nils Pförtner for reports (actually I found OFBIZ-3431 by chance while working on the issue)

Still, David was against this feature when I introduced it. And I must say that almost 6 years later I tend to agree with him. I have since crossed the same kind of OO patterns written in OFBiz based application by good Java programmers. They breaks the "simple" ways used in OFBiz at the application level (of course not at the framework level where those patterns are useful) and often finish to be problems...

Quoting David:
<<In general I don't like the pattern used in this patch. It doesn't follow best practices or use the tools and patterns established in OFBiz to make all of our lives easier, during implementation but even more importantly during maintenance and customization. There are very good reasons we recommend using object oriented patterns written in Java on only very rare occasions. This is a good example of something that is not complex enough to justify, and it will be a maintenance problem in the future.>>

Anyway this code is there now, and it's actually not as simple to replace...
------------------------------------------------------------------------

Modified:
    ofbiz/branches/release10.04/   (props changed)
    ofbiz/branches/release10.04/applications/order/webapp/ordermgr/WEB-INF/actions/order/FilterOrderList.groovy

Propchange: ofbiz/branches/release10.04/
------------------------------------------------------------------------------
  Merged /ofbiz/trunk:r1342980

Modified: ofbiz/branches/release10.04/applications/order/webapp/ordermgr/WEB-INF/actions/order/FilterOrderList.groovy
URL: http://svn.apache.org/viewvc/ofbiz/branches/release10.04/applications/order/webapp/ordermgr/WEB-INF/actions/order/FilterOrderList.groovy?rev=1342983&r1=1342982&r2=1342983&view=diff
==============================================================================
--- ofbiz/branches/release10.04/applications/order/webapp/ordermgr/WEB-INF/actions/order/FilterOrderList.groovy (original)
+++ ofbiz/branches/release10.04/applications/order/webapp/ordermgr/WEB-INF/actions/order/FilterOrderList.groovy Sat May 26 22:19:26 2012
@@ -51,7 +51,6 @@ if ((state.hasFilter("filterPartiallyRec
         state.hasFilter("filterPOsWithRejectedItems")) &&
         orderHeaderList) {
     orderHeaderList.each { orderHeader ->
-        facilityId = orderHeader.originFacilityId ?: productStore.inventoryFacilityId;
         orderReadHelper = OrderReadHelper.getHelper(orderHeader);
         if ("PURCHASE_ORDER".equals(orderHeader.orderTypeId)) {
             if (orderReadHelper.getRejectedOrderItems() &&