You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Si Chen <si...@opensourcestrategies.com> on 2007/01/05 20:04:39 UTC
[Fwd: svn commit: r489958 - /incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java]
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
Re: [Fwd: svn commit: r489958 - /incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java]
Posted by Si Chen <si...@opensourcestrategies.com>.
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: jonesde@apache.org
>>>> Date: December 23, 2006 5:25:54 PM MST
>>>> To: ofbiz-commits@incubator.apache.org
>>>> Subject: svn commit: r489958 -
>>>> /incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java
>>>>
>>>> Reply-To: ofbiz-dev@incubator.apache.org
>>>>
>>>>
>>>> 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;
>>>>
>>>>
>>>>
>>>>
>>>
>>
>
Re: [Fwd: svn commit: r489958 - /incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java]
Posted by David E Jones <jo...@undersunconsulting.com>.
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: jonesde@apache.org
>>> Date: December 23, 2006 5:25:54 PM MST
>>> To: ofbiz-commits@incubator.apache.org
>>> Subject: svn commit: r489958 - /incubator/ofbiz/trunk/
>>> applications/product/src/org/ofbiz/product/price/PriceServices.java
>>> Reply-To: ofbiz-dev@incubator.apache.org
>>>
>>>
>>> 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;
>>>
>>>
>>>
>>>
>>
>
Re: [Fwd: svn commit: r489958 - /incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java]
Posted by Si Chen <si...@opensourcestrategies.com>.
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: jonesde@apache.org
>> Date: December 23, 2006 5:25:54 PM MST
>> To: ofbiz-commits@incubator.apache.org
>> Subject: svn commit: r489958 -
>> /incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java
>>
>> Reply-To: ofbiz-dev@incubator.apache.org
>>
>>
>> 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;
>>
>>
>>
>>
>
Re: [Fwd: svn commit: r489958 - /incubator/ofbiz/trunk/applications/product/src/org/ofbiz/product/price/PriceServices.java]
Posted by David E Jones <jo...@undersunconsulting.com>.
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: jonesde@apache.org
> Date: December 23, 2006 5:25:54 PM MST
> To: ofbiz-commits@incubator.apache.org
> Subject: svn commit: r489958 - /incubator/ofbiz/trunk/applications/
> product/src/org/ofbiz/product/price/PriceServices.java
> Reply-To: ofbiz-dev@incubator.apache.org
>
>
> 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;
>
>
>
>