David,
I think I originally put it in to traverse up the product variant/virtual tree because we have products which have many tiers. It also seemed logical. What problems is it causing you? Maybe there is a way for us to solve it by specifying something in the product rule explicitly? Si Author: jonesde Date: Sat Dec 23 16:25:53 2006 New Revision: 489958 URL: http://svn.apache.org/viewvc?view=rev&rev=489958 Log: Changed so price conditions must succeed on the product itself and not on the product OR the virtual product if the main product is a variant; I don't know why that was put in there in the first place, so just commenting out for a while to see if it causes any problems; considering the virtual product does cause problems in certain circumstances with false positives on conditions Modified: incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java Modified: incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java URL: http://svn.apache.org/viewvc/incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java?view=diff&rev=489958&r1=489957&r2=489958 ============================================================================== --- incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java (original) +++ incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java Sat Dec 23 16:25:53 2006 @@ -536,10 +536,13 @@ if (!checkPriceCondition(productPriceCond, productId, prodCatalogId, productStoreGroupId, webSiteId, partyId, new Double(quantity), listPriceDbl.doubleValue(), currencyUomId, delegator, nowTimestamp)) { // if there is a virtualProductId, try that given that this one has failed if (virtualProductId != null) { + /* DEJ20061223 I don't know why we were trying conditions with the virtualProductId as well; this breaks various things you might want to do with price rules, so unless a need comes up for this in the future, removing it for now... if (!checkPriceCondition(productPriceCond, virtualProductId, prodCatalogId, productStoreGroupId, webSiteId, partyId, new Double(quantity), listPriceDbl.doubleValue(), currencyUomId, delegator, nowTimestamp)) { allExceptQuantTrue = false; } // otherwise, okay, this one made it so carry on checking + */ + allExceptQuantTrue = false; } else { allExceptQuantTrue = false; } @@ -885,11 +888,15 @@ if (!checkPriceCondition(productPriceCond, productId, prodCatalogId, productStoreGroupId, webSiteId, partyId, quantity, listPrice, currencyUomId, delegator, nowTimestamp)) { // if there is a virtualProductId, try that given that this one has failed if (virtualProductId != null) { + /* DEJ20061223 I don't know why we were trying conditions with the virtualProductId as well; this breaks various things you might want to do with price rules, so unless a need comes up for this in the future, removing it for now... if (!checkPriceCondition(productPriceCond, virtualProductId, prodCatalogId, productStoreGroupId, webSiteId, partyId, quantity, listPrice, currencyUomId, delegator, nowTimestamp)) { allTrue = false; break; } // otherwise, okay, this one made it so carry on checking + */ + allTrue = false; + break; } else { allTrue = false; break; |
We might be able to change this to look or not at the virtual product for a variant for different condition types. Let's start by comparing notes on the types we think important and why... The biggest problem we found with this is that you might was a feature or category associated with a variant product that affects the price, and it is possible or in some cases required that the virtual product is associated with the category or feature for each variant, so that instead of the price rule being applied to just one variant among a set for a virtual product, the rule ends up applying to _all_ of the variants because the virtual product is associated with the category or feature. Could you describe the structure and requirement you were running into where this was needed? Chances are we can find something that works all around and makes some sort of sense... -David On Jan 5, 2007, at 12:04 PM, Si Chen wrote: > David, > > I think I originally put it in to traverse up the product variant/ > virtual tree because we have products which have many tiers. It > also seemed logical. What problems is it causing you? Maybe there > is a way for us to solve it by specifying something in the product > rule explicitly? > > Si > > From: [hidden email] > Date: December 23, 2006 5:25:54 PM MST > To: [hidden email] > Subject: svn commit: r489958 - /incubator/ofbiz/trunk/applications/ > product/src/org/ofbiz/product/price/PriceServices.java > Reply-To: [hidden email] > > > Author: jonesde > Date: Sat Dec 23 16:25:53 2006 > New Revision: 489958 > > URL: http://svn.apache.org/viewvc?view=rev&rev=489958 > Log: > Changed so price conditions must succeed on the product itself and > not on the product OR the virtual product if the main product is a > variant; I don't know why that was put in there in the first place, > so just commenting out for a while to see if it causes any > problems; considering the virtual product does cause problems in > certain circumstances with false positives on conditions > > Modified: > incubator/ofbiz/trunk/applications/product/src/org/ofbiz/ > product/price/PriceServices.java > > Modified: incubator/ofbiz/trunk/applications/product/src/org/ofbiz/ > product/price/PriceServices.java > URL: http://svn.apache.org/viewvc/incubator/ofbiz/trunk/ > applications/product/src/org/ofbiz/product/price/PriceServices.java? > view=diff&rev=489958&r1=489957&r2=489958 > ====================================================================== > ======== > --- incubator/ofbiz/trunk/applications/product/src/org/ofbiz/ > product/price/PriceServices.java (original) > +++ incubator/ofbiz/trunk/applications/product/src/org/ofbiz/ > product/price/PriceServices.java Sat Dec 23 16:25:53 2006 > @@ -536,10 +536,13 @@ > if (!checkPriceCondition > (productPriceCond, productId, prodCatalogId, productStoreGroupId, > webSiteId, partyId, new Double(quantity), listPriceDbl.doubleValue > (), currencyUomId, delegator, nowTimestamp)) { > // if there is a > virtualProductId, try that given that this one has failed > if (virtualProductId != null) { > + /* DEJ20061223 I don't > know why we were trying conditions with the virtualProductId as > well; this breaks various things you might want to do with price > rules, so unless a need comes up for this in the future, removing > it for now... > if (!checkPriceCondition > (productPriceCond, virtualProductId, prodCatalogId, > productStoreGroupId, webSiteId, partyId, new Double(quantity), > listPriceDbl.doubleValue(), currencyUomId, delegator, nowTimestamp)) { > allExceptQuantTrue = > false; > } > // otherwise, okay, this > one made it so carry on checking > + */ > + allExceptQuantTrue = false; > } else { > allExceptQuantTrue = false; > } > @@ -885,11 +888,15 @@ > if (!checkPriceCondition(productPriceCond, > productId, prodCatalogId, productStoreGroupId, webSiteId, partyId, > quantity, listPrice, currencyUomId, delegator, nowTimestamp)) { > // if there is a virtualProductId, try that > given that this one has failed > if (virtualProductId != null) { > + /* DEJ20061223 I don't know why we were > trying conditions with the virtualProductId as well; this breaks > various things you might want to do with price rules, so unless a > need comes up for this in the future, removing it for now... > if (!checkPriceCondition(productPriceCond, > virtualProductId, prodCatalogId, productStoreGroupId, webSiteId, > partyId, quantity, listPrice, currencyUomId, delegator, > nowTimestamp)) { > allTrue = false; > break; > } > // otherwise, okay, this one made it so > carry on checking > + */ > + allTrue = false; > + break; > } else { > allTrue = false; > break; > > > > smime.p7s (3K) Download Attachment |
David,
My situation is simply that I might have a product whose variants are virtual and have variants of their own. It's pretty nice the ofbiz model can handle this. In those cases, the middle variants wouldn't have prices of their own. The lower variants would have the same features plus some more than the middle variants. Is it possible to make it so that it keeps traversing up until it finds a product with a price and then stops there? That would work for me. If it doesn't work for you, can you explain your use case more? Sorry I don't completely understand your use case and would appreciate an example. An alternative scheme might be just to have a flag on the price rule condition "traverseUpVariants" or something. That'll work for me too but might be a bit brute force. David E Jones wrote: > > We might be able to change this to look or not at the virtual product > for a variant for different condition types. > > Let's start by comparing notes on the types we think important and why... > > The biggest problem we found with this is that you might was a feature > or category associated with a variant product that affects the price, > and it is possible or in some cases required that the virtual product > is associated with the category or feature for each variant, so that > instead of the price rule being applied to just one variant among a > set for a virtual product, the rule ends up applying to _all_ of the > variants because the virtual product is associated with the category > or feature. > > Could you describe the structure and requirement you were running into > where this was needed? Chances are we can find something that works > all around and makes some sort of sense... > > -David > > > On Jan 5, 2007, at 12:04 PM, Si Chen wrote: > >> David, >> >> I think I originally put it in to traverse up the product >> variant/virtual tree because we have products which have many tiers. >> It also seemed logical. What problems is it causing you? Maybe >> there is a way for us to solve it by specifying something in the >> product rule explicitly? >> >> Si >> >> From: [hidden email] >> Date: December 23, 2006 5:25:54 PM MST >> To: [hidden email] >> Subject: svn commit: r489958 - >> /incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java >> >> Reply-To: [hidden email] >> >> >> Author: jonesde >> Date: Sat Dec 23 16:25:53 2006 >> New Revision: 489958 >> >> URL: http://svn.apache.org/viewvc?view=rev&rev=489958 >> Log: >> Changed so price conditions must succeed on the product itself and >> not on the product OR the virtual product if the main product is a >> variant; I don't know why that was put in there in the first place, >> so just commenting out for a while to see if it causes any problems; >> considering the virtual product does cause problems in certain >> circumstances with false positives on conditions >> >> Modified: >> >> incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java >> >> >> Modified: >> incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java >> >> URL: >> http://svn.apache.org/viewvc/incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java?view=diff&rev=489958&r1=489957&r2=489958 >> >> ============================================================================== >> >> --- >> incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java >> (original) >> +++ >> incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java >> Sat Dec 23 16:25:53 2006 >> @@ -536,10 +536,13 @@ >> if >> (!checkPriceCondition(productPriceCond, productId, prodCatalogId, >> productStoreGroupId, webSiteId, partyId, new Double(quantity), >> listPriceDbl.doubleValue(), currencyUomId, delegator, nowTimestamp)) { >> // if there is a >> virtualProductId, try that given that this one has failed >> if (virtualProductId != null) { >> + /* DEJ20061223 I don't know >> why we were trying conditions with the virtualProductId as well; this >> breaks various things you might want to do with price rules, so >> unless a need comes up for this in the future, removing it for now... >> if >> (!checkPriceCondition(productPriceCond, virtualProductId, >> prodCatalogId, productStoreGroupId, webSiteId, partyId, new >> Double(quantity), listPriceDbl.doubleValue(), currencyUomId, >> delegator, nowTimestamp)) { >> allExceptQuantTrue = false; >> } >> // otherwise, okay, this one >> made it so carry on checking >> + */ >> + allExceptQuantTrue = false; >> } else { >> allExceptQuantTrue = false; >> } >> @@ -885,11 +888,15 @@ >> if (!checkPriceCondition(productPriceCond, >> productId, prodCatalogId, productStoreGroupId, webSiteId, partyId, >> quantity, listPrice, currencyUomId, delegator, nowTimestamp)) { >> // if there is a virtualProductId, try that >> given that this one has failed >> if (virtualProductId != null) { >> + /* DEJ20061223 I don't know why we were >> trying conditions with the virtualProductId as well; this breaks >> various things you might want to do with price rules, so unless a >> need comes up for this in the future, removing it for now... >> if (!checkPriceCondition(productPriceCond, >> virtualProductId, prodCatalogId, productStoreGroupId, webSiteId, >> partyId, quantity, listPrice, currencyUomId, delegator, nowTimestamp)) { >> allTrue = false; >> break; >> } >> // otherwise, okay, this one made it so >> carry on checking >> + */ >> + allTrue = false; >> + break; >> } else { >> allTrue = false; >> break; >> >> >> >> > |
Si, It sounds like you're talking about a different part of the price calculation code. This section that is now commented out is ONLY related to checking conditions using a virtual product when the condition fails for a variant. It has nothing to do with the code (a bit further up in the file) that looks for prices on virtual products when there is no price of a certain type on a variant. That could is certainly still there, and could be extended to support checking to see if the virtual product is also a variant and keep going up the chain... -David On Jan 5, 2007, at 2:16 PM, Si Chen wrote: > David, > > My situation is simply that I might have a product whose variants > are virtual and have variants of their own. It's pretty nice the > ofbiz model can handle this. In those cases, the middle variants > wouldn't have prices of their own. The lower variants would have > the same features plus some more than the middle variants. > > Is it possible to make it so that it keeps traversing up until it > finds a product with a price and then stops there? That would work > for me. If it doesn't work for you, can you explain your use case > more? Sorry I don't completely understand your use case and would > appreciate an example. > > An alternative scheme might be just to have a flag on the price > rule condition "traverseUpVariants" or something. That'll work for > me too but might be a bit brute force. > > David E Jones wrote: >> >> We might be able to change this to look or not at the virtual >> product for a variant for different condition types. >> >> Let's start by comparing notes on the types we think important and >> why... >> >> The biggest problem we found with this is that you might was a >> feature or category associated with a variant product that affects >> the price, and it is possible or in some cases required that the >> virtual product is associated with the category or feature for >> each variant, so that instead of the price rule being applied to >> just one variant among a set for a virtual product, the rule ends >> up applying to _all_ of the variants because the virtual product >> is associated with the category or feature. >> >> Could you describe the structure and requirement you were running >> into where this was needed? Chances are we can find something that >> works all around and makes some sort of sense... >> >> -David >> >> >> On Jan 5, 2007, at 12:04 PM, Si Chen wrote: >> >>> David, >>> >>> I think I originally put it in to traverse up the product variant/ >>> virtual tree because we have products which have many tiers. It >>> also seemed logical. What problems is it causing you? Maybe >>> there is a way for us to solve it by specifying something in the >>> product rule explicitly? >>> >>> Si >>> >>> From: [hidden email] >>> Date: December 23, 2006 5:25:54 PM MST >>> To: [hidden email] >>> Subject: svn commit: r489958 - /incubator/ofbiz/trunk/ >>> applications/product/src/org/ofbiz/product/price/PriceServices.java >>> Reply-To: [hidden email] >>> >>> >>> Author: jonesde >>> Date: Sat Dec 23 16:25:53 2006 >>> New Revision: 489958 >>> >>> URL: http://svn.apache.org/viewvc?view=rev&rev=489958 >>> Log: >>> Changed so price conditions must succeed on the product itself >>> and not on the product OR the virtual product if the main product >>> is a variant; I don't know why that was put in there in the first >>> place, so just commenting out for a while to see if it causes any >>> problems; considering the virtual product does cause problems in >>> certain circumstances with false positives on conditions >>> >>> Modified: >>> incubator/ofbiz/trunk/applications/product/src/org/ofbiz/ >>> product/price/PriceServices.java >>> >>> Modified: incubator/ofbiz/trunk/applications/product/src/org/ >>> ofbiz/product/price/PriceServices.java >>> URL: http://svn.apache.org/viewvc/incubator/ofbiz/trunk/ >>> applications/product/src/org/ofbiz/product/price/ >>> PriceServices.java?view=diff&rev=489958&r1=489957&r2=489958 >>> ==================================================================== >>> ========== >>> --- incubator/ofbiz/trunk/applications/product/src/org/ofbiz/ >>> product/price/PriceServices.java (original) >>> +++ incubator/ofbiz/trunk/applications/product/src/org/ofbiz/ >>> product/price/PriceServices.java Sat Dec 23 16:25:53 2006 >>> @@ -536,10 +536,13 @@ >>> if (!checkPriceCondition >>> (productPriceCond, productId, prodCatalogId, productStoreGroupId, >>> webSiteId, partyId, new Double(quantity), listPriceDbl.doubleValue >>> (), currencyUomId, delegator, nowTimestamp)) { >>> // if there is a >>> virtualProductId, try that given that this one has failed >>> if (virtualProductId != null) { >>> + /* DEJ20061223 I don't >>> know why we were trying conditions with the virtualProductId as >>> well; this breaks various things you might want to do with price >>> rules, so unless a need comes up for this in the future, removing >>> it for now... >>> if (!checkPriceCondition >>> (productPriceCond, virtualProductId, prodCatalogId, >>> productStoreGroupId, webSiteId, partyId, new Double(quantity), >>> listPriceDbl.doubleValue(), currencyUomId, delegator, >>> nowTimestamp)) { >>> allExceptQuantTrue = >>> false; >>> } >>> // otherwise, okay, this >>> one made it so carry on checking >>> + */ >>> + allExceptQuantTrue = false; >>> } else { >>> allExceptQuantTrue = false; >>> } >>> @@ -885,11 +888,15 @@ >>> if (!checkPriceCondition(productPriceCond, >>> productId, prodCatalogId, productStoreGroupId, webSiteId, >>> partyId, quantity, listPrice, currencyUomId, delegator, >>> nowTimestamp)) { >>> // if there is a virtualProductId, try that >>> given that this one has failed >>> if (virtualProductId != null) { >>> + /* DEJ20061223 I don't know why we were >>> trying conditions with the virtualProductId as well; this breaks >>> various things you might want to do with price rules, so unless a >>> need comes up for this in the future, removing it for now... >>> if (!checkPriceCondition >>> (productPriceCond, virtualProductId, prodCatalogId, >>> productStoreGroupId, webSiteId, partyId, quantity, listPrice, >>> currencyUomId, delegator, nowTimestamp)) { >>> allTrue = false; >>> break; >>> } >>> // otherwise, okay, this one made it so >>> carry on checking >>> + */ >>> + allTrue = false; >>> + break; >>> } else { >>> allTrue = false; >>> break; >>> >>> >>> >>> >> > smime.p7s (3K) Download Attachment |
Ok, thanks for taking a look at that. I guess we'll try it when we
upgrade and let you know if there are problems, but this sounds fine. Have a great weekend. David E Jones wrote: > > Si, > > It sounds like you're talking about a different part of the price > calculation code. This section that is now commented out is ONLY > related to checking conditions using a virtual product when the > condition fails for a variant. > > It has nothing to do with the code (a bit further up in the file) that > looks for prices on virtual products when there is no price of a > certain type on a variant. That could is certainly still there, and > could be extended to support checking to see if the virtual product is > also a variant and keep going up the chain... > > -David > > > On Jan 5, 2007, at 2:16 PM, Si Chen wrote: > >> David, >> >> My situation is simply that I might have a product whose variants are >> virtual and have variants of their own. It's pretty nice the ofbiz >> model can handle this. In those cases, the middle variants wouldn't >> have prices of their own. The lower variants would have the same >> features plus some more than the middle variants. >> >> Is it possible to make it so that it keeps traversing up until it >> finds a product with a price and then stops there? That would work >> for me. If it doesn't work for you, can you explain your use case >> more? Sorry I don't completely understand your use case and would >> appreciate an example. >> >> An alternative scheme might be just to have a flag on the price rule >> condition "traverseUpVariants" or something. That'll work for me too >> but might be a bit brute force. >> >> David E Jones wrote: >>> >>> We might be able to change this to look or not at the virtual >>> product for a variant for different condition types. >>> >>> Let's start by comparing notes on the types we think important and >>> why... >>> >>> The biggest problem we found with this is that you might was a >>> feature or category associated with a variant product that affects >>> the price, and it is possible or in some cases required that the >>> virtual product is associated with the category or feature for each >>> variant, so that instead of the price rule being applied to just one >>> variant among a set for a virtual product, the rule ends up applying >>> to _all_ of the variants because the virtual product is associated >>> with the category or feature. >>> >>> Could you describe the structure and requirement you were running >>> into where this was needed? Chances are we can find something that >>> works all around and makes some sort of sense... >>> >>> -David >>> >>> >>> On Jan 5, 2007, at 12:04 PM, Si Chen wrote: >>> >>>> David, >>>> >>>> I think I originally put it in to traverse up the product >>>> variant/virtual tree because we have products which have many >>>> tiers. It also seemed logical. What problems is it causing you? >>>> Maybe there is a way for us to solve it by specifying something in >>>> the product rule explicitly? >>>> >>>> Si >>>> >>>> From: [hidden email] >>>> Date: December 23, 2006 5:25:54 PM MST >>>> To: [hidden email] >>>> Subject: svn commit: r489958 - >>>> /incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java >>>> >>>> Reply-To: [hidden email] >>>> >>>> >>>> Author: jonesde >>>> Date: Sat Dec 23 16:25:53 2006 >>>> New Revision: 489958 >>>> >>>> URL: http://svn.apache.org/viewvc?view=rev&rev=489958 >>>> Log: >>>> Changed so price conditions must succeed on the product itself and >>>> not on the product OR the virtual product if the main product is a >>>> variant; I don't know why that was put in there in the first place, >>>> so just commenting out for a while to see if it causes any >>>> problems; considering the virtual product does cause problems in >>>> certain circumstances with false positives on conditions >>>> >>>> Modified: >>>> >>>> incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java >>>> >>>> >>>> Modified: >>>> incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java >>>> >>>> URL: >>>> http://svn.apache.org/viewvc/incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java?view=diff&rev=489958&r1=489957&r2=489958 >>>> >>>> ============================================================================== >>>> >>>> --- >>>> incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java >>>> (original) >>>> +++ >>>> incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java >>>> Sat Dec 23 16:25:53 2006 >>>> @@ -536,10 +536,13 @@ >>>> if >>>> (!checkPriceCondition(productPriceCond, productId, prodCatalogId, >>>> productStoreGroupId, webSiteId, partyId, new Double(quantity), >>>> listPriceDbl.doubleValue(), currencyUomId, delegator, nowTimestamp)) { >>>> // if there is a >>>> virtualProductId, try that given that this one has failed >>>> if (virtualProductId != null) { >>>> + /* DEJ20061223 I don't >>>> know why we were trying conditions with the virtualProductId as >>>> well; this breaks various things you might want to do with price >>>> rules, so unless a need comes up for this in the future, removing >>>> it for now... >>>> if >>>> (!checkPriceCondition(productPriceCond, virtualProductId, >>>> prodCatalogId, productStoreGroupId, webSiteId, partyId, new >>>> Double(quantity), listPriceDbl.doubleValue(), currencyUomId, >>>> delegator, nowTimestamp)) { >>>> allExceptQuantTrue = >>>> false; >>>> } >>>> // otherwise, okay, this >>>> one made it so carry on checking >>>> + */ >>>> + allExceptQuantTrue = false; >>>> } else { >>>> allExceptQuantTrue = false; >>>> } >>>> @@ -885,11 +888,15 @@ >>>> if (!checkPriceCondition(productPriceCond, >>>> productId, prodCatalogId, productStoreGroupId, webSiteId, partyId, >>>> quantity, listPrice, currencyUomId, delegator, nowTimestamp)) { >>>> // if there is a virtualProductId, try that >>>> given that this one has failed >>>> if (virtualProductId != null) { >>>> + /* DEJ20061223 I don't know why we were >>>> trying conditions with the virtualProductId as well; this breaks >>>> various things you might want to do with price rules, so unless a >>>> need comes up for this in the future, removing it for now... >>>> if (!checkPriceCondition(productPriceCond, >>>> virtualProductId, prodCatalogId, productStoreGroupId, webSiteId, >>>> partyId, quantity, listPrice, currencyUomId, delegator, >>>> nowTimestamp)) { >>>> allTrue = false; >>>> break; >>>> } >>>> // otherwise, okay, this one made it so >>>> carry on checking >>>> + */ >>>> + allTrue = false; >>>> + break; >>>> } else { >>>> allTrue = false; >>>> break; >>>> >>>> >>>> >>>> >>> >> > |
Free forum by Nabble | Edit this page |