Re: svn commit: r1345661 - /ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1345661 - /ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy

Adam Heath-2
On 06/03/2012 07:20 AM, [hidden email] wrote:

> Author: jleroux
> Date: Sun Jun  3 12:20:48 2012
> New Revision: 1345661
>
> URL: http://svn.apache.org/viewvc?rev=1345661&view=rev
> Log:
> Revert r1345660, I was caught by the cache I thought it was only that when another change I put before was also needed
>
> Modified:
>      ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>
> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy?rev=1345661&r1=1345660&r2=1345661&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy (original)
> +++ ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy Sun Jun  3 12:20:48 2012
> @@ -33,7 +33,6 @@ import org.ofbiz.product.store.*;
>   import org.ofbiz.order.shoppingcart.*;
>   import org.ofbiz.product.product.ProductWorker;
>   import java.text.NumberFormat;
> -import javolution.util.FastList;
>
>   //either optProduct, optProductId or productId must be specified
>   product = request.getAttribute("optProduct");
> @@ -134,7 +133,7 @@ if (product) {
>       boolean isAlternativePacking = ProductWorker.isAlternativePacking(delegator, product.productId, null);
>       mainProducts = [];
>       if(isAlternativePacking){
> -        productVirtualVariants = delegator.findByAnd("ProductAssoc", UtilMisc.toMap("productIdTo", product.productId , "productAssocTypeId", "ALTERNATIVE_PACKAGE"), true);
> +        productVirtualVariants = delegator.findByAnd("ProductAssoc", UtilMisc.toMap("productIdTo", product.productId , "productAssocTypeId", "ALTERNATIVE_PACKAGE"), null, true);

Is this a line that I missed?  Or is this a line you added recently, but
didn't use the new methods I've added?  In either case, there should
still be the old, deprecated methods that would allow it to continue to
work.

>           if(productVirtualVariants){
>               productVirtualVariants.each { virtualVariantKey ->
>                   mainProductMap = [:];
>
>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1345661 - /ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy

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

> On 06/03/2012 07:20 AM, [hidden email] wrote:
>> Author: jleroux
>> Date: Sun Jun  3 12:20:48 2012
>> New Revision: 1345661
>>
>> URL: http://svn.apache.org/viewvc?rev=1345661&view=rev
>> Log:
>> Revert r1345660, I was caught by the cache I thought it was only that when another change I put before was also needed
>>
>> Modified:
>>      ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>
>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy?rev=1345661&r1=1345660&r2=1345661&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy (original)
>> +++ ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy Sun Jun  3 12:20:48 2012
>> @@ -33,7 +33,6 @@ import org.ofbiz.product.store.*;
>>   import org.ofbiz.order.shoppingcart.*;
>>   import org.ofbiz.product.product.ProductWorker;
>>   import java.text.NumberFormat;
>> -import javolution.util.FastList;
>>
>>   //either optProduct, optProductId or productId must be specified
>>   product = request.getAttribute("optProduct");
>> @@ -134,7 +133,7 @@ if (product) {
>>       boolean isAlternativePacking = ProductWorker.isAlternativePacking(delegator, product.productId, null);
>>       mainProducts = [];
>>       if(isAlternativePacking){
>> -        productVirtualVariants = delegator.findByAnd("ProductAssoc", UtilMisc.toMap("productIdTo", product.productId ,
>> "productAssocTypeId", "ALTERNATIVE_PACKAGE"), true);
>> +        productVirtualVariants = delegator.findByAnd("ProductAssoc", UtilMisc.toMap("productIdTo", product.productId ,
>> "productAssocTypeId", "ALTERNATIVE_PACKAGE"), null, true);
>
> Is this a line that I missed?  Or is this a line you added recently, but didn't use the new methods I've added?  In either case,
> there should still be the old, deprecated methods that would allow it to continue to work.

See updated comment done just after the commit at http://svn.apache.org/viewvc?view=revision&revision=1345661

Actually the old, deprecated method with this signature seems to any longer exist. That's why I warned about the other
instances. So maybe a commplete fix would be to re-add the the old, deprecated method with this signature
This commit can stay as is

Jacques


>>           if(productVirtualVariants){
>>               productVirtualVariants.each { virtualVariantKey ->
>>                   mainProductMap = [:];
>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1345661 - /ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy

Adam Heath-2
On 06/03/2012 02:52 PM, Jacques Le Roux wrote:

> From: "Adam Heath" <[hidden email]>
>> On 06/03/2012 07:20 AM, [hidden email] wrote:
>>> Author: jleroux
>>> Date: Sun Jun 3 12:20:48 2012
>>> New Revision: 1345661
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1345661&view=rev
>>> Log:
>>> Revert r1345660, I was caught by the cache I thought it was only that
>>> when another change I put before was also needed
>>>
>>> Modified:
>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>
>>>
>>> Modified:
>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy?rev=1345661&r1=1345660&r2=1345661&view=diff
>>>
>>> ==============================================================================
>>>
>>> ---
>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>> (original)
>>> +++
>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>> Sun Jun 3 12:20:48 2012
>>> @@ -33,7 +33,6 @@ import org.ofbiz.product.store.*;
>>> import org.ofbiz.order.shoppingcart.*;
>>> import org.ofbiz.product.product.ProductWorker;
>>> import java.text.NumberFormat;
>>> -import javolution.util.FastList;
>>>
>>> //either optProduct, optProductId or productId must be specified
>>> product = request.getAttribute("optProduct");
>>> @@ -134,7 +133,7 @@ if (product) {
>>> boolean isAlternativePacking =
>>> ProductWorker.isAlternativePacking(delegator, product.productId, null);
>>> mainProducts = [];
>>> if(isAlternativePacking){
>>> - productVirtualVariants = delegator.findByAnd("ProductAssoc",
>>> UtilMisc.toMap("productIdTo", product.productId ,
>>> "productAssocTypeId", "ALTERNATIVE_PACKAGE"), true);
>>> + productVirtualVariants = delegator.findByAnd("ProductAssoc",
>>> UtilMisc.toMap("productIdTo", product.productId ,
>>> "productAssocTypeId", "ALTERNATIVE_PACKAGE"), null, true);
>>
>> Is this a line that I missed? Or is this a line you added recently,
>> but didn't use the new methods I've added? In either case,
>> there should still be the old, deprecated methods that would allow it
>> to continue to work.
>
> See updated comment done just after the commit at
> http://svn.apache.org/viewvc?view=revision&revision=1345661
>
> Actually the old, deprecated method with this signature seems to any
> longer exist. That's why I warned about the other
> instances. So maybe a commplete fix would be to re-add the the old,
> deprecated method with this signature
> This commit can stay as is

Huh?  There was never a findByAnd(String, Map, true).  Full stop.

The following methods existed before I started deprecating.

findByAnd(String, Map)
findByAnd(String, Map, List)
findByAnd(String, Object...)
findByAndCache(String, Map)
findByAndCache(String, Map, List)

I added the following method:

findByAnd(String, Map, List, boolean).

Your call, before and after my deprecation work, is just completely
broken.  The only method it could use is:

findByAnd(String, Object...)

Which means a key that is a map, and a value that is a boolean.  Which
is of course broken.
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1345661 - /ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy

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

> On 06/03/2012 02:52 PM, Jacques Le Roux wrote:
>> From: "Adam Heath" <[hidden email]>
>>> On 06/03/2012 07:20 AM, [hidden email] wrote:
>>>> Author: jleroux
>>>> Date: Sun Jun 3 12:20:48 2012
>>>> New Revision: 1345661
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1345661&view=rev
>>>> Log:
>>>> Revert r1345660, I was caught by the cache I thought it was only that
>>>> when another change I put before was also needed
>>>>
>>>> Modified:
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>>
>>>>
>>>> Modified:
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy?rev=1345661&r1=1345660&r2=1345661&view=diff
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>> (original)
>>>> +++
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>> Sun Jun 3 12:20:48 2012
>>>> @@ -33,7 +33,6 @@ import org.ofbiz.product.store.*;
>>>> import org.ofbiz.order.shoppingcart.*;
>>>> import org.ofbiz.product.product.ProductWorker;
>>>> import java.text.NumberFormat;
>>>> -import javolution.util.FastList;
>>>>
>>>> //either optProduct, optProductId or productId must be specified
>>>> product = request.getAttribute("optProduct");
>>>> @@ -134,7 +133,7 @@ if (product) {
>>>> boolean isAlternativePacking =
>>>> ProductWorker.isAlternativePacking(delegator, product.productId, null);
>>>> mainProducts = [];
>>>> if(isAlternativePacking){
>>>> - productVirtualVariants = delegator.findByAnd("ProductAssoc",
>>>> UtilMisc.toMap("productIdTo", product.productId ,
>>>> "productAssocTypeId", "ALTERNATIVE_PACKAGE"), true);
>>>> + productVirtualVariants = delegator.findByAnd("ProductAssoc",
>>>> UtilMisc.toMap("productIdTo", product.productId ,
>>>> "productAssocTypeId", "ALTERNATIVE_PACKAGE"), null, true);
>>>
>>> Is this a line that I missed? Or is this a line you added recently,
>>> but didn't use the new methods I've added? In either case,
>>> there should still be the old, deprecated methods that would allow it
>>> to continue to work.
>>
>> See updated comment done just after the commit at
>> http://svn.apache.org/viewvc?view=revision&revision=1345661
>>
>> Actually the old, deprecated method with this signature seems to any
>> longer exist. That's why I warned about the other
>> instances. So maybe a commplete fix would be to re-add the the old,
>> deprecated method with this signature
>> This commit can stay as is
>
> Huh?  There was never a findByAnd(String, Map, true).  Full stop.
>
> The following methods existed before I started deprecating.
>
> findByAnd(String, Map)
> findByAnd(String, Map, List)
> findByAnd(String, Object...)
> findByAndCache(String, Map)
> findByAndCache(String, Map, List)
>
> I added the following method:
>
> findByAnd(String, Map, List, boolean).
>
> Your call, before and after my deprecation work, is just completely broken.  The only method it could use is:
>
> findByAnd(String, Object...)
>
> Which means a key that is a map, and a value that is a boolean.  Which is of course broken.

-        productVirtualVariants = delegator.findByAndCache("ProductAssoc", UtilMisc.toMap("productIdTo", product.productId ,
"productAssocTypeId", "ALTERNATIVE_PACKAGE"));
+        productVirtualVariants = delegator.findByAnd("ProductAssoc", UtilMisc.toMap("productIdTo", product.productId ,
"productAssocTypeId", "ALTERNATIVE_PACKAGE"), true);

http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy?r1=1338409&r2=1338408&pathrev=1338409

There was a bug I fixed it. Above is your commit. You obviously forgot to put a ",null" for the orderBy list before the ",true"
I now run out of time, if you feel it should be fixed otherwise please do https://issues.apache.org/jira/browse/OFBIZ-4882

Jacques
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1345661 - /ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy

Adam Heath-2
On 06/03/2012 03:45 PM, Jacques Le Roux wrote:

> From: "Adam Heath" <[hidden email]>
>> On 06/03/2012 02:52 PM, Jacques Le Roux wrote:
>>> From: "Adam Heath" <[hidden email]>
>>>> On 06/03/2012 07:20 AM, [hidden email] wrote:
>>>>> Author: jleroux
>>>>> Date: Sun Jun 3 12:20:48 2012
>>>>> New Revision: 1345661
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1345661&view=rev
>>>>> Log:
>>>>> Revert r1345660, I was caught by the cache I thought it was only that
>>>>> when another change I put before was also needed
>>>>>
>>>>> Modified:
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>>>
>>>>>
>>>>>
>>>>> Modified:
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>>>
>>>>>
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy?rev=1345661&r1=1345660&r2=1345661&view=diff
>>>>>
>>>>>
>>>>> ==============================================================================
>>>>>
>>>>>
>>>>> ---
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>>>
>>>>> (original)
>>>>> +++
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>>>
>>>>> Sun Jun 3 12:20:48 2012
>>>>> @@ -33,7 +33,6 @@ import org.ofbiz.product.store.*;
>>>>> import org.ofbiz.order.shoppingcart.*;
>>>>> import org.ofbiz.product.product.ProductWorker;
>>>>> import java.text.NumberFormat;
>>>>> -import javolution.util.FastList;
>>>>>
>>>>> //either optProduct, optProductId or productId must be specified
>>>>> product = request.getAttribute("optProduct");
>>>>> @@ -134,7 +133,7 @@ if (product) {
>>>>> boolean isAlternativePacking =
>>>>> ProductWorker.isAlternativePacking(delegator, product.productId,
>>>>> null);
>>>>> mainProducts = [];
>>>>> if(isAlternativePacking){
>>>>> - productVirtualVariants = delegator.findByAnd("ProductAssoc",
>>>>> UtilMisc.toMap("productIdTo", product.productId ,
>>>>> "productAssocTypeId", "ALTERNATIVE_PACKAGE"), true);
>>>>> + productVirtualVariants = delegator.findByAnd("ProductAssoc",
>>>>> UtilMisc.toMap("productIdTo", product.productId ,
>>>>> "productAssocTypeId", "ALTERNATIVE_PACKAGE"), null, true);
>>>>
>>>> Is this a line that I missed? Or is this a line you added recently,
>>>> but didn't use the new methods I've added? In either case,
>>>> there should still be the old, deprecated methods that would allow it
>>>> to continue to work.
>>>
>>> See updated comment done just after the commit at
>>> http://svn.apache.org/viewvc?view=revision&revision=1345661
>>>
>>> Actually the old, deprecated method with this signature seems to any
>>> longer exist. That's why I warned about the other
>>> instances. So maybe a commplete fix would be to re-add the the old,
>>> deprecated method with this signature
>>> This commit can stay as is
>>
>> Huh? There was never a findByAnd(String, Map, true). Full stop.
>>
>> The following methods existed before I started deprecating.
>>
>> findByAnd(String, Map)
>> findByAnd(String, Map, List)
>> findByAnd(String, Object...)
>> findByAndCache(String, Map)
>> findByAndCache(String, Map, List)
>>
>> I added the following method:
>>
>> findByAnd(String, Map, List, boolean).
>>
>> Your call, before and after my deprecation work, is just completely
>> broken. The only method it could use is:
>>
>> findByAnd(String, Object...)
>>
>> Which means a key that is a map, and a value that is a boolean. Which
>> is of course broken.
>
> - productVirtualVariants = delegator.findByAndCache("ProductAssoc",
> UtilMisc.toMap("productIdTo", product.productId , "productAssocTypeId",
> "ALTERNATIVE_PACKAGE"));
> + productVirtualVariants = delegator.findByAnd("ProductAssoc",
> UtilMisc.toMap("productIdTo", product.productId , "productAssocTypeId",
> "ALTERNATIVE_PACKAGE"), true);
>
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy?r1=1338409&r2=1338408&pathrev=1338409
>
>
> There was a bug I fixed it. Above is your commit. You obviously forgot
> to put a ",null" for the orderBy list before the ",true"
> I now run out of time, if you feel it should be fixed otherwise please
> do https://issues.apache.org/jira/browse/OFBIZ-4882

I guess we are playing tennis, and I just missed the shot.

Sorry for the bug.  I did check over every diff I sent, but I do admit
to getting a little bug-eyed while doing so.

If this ends up being the only missed problem, then that is still pretty
good.

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1345661 - /ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy

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

> On 06/03/2012 03:45 PM, Jacques Le Roux wrote:
>> From: "Adam Heath" <[hidden email]>
>>> On 06/03/2012 02:52 PM, Jacques Le Roux wrote:
>>>> From: "Adam Heath" <[hidden email]>
>>>>> On 06/03/2012 07:20 AM, [hidden email] wrote:
>>>>>> Author: jleroux
>>>>>> Date: Sun Jun 3 12:20:48 2012
>>>>>> New Revision: 1345661
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1345661&view=rev
>>>>>> Log:
>>>>>> Revert r1345660, I was caught by the cache I thought it was only that
>>>>>> when another change I put before was also needed
>>>>>>
>>>>>> Modified:
>>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>>>>
>>>>>>
>>>>>>
>>>>>> Modified:
>>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>>>>
>>>>>>
>>>>>> URL:
>>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy?rev=1345661&r1=1345660&r2=1345661&view=diff
>>>>>>
>>>>>>
>>>>>> ==============================================================================
>>>>>>
>>>>>>
>>>>>> ---
>>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>>>>
>>>>>> (original)
>>>>>> +++
>>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy
>>>>>>
>>>>>> Sun Jun 3 12:20:48 2012
>>>>>> @@ -33,7 +33,6 @@ import org.ofbiz.product.store.*;
>>>>>> import org.ofbiz.order.shoppingcart.*;
>>>>>> import org.ofbiz.product.product.ProductWorker;
>>>>>> import java.text.NumberFormat;
>>>>>> -import javolution.util.FastList;
>>>>>>
>>>>>> //either optProduct, optProductId or productId must be specified
>>>>>> product = request.getAttribute("optProduct");
>>>>>> @@ -134,7 +133,7 @@ if (product) {
>>>>>> boolean isAlternativePacking =
>>>>>> ProductWorker.isAlternativePacking(delegator, product.productId,
>>>>>> null);
>>>>>> mainProducts = [];
>>>>>> if(isAlternativePacking){
>>>>>> - productVirtualVariants = delegator.findByAnd("ProductAssoc",
>>>>>> UtilMisc.toMap("productIdTo", product.productId ,
>>>>>> "productAssocTypeId", "ALTERNATIVE_PACKAGE"), true);
>>>>>> + productVirtualVariants = delegator.findByAnd("ProductAssoc",
>>>>>> UtilMisc.toMap("productIdTo", product.productId ,
>>>>>> "productAssocTypeId", "ALTERNATIVE_PACKAGE"), null, true);
>>>>>
>>>>> Is this a line that I missed? Or is this a line you added recently,
>>>>> but didn't use the new methods I've added? In either case,
>>>>> there should still be the old, deprecated methods that would allow it
>>>>> to continue to work.
>>>>
>>>> See updated comment done just after the commit at
>>>> http://svn.apache.org/viewvc?view=revision&revision=1345661
>>>>
>>>> Actually the old, deprecated method with this signature seems to any
>>>> longer exist. That's why I warned about the other
>>>> instances. So maybe a commplete fix would be to re-add the the old,
>>>> deprecated method with this signature
>>>> This commit can stay as is
>>>
>>> Huh? There was never a findByAnd(String, Map, true). Full stop.
>>>
>>> The following methods existed before I started deprecating.
>>>
>>> findByAnd(String, Map)
>>> findByAnd(String, Map, List)
>>> findByAnd(String, Object...)
>>> findByAndCache(String, Map)
>>> findByAndCache(String, Map, List)
>>>
>>> I added the following method:
>>>
>>> findByAnd(String, Map, List, boolean).
>>>
>>> Your call, before and after my deprecation work, is just completely
>>> broken. The only method it could use is:
>>>
>>> findByAnd(String, Object...)
>>>
>>> Which means a key that is a map, and a value that is a boolean. Which
>>> is of course broken.
>>
>> - productVirtualVariants = delegator.findByAndCache("ProductAssoc",
>> UtilMisc.toMap("productIdTo", product.productId , "productAssocTypeId",
>> "ALTERNATIVE_PACKAGE"));
>> + productVirtualVariants = delegator.findByAnd("ProductAssoc",
>> UtilMisc.toMap("productIdTo", product.productId , "productAssocTypeId",
>> "ALTERNATIVE_PACKAGE"), true);
>>
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy?r1=1338409&r2=1338408&pathrev=1338409
>>
>>
>> There was a bug I fixed it. Above is your commit. You obviously forgot
>> to put a ",null" for the orderBy list before the ",true"
>> I now run out of time, if you feel it should be fixed otherwise please
>> do https://issues.apache.org/jira/browse/OFBIZ-4882
>
> I guess we are playing tennis, and I just missed the shot.
>
> Sorry for the bug.  I did check over every diff I sent, but I do admit to getting a little bug-eyed while doing so.
>
> If this ends up being the only missed problem, then that is still pretty good.

As I said in my last commit comment (link above) there are maybe more like that. I did not check much further...

Jacques
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1345661 - /ofbiz/trunk/applications/order/webapp/ordermgr/WEB-INF/actions/entry/catalog/ProductSummary.groovy

Jacopo Cappellato-4
In reply to this post by Adam Heath-2

On Jun 3, 2012, at 10:50 PM, Adam Heath wrote:

> Sorry for the bug.  I did check over every diff I sent, but I do admit to getting a little bug-eyed while doing so.
>
> If this ends up being the only missed problem, then that is still pretty good.

Adam,

I recently fixed a similar error that I suspect you introduced while refactoring getRelated; please have a look at my commit r. 1345528
It may be easier for you to check if your commit actually introduced other similar.

Thanks,

Jacopo