Re: svn commit: r931594 - /ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java

[Scott Gray-2]

This commit broke the demo system, no cost estimates are returned during checkout any more.

Regards
Scott

HotWax Media
http://www.hotwaxmedia.com

On 8/04/2010, at 3:47 AM, [hidden email] wrote:

> Author: doogie
> Date: Wed Apr  7 15:47:27 2010
> New Revision: 931594
>
> URL: http://svn.apache.org/viewvc?rev=931594&view=rev
> Log:
> If an exact productStoreShipMethId is requested, then do an exact AND
> match, not an OR.  Otherwise, both the original exact entity will be
> returned, *and* any others that happen to have the same
> carrierPartyId/shipmentMethodTypeId pair.  Then, when getFirst is
> called, it will effectively be random.  This bug shows up with showing
> cost estimates for a shipping method picker during checkout.
>
> Modified:
>    ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java
>
> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java?rev=931594&r1=931593&r2=931594&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java (original)
> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java Wed Apr  7 15:47:27 2010
> @@ -250,7 +250,7 @@ public class ShipmentServices {
>         if (UtilValidate.isNotEmpty(productStoreShipMethId)) {
>             // if the productStoreShipMethId field is passed, then also get estimates that have the field set
>             List<EntityCondition> condList = UtilMisc.toList(EntityCondition.makeCondition("productStoreShipMethId", EntityOperator.EQUALS, productStoreShipMethId), estFieldsCond);
> -            estFieldsCond = EntityCondition.makeCondition(condList, EntityOperator.OR);
> +            estFieldsCond = EntityCondition.makeCondition(condList, EntityOperator.AND);
>         }
>
>         Collection<GenericValue> estimates = null;
>
>

[Jacopo Cappellato-4]

I don't know if this is related to the commit or not (maybe not) but I have noticed that also the multi ship group features on the order entry backend are not working properly:
* when, in the first checkout screen you select "New Ship Group" a new ship group is correctly created but when you hit the "Continue" button the system creates a third one (!)
* if you assign the promotional items to the second ship group and then you complete the checkout the promotional items are moved back to the first ship group

Jacopo

On Apr 17, 2010, at 7:03 AM, Scott Gray wrote:

> This commit broke the demo system, no cost estimates are returned during checkout any more.
>
> Regards
> Scott
>
> HotWax Media
> http://www.hotwaxmedia.com
>
> On 8/04/2010, at 3:47 AM, [hidden email] wrote:
>
>> Author: doogie
>> Date: Wed Apr  7 15:47:27 2010
>> New Revision: 931594
>>
>> URL: http://svn.apache.org/viewvc?rev=931594&view=rev
>> Log:
>> If an exact productStoreShipMethId is requested, then do an exact AND
>> match, not an OR.  Otherwise, both the original exact entity will be
>> returned, *and* any others that happen to have the same
>> carrierPartyId/shipmentMethodTypeId pair.  Then, when getFirst is
>> called, it will effectively be random.  This bug shows up with showing
>> cost estimates for a shipping method picker during checkout.
>>
>> Modified:
>>   ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java
>>
>> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java?rev=931594&r1=931593&r2=931594&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java (original)
>> +++ ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java Wed Apr  7 15:47:27 2010
>> @@ -250,7 +250,7 @@ public class ShipmentServices {
>>        if (UtilValidate.isNotEmpty(productStoreShipMethId)) {
>>            // if the productStoreShipMethId field is passed, then also get estimates that have the field set
>>            List<EntityCondition> condList = UtilMisc.toList(EntityCondition.makeCondition("productStoreShipMethId", EntityOperator.EQUALS, productStoreShipMethId), estFieldsCond);
>> -            estFieldsCond = EntityCondition.makeCondition(condList, EntityOperator.OR);
>> +            estFieldsCond = EntityCondition.makeCondition(condList, EntityOperator.AND);
>>        }
>>
>>        Collection<GenericValue> estimates = null;
>>
>>
>

[Ean Schuessler]

I'm fairly sure this fix is correct. We need to look at what the demo store is doing. Matching with an OR doesn't make sense to me. If you've specified an exact productStoreShipMethId, why should you receive anything other than the corresponding entity?

----- "Scott Gray" wrote:
> This commit broke the demo system, no cost estimates are returned during checkout any more.

[Scott Gray-2]

I didn't say the fix was incorrect, just that it broke something which previously worked.  The demo estimates have a null productStoreShipMethId so no results are returned.  Possibly the code should remove that condition if no results are returned and try again without it.

Regards
Scott

On 17/04/2010, at 6:27 PM, Ean Schuessler wrote:

> I'm fairly sure this fix is correct. We need to look at what the demo store is doing. Matching with an OR doesn't make sense to me. If you've specified an exact productStoreShipMethId, why should you receive anything other than the corresponding entity?
>
> ----- "Scott Gray" wrote:
>> This commit broke the demo system, no cost estimates are returned during checkout any more.
>

[rohit]

Hi cost calculation is still broken and no shipping costs are returned.

Another interesting thing that i noticed was that although shipping costs for all options are showing as '0' and the log also says that no shipping estimates are returned, if select any of the shipping options and move to the next step in the checkout process, the shopping cart total shows the correctly calculated shipping total. Also the order confirmation page shows the selected shipping options and the calculated shipping estimate.

Thus, i guess the code has got broken only in the checkoutopions page, where the shipping estimated are showing as '0'.

thanks,

rohit


2010-06-27 13:49:03,129 (TP-Processor8) [   ShipmentServices.java:266:WARN ] No shipping estimates found; the shipping amount returned is 0!

[Jacques Le Roux]

I think there are 2 solutions for this.
We could add productStoreShipMethId values to ShipmentCostEstimate demo data. This implies to also set productStoreShipMethId values
to ProductStoreShipmentMeth
Now think about it, there are no productStoreShipMethId values in demo data. While installing demo data, they are created on the fly
in the order of appearance for ProductStoreShipmentMeth because it's primary key, but not in ShipmentCostEstimate.
So they are missing and it's the problem at hand. Note that productStoreShipMethId is not added to ShipmentCostEstimate by
createShipmentEstimate. It's used to get information from ProductStoreShipmentMeth but not put in ShipmentCostEstimate yet.

1) A very simple solution could be to remove productStoreShipMethId from ShipmentCostEstimate.
2) But for me, the best solution is to add productStoreShipMethId values for both ProductStoreShipmentMeth and ShipmentCostEstimate
demo data and to add productStoreShipMethId to ShipmentCostEstimate when created by createShipmentEstimate

