|
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 = [:]; > > |
|
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 = [:]; >> >> > |
|
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. |
|
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 |
|
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. |
|
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 |
|
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 |
| Free forum by Nabble | Edit this page |
