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

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;
         }
     }

[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


[hidden email] 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;
>          }
>      }
>
>