If nobody see a problem with that I will do 2)

Thanks

Jacques

From: "rohit" <[hidden email]>

> Hi cost calculation is still broken and no shipping costs are returned.
>
> Another interesting thing that i noticed was that although shipping costs
> for all options are showing as '0' and the log also says that no shipping
> estimates are returned, if select any of the shipping options and move to
> the next step in the checkout process, the shopping cart total shows the
> correctly calculated shipping total. Also the order confirmation page shows
> the selected shipping options and the calculated shipping estimate.
>
> Thus, i guess the code has got broken only in the checkoutopions page, where
> the shipping estimated are showing as '0'.
>
> thanks,
>
> rohit
>
>
> 2010-06-27 13:49:03,129 (TP-Processor8) [   ShipmentServices.java:266:WARN ]
> No shipping estimates found; the shipping amount returned is 0!
> --
> View this message in context:
> http://ofbiz.135035.n4.nabble.com/Re-svn-commit-r931594-ofbiz-trunk-applications-product-src-org-ofbiz-shipment-shipment-ShipmentServia-tp2013854p2270198.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>

[Jacques Le Roux]

"Cost calculation is still broken and no shipping costs are returned." is fixed at revision: 985856  in trunk, r985860 in R10.04

I have also backported r931594+931595 in R9.04 at r985858+985862 and merged 985856  at r985872

And yes, I ran tests (hope you appreciate the effort ;o). But woooo... it's a long process with Derby with ant clean-all and
run-install before... I did not look into details though, I ran them, had a quick look at the log, that's all.

Note that nothing still is calculated for Fedex. I'm not quite sure if it's related, but Fedex has changed/is changing how things
work. The API is deprecated, replaced by web services and will be totally unusable in June 2012
http://fedex.com/us/developer/product/migration.html. I should work on this for a client and will contribute as much as possible...

Jacques

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

