Re: svn commit: r571591 - /ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java

[David E Jones]

Jacques,

I've commented on this a lot before, but this REALLY isn't a bug fix, at all. 
This is definitely in the improvement camp, and just an improvement in error 
messages and handling.

While maintaining the release branch is really great, just remember the rule 
for releases is: LESS IS MORE!

Every change to the release branch creates the possibility of introducing MORE 
bugs. So if it's not a bug fix, we don't want to do it.

Just so we're all on the same page: what we REALLY need for the release branch 
is use and testing. Just applying fixes from the trunk to the branch will never 
get it to a releasable state...

-David


[EMAIL PROTECTED] wrote:

Author: jleroux
Date: Fri Aug 31 14:22:44 2007
New Revision: 571591

URL: http://svn.apache.org/viewvc?rev=571591&view=rev
Log:
Applied fix from trunk for revision: 567520

Modified:
    
ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java

Modified: 
ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java
URL: 
http://svn.apache.org/viewvc/ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java?rev=571591&r1=571590&r2=571591&view=diff
==============================================================================
--- 
ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java
 (original)
+++ 
ofbiz/branches/release4.0/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java
 Fri Aug 31 14:22:44 2007
@@ -108,7 +108,7 @@
             }
         }
         if (this.orderHeader == null) {
-            throw new IllegalArgumentException("Order header is not valid");
+            throw new IllegalArgumentException("Order header passed in is not valid for orderId [" 
+ orderHeader.getString("orderId") + "]");
         }
     }

@@ -125,7 +125,12 @@

         try {
             this.orderHeader = delegator.findByPrimaryKey("OrderHeader", 
UtilMisc.toMap("orderId", orderId));
         } catch (GenericEntityException e) {
-            throw new IllegalArgumentException("Invalid orderId");
+            String errMsg = "Error finding order with ID [" + orderId + "]: " 
+ e.toString();
+            Debug.logError(e, errMsg, module);
+            throw new IllegalArgumentException(errMsg);
+        }
+        if (this.orderHeader == null) {
+            throw new IllegalArgumentException("Order not found with orderId [" + 
orderId + "]");
         }
     }

@@ -155,7 +160,7 @@

             GenericValue productStore = delegator.findByPrimaryKeyCache("ProductStore", 
UtilMisc.toMap("productStoreId", productStoreId));
             return productStore;
         } catch (GenericEntityException ex) {
-            Debug.logError("Failed to get product store for order header [" + 
orderHeader + "] due to exception "+ ex.getMessage(), module);
+            Debug.logError(ex, "Failed to get product store for order header [" + 
orderHeader + "] due to exception "+ ex.getMessage(), module);
             return null;
         }
     }