Re: svn commit: r921087 - /ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java

[Scott Gray-2]

On 9/03/2010, at 12:53 PM, [hidden email] wrote:

> Author: doogie
> Date: Tue Mar  9 19:53:25 2010
> New Revision: 921087
>
> URL: http://svn.apache.org/viewvc?rev=921087&view=rev
> Log:
> Add helper method for determining whether a ship group contains only
> digital goods.
>
> Modified:
>    ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java
>
> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java?rev=921087&r1=921086&r2=921087&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java (original)
> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java Tue Mar  9 19:53:25 2010
> @@ -818,6 +818,28 @@ public class ShoppingCart implements Ser
>         return true;
>     }
>
> +    /**
> +     * Check to see if the ship group contains only Digital Goods, ie no Finished Goods and no Finished/Digital Goods, et cetera.
> +     * This is determined by making sure no Product has a type where ProductType.isPhysical!=N.
> +     */
> +    public boolean containOnlyDigitalGoods(int shipGroupIdx) {
> +        CartShipInfo shipInfo = getShipInfo(shipGroupIdx);
> +        for (ShoppingCartItem cartItem: shipInfo.getShipItems()) {
> +            GenericValue product = cartItem.getProduct();
> +            try {
> +                GenericValue productType = product.getRelatedOneCache("ProductType");
> +                if (productType == null || !"N".equals(productType.getString("isPhysical"))) {
> +                    return false;
> +                }
> +            } catch (GenericEntityException e) {
> +                Debug.logError(e, "Error looking up ProductType: " + e.toString(), module);
> +                // consider this not a digital good if we don't have "proof"
> +                return false;
> +            }
> +        }
> +        return true;
> +    }
> +

It looks like we've ended up with some duplicated functionality around this at some point, shippingApplies() and containOnlyDigitalGoods() do essentially the same thing and we should probably deprecate one of them.   I'd prefer to keep the former because digital good doesn't do a good job of describing the purpose of the method e.g. shipping doesn't apply to a service item or non-product item yet they aren't digital goods.

Regards
Scott

[Adam Heath-2]

Scott Gray wrote:

> On 9/03/2010, at 12:53 PM, [hidden email] wrote:
>
>> Author: doogie
>> Date: Tue Mar  9 19:53:25 2010
>> New Revision: 921087
>>
>> URL: http://svn.apache.org/viewvc?rev=921087&view=rev
>> Log:
>> Add helper method for determining whether a ship group contains only
>> digital goods.
>>
>> Modified:
>>    ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java
>>
>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java?rev=921087&r1=921086&r2=921087&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java (original)
>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java Tue Mar  9 19:53:25 2010
>> @@ -818,6 +818,28 @@ public class ShoppingCart implements Ser
>>         return true;
>>     }
>>
>> +    /**
>> +     * Check to see if the ship group contains only Digital Goods, ie no Finished Goods and no Finished/Digital Goods, et cetera.
>> +     * This is determined by making sure no Product has a type where ProductType.isPhysical!=N.
>> +     */
>> +    public boolean containOnlyDigitalGoods(int shipGroupIdx) {
>> +        CartShipInfo shipInfo = getShipInfo(shipGroupIdx);
>> +        for (ShoppingCartItem cartItem: shipInfo.getShipItems()) {
>> +            GenericValue product = cartItem.getProduct();
>> +            try {
>> +                GenericValue productType = product.getRelatedOneCache("ProductType");
>> +                if (productType == null || !"N".equals(productType.getString("isPhysical"))) {
>> +                    return false;
>> +                }
>> +            } catch (GenericEntityException e) {
>> +                Debug.logError(e, "Error looking up ProductType: " + e.toString(), module);
>> +                // consider this not a digital good if we don't have "proof"
>> +                return false;
>> +            }
>> +        }
>> +        return true;
>> +    }
>> +
>
> It looks like we've ended up with some duplicated functionality around this at some point, shippingApplies() and containOnlyDigitalGoods() do essentially the same thing and we should probably deprecate one of them.   I'd prefer to keep the former because digital good doesn't do a good job of describing the purpose of the method e.g. shipping doesn't apply to a service item or non-product item yet they aren't digital goods.

I don't care which, and will happily use another method instead of
this new one I just added.

I use this method to skip PostalAddress queries/creation if a
shipgroup only contains digital goods(our frontend puts all digital
goods into a single shipgroup).

>
> Regards
> Scott
>