>I think there are 2 solutions for this.
> We could add productStoreShipMethId values to ShipmentCostEstimate demo data. This implies to also set productStoreShipMethId
> values
> to ProductStoreShipmentMeth
> Now think about it, there are no productStoreShipMethId values in demo data. While installing demo data, they are created on the
> fly
> in the order of appearance for ProductStoreShipmentMeth because it's primary key, but not in ShipmentCostEstimate.
> So they are missing and it's the problem at hand. Note that productStoreShipMethId is not added to ShipmentCostEstimate by
> createShipmentEstimate. It's used to get information from ProductStoreShipmentMeth but not put in ShipmentCostEstimate yet.
>
> 1) A very simple solution could be to remove productStoreShipMethId from ShipmentCostEstimate.
> 2) But for me, the best solution is to add productStoreShipMethId values for both ProductStoreShipmentMeth and
> ShipmentCostEstimate
> demo data and to add productStoreShipMethId to ShipmentCostEstimate when created by createShipmentEstimate
>
> If nobody see a problem with that I will do 2)
>
> Thanks
>
> Jacques
>
> From: "rohit" <[hidden email]>
>> Hi cost calculation is still broken and no shipping costs are returned.
>>
>> Another interesting thing that i noticed was that although shipping costs
>> for all options are showing as '0' and the log also says that no shipping
>> estimates are returned, if select any of the shipping options and move to
>> the next step in the checkout process, the shopping cart total shows the
>> correctly calculated shipping total. Also the order confirmation page shows
>> the selected shipping options and the calculated shipping estimate.
>>
>> Thus, i guess the code has got broken only in the checkoutopions page, where
>> the shipping estimated are showing as '0'.
>>
>> thanks,
>>
>> rohit
>>
>>
>> 2010-06-27 13:49:03,129 (TP-Processor8) [   ShipmentServices.java:266:WARN ]
>> No shipping estimates found; the shipping amount returned is 0!
>> --
>> View this message in context:
>> http://ofbiz.135035.n4.nabble.com/Re-svn-commit-r931594-ofbiz-trunk-applications-product-src-org-ofbiz-shipment-shipment-ShipmentServia-tp2013854p2270198.html
>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>>
>
>

[Jacques Le Roux]

I forgot to commit the change in ShipmentCostEstimate: add productStoreShipMethId to ShipmentCostEstimate when created by
createShipmentEstimate
It's done at r985902 and will be backported in R10.04 and R9.04

Jacques

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

> "Cost calculation is still broken and no shipping costs are returned." is fixed at revision: 985856  in trunk, r985860 in R10.04
>
> I have also backported r931594+931595 in R9.04 at r985858+985862 and merged 985856  at r985872
>
> And yes, I ran tests (hope you appreciate the effort ;o). But woooo... it's a long process with Derby with ant clean-all and
> run-install before... I did not look into details though, I ran them, had a quick look at the log, that's all.
>
> Note that nothing still is calculated for Fedex. I'm not quite sure if it's related, but Fedex has changed/is changing how things
> work. The API is deprecated, replaced by web services and will be totally unusable in June 2012
> http://fedex.com/us/developer/product/migration.html. I should work on this for a client and will contribute as much as
> possible...
>
> Jacques
>
> From: "Jacques Le Roux" <[hidden email]>
>>I think there are 2 solutions for this.
>> We could add productStoreShipMethId values to ShipmentCostEstimate demo data. This implies to also set productStoreShipMethId
>> values
>> to ProductStoreShipmentMeth
>> Now think about it, there are no productStoreShipMethId values in demo data. While installing demo data, they are created on the
>> fly
>> in the order of appearance for ProductStoreShipmentMeth because it's primary key, but not in ShipmentCostEstimate.
>> So they are missing and it's the problem at hand. Note that productStoreShipMethId is not added to ShipmentCostEstimate by
>> createShipmentEstimate. It's used to get information from ProductStoreShipmentMeth but not put in ShipmentCostEstimate yet.
>>
>> 1) A very simple solution could be to remove productStoreShipMethId from ShipmentCostEstimate.
>> 2) But for me, the best solution is to add productStoreShipMethId values for both ProductStoreShipmentMeth and
>> ShipmentCostEstimate
>> demo data and to add productStoreShipMethId to ShipmentCostEstimate when created by createShipmentEstimate
>>
>> If nobody see a problem with that I will do 2)
>>
>> Thanks
>>
>> Jacques
>>
>> From: "rohit" <[hidden email]>
>>> Hi cost calculation is still broken and no shipping costs are returned.
>>>
>>> Another interesting thing that i noticed was that although shipping costs
>>> for all options are showing as '0' and the log also says that no shipping
>>> estimates are returned, if select any of the shipping options and move to
>>> the next step in the checkout process, the shopping cart total shows the
>>> correctly calculated shipping total. Also the order confirmation page shows
>>> the selected shipping options and the calculated shipping estimate.
>>>
>>> Thus, i guess the code has got broken only in the checkoutopions page, where
>>> the shipping estimated are showing as '0'.
>>>
>>> thanks,
>>>
>>> rohit
>>>
>>>
>>> 2010-06-27 13:49:03,129 (TP-Processor8) [   ShipmentServices.java:266:WARN ]
>>> No shipping estimates found; the shipping amount returned is 0!
>>> --
>>> View this message in context:
>>> http://ofbiz.135035.n4.nabble.com/Re-svn-commit-r931594-ofbiz-trunk-applications-product-src-org-ofbiz-shipment-shipment-ShipmentServia-tp2013854p2270198.html
>>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>>>
>>
>>
>
>