Re: svn commit: r812159 - /ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderLookupServices.java

[Scott Gray-2]

Hi Jacques

That is not the correct fix, there is no need to add the  
BillingAccount entity to the view because the billingAccountId is  
present on the OrderHeader and can be searched from there.  Only the  
first chunk of the patch is actually needed to fix the problem.

BTW, I think it would be great if you could consider waiting a day  
before back porting fixes to 9.04, that way there is an opportunity  
for review and decreases the chances that new bugs will be introduced  
to the release.

Regards
Scott

On 8/09/2009, at 2:42 AM, [hidden email] wrote:

> Author: jleroux
> Date: Mon Sep  7 14:42:42 2009
> New Revision: 812159
>
> URL: http://svn.apache.org/viewvc?rev=812159&view=rev
> Log:
> A patch from Akash Jain "Find Order functionality thrown error when  
> find orders for billingAccountId" (https://issues.apache.org/jira/browse/OFBIZ-2843 
> ) - OFBIZ-2843
>
> Modified:
>    ofbiz/trunk/applications/order/src/org/ofbiz/order/order/
> OrderLookupServices.java
>
> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/
> OrderLookupServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderLookupServices.java?rev=812159&r1=812158&r2=812159&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/
> OrderLookupServices.java (original)
> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/
> OrderLookupServices.java Mon Sep  7 14:42:42 2009
> @@ -417,9 +417,8 @@
>         String cardNumber = (String) context.get("cardNumber");
>         String accountNumber = (String) context.get("accountNumber");
>
> -        if (billingAccountId != null || finAccountId != null ||  
> cardNumber != null || accountNumber != null) {
> +        if (finAccountId != null || cardNumber != null ||  
> accountNumber != null) {
>             dve.addMemberEntity("OP", "OrderPaymentPreference");
> -            dve.addAlias("OP", "billingAccountId");
>             dve.addAlias("OP", "finAccountId");
>             dve.addAlias("OP", "paymentMethodId");
>             dve.addViewLink("OH", "OP", Boolean.FALSE,  
> UtilMisc.toList(new ModelKeyMap("orderId", "orderId")));
> @@ -427,6 +426,9 @@
>
>         // search by billing account ID
>         if (UtilValidate.isNotEmpty(billingAccountId)) {
> +            dve.addMemberEntity("BL", "BillingAccount");
> +            dve.addAlias("BL", "billingAccountId");
> +            dve.addViewLink("OH", "BL", Boolean.FALSE,  
> UtilMisc.toList(new ModelKeyMap("billingAccountId",  
> "billingAccountId")));
>             paramList.add("billingAccountId=" + billingAccountId);
>             conditions.add(makeExpr("billingAccountId",  
> billingAccountId));
>         }
>
>

[Jacques Le Roux]

Hi Scott,

Right, actually I tested, it worked, I commited. It's not a big deal I will suppress the 2 part (which is not really a bug only bad
code) in trunk and R9.04
OK for a day before backporting, seems wise indeed...

Thanks

Jacques

From: "Scott Gray" <[hidden email]>

> Hi Jacques
>
> That is not the correct fix, there is no need to add the  BillingAccount entity to the view because the billingAccountId is
> present on the OrderHeader and can be searched from there.  Only the  first chunk of the patch is actually needed to fix the
> problem.
>
> BTW, I think it would be great if you could consider waiting a day  before back porting fixes to 9.04, that way there is an
> opportunity  for review and decreases the chances that new bugs will be introduced  to the release.
>
> Regards
> Scott
>
> On 8/09/2009, at 2:42 AM, [hidden email] wrote:
>
>> Author: jleroux
>> Date: Mon Sep  7 14:42:42 2009
>> New Revision: 812159
>>
>> URL: http://svn.apache.org/viewvc?rev=812159&view=rev
>> Log:
>> A patch from Akash Jain "Find Order functionality thrown error when  find orders for billingAccountId"
>> (https://issues.apache.org/jira/browse/OFBIZ-2843 ) - OFBIZ-2843
>>
>> Modified:
>>    ofbiz/trunk/applications/order/src/org/ofbiz/order/order/ OrderLookupServices.java
>>
>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/ OrderLookupServices.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderLookupServices.java?rev=812159&r1=812158&r2=812159&view=diff
>> = = = = = = = = ======================================================================
>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/ OrderLookupServices.java (original)
>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/ OrderLookupServices.java Mon Sep  7 14:42:42 2009
>> @@ -417,9 +417,8 @@
>>         String cardNumber = (String) context.get("cardNumber");
>>         String accountNumber = (String) context.get("accountNumber");
>>
>> -        if (billingAccountId != null || finAccountId != null ||  cardNumber != null || accountNumber != null) {
>> +        if (finAccountId != null || cardNumber != null ||  accountNumber != null) {
>>             dve.addMemberEntity("OP", "OrderPaymentPreference");
>> -            dve.addAlias("OP", "billingAccountId");
>>             dve.addAlias("OP", "finAccountId");
>>             dve.addAlias("OP", "paymentMethodId");
>>             dve.addViewLink("OH", "OP", Boolean.FALSE,  UtilMisc.toList(new ModelKeyMap("orderId", "orderId")));
>> @@ -427,6 +426,9 @@
>>
>>         // search by billing account ID
>>         if (UtilValidate.isNotEmpty(billingAccountId)) {
>> +            dve.addMemberEntity("BL", "BillingAccount");
>> +            dve.addAlias("BL", "billingAccountId");
>> +            dve.addViewLink("OH", "BL", Boolean.FALSE,  UtilMisc.toList(new ModelKeyMap("billingAccountId",
>> "billingAccountId")));
>>             paramList.add("billingAccountId=" + billingAccountId);
>>             conditions.add(makeExpr("billingAccountId",  billingAccountId));
>>         }
>>
>>
>
>

[Jacques Le Roux]

I meaned the 2d part, of course. Done at r812192 and r812197
For this one I did not wait for backporting as I was quite sure of it

Jacques

From: "Jacques Le Roux" <[hidden email]>

> Hi Scott,
>
> Right, actually I tested, it worked, I commited. It's not a big deal I will suppress the 2 part (which is not really a bug only
> bad code) in trunk and R9.04
> OK for a day before backporting, seems wise indeed...
>
> Thanks
>
> Jacques
>
> From: "Scott Gray" <[hidden email]>
>> Hi Jacques
>>
>> That is not the correct fix, there is no need to add the  BillingAccount entity to the view because the billingAccountId is
>> present on the OrderHeader and can be searched from there.  Only the  first chunk of the patch is actually needed to fix the
>> problem.
>>
>> BTW, I think it would be great if you could consider waiting a day  before back porting fixes to 9.04, that way there is an
>> opportunity  for review and decreases the chances that new bugs will be introduced  to the release.
>>
>> Regards
>> Scott
>>
>> On 8/09/2009, at 2:42 AM, [hidden email] wrote:
>>
>>> Author: jleroux
>>> Date: Mon Sep  7 14:42:42 2009
>>> New Revision: 812159
>>>
>>> URL: http://svn.apache.org/viewvc?rev=812159&view=rev
>>> Log:
>>> A patch from Akash Jain "Find Order functionality thrown error when  find orders for billingAccountId"
>>> (https://issues.apache.org/jira/browse/OFBIZ-2843 ) - OFBIZ-2843
>>>
>>> Modified:
>>>    ofbiz/trunk/applications/order/src/org/ofbiz/order/order/ OrderLookupServices.java
>>>
>>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/order/ OrderLookupServices.java
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderLookupServices.java?rev=812159&r1=812158&r2=812159&view=diff
>>> = = = = = = = = ======================================================================
>>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/order/ OrderLookupServices.java (original)
>>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/order/ OrderLookupServices.java Mon Sep  7 14:42:42 2009
>>> @@ -417,9 +417,8 @@
>>>         String cardNumber = (String) context.get("cardNumber");
>>>         String accountNumber = (String) context.get("accountNumber");
>>>
>>> -        if (billingAccountId != null || finAccountId != null ||  cardNumber != null || accountNumber != null) {
>>> +        if (finAccountId != null || cardNumber != null ||  accountNumber != null) {
>>>             dve.addMemberEntity("OP", "OrderPaymentPreference");
>>> -            dve.addAlias("OP", "billingAccountId");
>>>             dve.addAlias("OP", "finAccountId");
>>>             dve.addAlias("OP", "paymentMethodId");
>>>             dve.addViewLink("OH", "OP", Boolean.FALSE,  UtilMisc.toList(new ModelKeyMap("orderId", "orderId")));
>>> @@ -427,6 +426,9 @@
>>>
>>>         // search by billing account ID
>>>         if (UtilValidate.isNotEmpty(billingAccountId)) {
>>> +            dve.addMemberEntity("BL", "BillingAccount");
>>> +            dve.addAlias("BL", "billingAccountId");
>>> +            dve.addViewLink("OH", "BL", Boolean.FALSE,  UtilMisc.toList(new ModelKeyMap("billingAccountId",
>>> "billingAccountId")));
>>>             paramList.add("billingAccountId=" + billingAccountId);
>>>             conditions.add(makeExpr("billingAccountId",  billingAccountId));
>>>         }
>>>
>>>
>>
>>
>
>