Re: svn commit: r957160 - /ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl

[Scott Gray-2]

Check the createProduct service definition.  productTypeId is required and the out of the box code shouldn't have to deal with any other scenario, please revert this.

Thanks
Scott

HotWax Media
http://www.hotwaxmedia.com

On 23/06/2010, at 11:16 PM, [hidden email] wrote:

> Author: jleroux
> Date: Wed Jun 23 11:16:33 2010
> New Revision: 957160
>
> URL: http://svn.apache.org/viewvc?rev=957160&view=rev
> Log:
> Product.productTypeId may not exist in a custom application
>
> Modified:
>    ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>
> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl?rev=957160&r1=957159&r2=957160&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl (original)
> +++ ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl Wed Jun 23 11:16:33 2010
> @@ -156,7 +156,7 @@ under the License.
>                       <span style="color: red; font-size: 15px;">[${backOrdered?if_exists}&nbsp;${uiLabelMap.OrderBackOrdered}]</span>
>                     </#if>
>                     </#if>
> -                    <#if (availableToPromiseMap.get(cartLine.getProductId()) <= 0) && (shoppingCart.getOrderType() == 'SALES_ORDER') && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId != "MARKETING_PKG_AUTO" && product.productTypeId != "MARKETING_PKG_PICK">
> +                    <#if (availableToPromiseMap.get(cartLine.getProductId()) <= 0) && (shoppingCart.getOrderType() == 'SALES_ORDER') && product.productTypeId! != "DIGITAL_GOOD" && product.productTypeId! != "MARKETING_PKG_AUTO" && product.productTypeId! != "MARKETING_PKG_PICK">
>                       <span style="color: red;">[${cartLine.getQuantity()}&nbsp;${uiLabelMap.OrderBackOrdered}]</span>
>                     <#else>
>                       <#if (availableToPromiseMap.get(cartLine.getProductId()) < cartLine.getQuantity()) && (shoppingCart.getOrderType() == 'SALES_ORDER') && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId != "MARKETING_PKG_AUTO" && product.productTypeId != "MARKETING_PKG_PICK">
>
>

[Jacques Le Roux]

I think we should discuss this. Product.productTypeId is not part of Product prmary key.
You may create other services that may not rely on OOTB createProduct and then you get caught by this when using the UI (even an UI
based on OOTB UI)
Also consider that this is only checking if the value is there. As OOTB it's mandatory there is no problems: it can't hurt (I know
you don't like this sentence ;o).

When I say we should discuss this, I don't think about this peculiar case but at large: should we consider only OOTB cases when
enforcing NPE or such?

Jacques

Scott Gray wrote:

> Check the createProduct service definition.  productTypeId is required and the out of the box code shouldn't have to deal with
> any other scenario, please revert this.
>
> Thanks
> Scott
>
> HotWax Media
> http://www.hotwaxmedia.com
>
> On 23/06/2010, at 11:16 PM, [hidden email] wrote:
>
>> Author: jleroux
>> Date: Wed Jun 23 11:16:33 2010
>> New Revision: 957160
>>
>> URL: http://svn.apache.org/viewvc?rev=957160&view=rev
>> Log:
>> Product.productTypeId may not exist in a custom application
>>
>> Modified:
>>    ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>
>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl?rev=957160&r1=957159&r2=957160&view=diff
>> ============================================================================== ---
>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl (original) +++
>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl Wed Jun 23 11:16:33 2010 @@ -156,7 +156,7 @@ under
>>                       the License. <span style="color: red; font-size:
>>                     15px;">[${backOrdered?if_exists}&nbsp;${uiLabelMap.OrderBackOrdered}]</span> </#if>
>>                     </#if>
>> -                    <#if (availableToPromiseMap.get(cartLine.getProductId()) <= 0) && (shoppingCart.getOrderType() ==
>> 'SALES_ORDER') && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId != "MARKETING_PKG_AUTO" &&
>>                       product.productTypeId != "MARKETING_PKG_PICK"> +                    <#if
>>                     (availableToPromiseMap.get(cartLine.getProductId()) <= 0) && (shoppingCart.getOrderType() == 'SALES_ORDER')
>>                       && product.productTypeId! != "DIGITAL_GOOD" && product.productTypeId! != "MARKETING_PKG_AUTO" &&
>> product.productTypeId! != "MARKETING_PKG_PICK"> <span style="color:
>> red;">[${cartLine.getQuantity()}&nbsp;${uiLabelMap.OrderBackOrdered}]</span> <#else> <#if
>> (availableToPromiseMap.get(cartLine.getProductId()) < cartLine.getQuantity()) && (shoppingCart.getOrderType() == 'SALES_ORDER')
>> && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId != "MARKETING_PKG_AUTO" && product.productTypeId !=
>> "MARKETING_PKG_PICK">

[Scott Gray-2]

Of course it's not part of the primary key but that is irrelevant.

If you are creating Products outside of createProduct then chances are you are doing it wrong anyway but all you have to do is make sure some sort of default type gets set.

OFBiz is heavily dependent on the type pattern in general and I don't want to see people making a mess of that while they hack their way through custom projects.  Maybe next week you will want to create Parties without types or WorkEfforts, CommunicationEvents, Agreements, Content, it is not for OFBiz to try and accommodate every strange little thing you may decide to do.

And I believe it does actually hurt, we end up with inconsistent code and if people start to think that TypeIds are optional then it opens up room for more of this type of thing.  There was no bug in trunk or 10.04, the bug is in your custom code.

Regards
Scott

On 24/06/2010, at 1:47 AM, Jacques Le Roux wrote:

> I think we should discuss this. Product.productTypeId is not part of Product prmary key.
> You may create other services that may not rely on OOTB createProduct and then you get caught by this when using the UI (even an UI based on OOTB UI)
> Also consider that this is only checking if the value is there. As OOTB it's mandatory there is no problems: it can't hurt (I know you don't like this sentence ;o).
>
> When I say we should discuss this, I don't think about this peculiar case but at large: should we consider only OOTB cases when enforcing NPE or such?
>
> Jacques
>
> Scott Gray wrote:
>> Check the createProduct service definition.  productTypeId is required and the out of the box code shouldn't have to deal with
>> any other scenario, please revert this.
>>
>> Thanks
>> Scott
>>
>> HotWax Media
>> http://www.hotwaxmedia.com
>>
>> On 23/06/2010, at 11:16 PM, [hidden email] wrote:
>>
>>> Author: jleroux
>>> Date: Wed Jun 23 11:16:33 2010
>>> New Revision: 957160
>>>
>>> URL: http://svn.apache.org/viewvc?rev=957160&view=rev
>>> Log:
>>> Product.productTypeId may not exist in a custom application
>>>
>>> Modified:
>>>   ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>
>>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl?rev=957160&r1=957159&r2=957160&view=diff
>>> ============================================================================== ---
>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl (original) +++
>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl Wed Jun 23 11:16:33 2010 @@ -156,7 +156,7 @@ under
>>>                      the License. <span style="color: red; font-size:
>>>                    15px;">[${backOrdered?if_exists}&nbsp;${uiLabelMap.OrderBackOrdered}]</span> </#if>
>>>                    </#if>
>>> -                    <#if (availableToPromiseMap.get(cartLine.getProductId()) <= 0) && (shoppingCart.getOrderType() ==
>>> 'SALES_ORDER') && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId != "MARKETING_PKG_AUTO" &&
>>>                      product.productTypeId != "MARKETING_PKG_PICK"> +                    <#if
>>>                    (availableToPromiseMap.get(cartLine.getProductId()) <= 0) && (shoppingCart.getOrderType() == 'SALES_ORDER')
>>>                      && product.productTypeId! != "DIGITAL_GOOD" && product.productTypeId! != "MARKETING_PKG_AUTO" &&
>>> product.productTypeId! != "MARKETING_PKG_PICK"> <span style="color:
>>> red;">[${cartLine.getQuantity()}&nbsp;${uiLabelMap.OrderBackOrdered}]</span> <#else> <#if
>>> (availableToPromiseMap.get(cartLine.getProductId()) < cartLine.getQuantity()) && (shoppingCart.getOrderType() == 'SALES_ORDER')
>>> && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId != "MARKETING_PKG_AUTO" && product.productTypeId !=
>>> "MARKETING_PKG_PICK">
>
>

[Adam Heath-2]

Scott Gray wrote:
> Of course it's not part of the primary key but that is irrelevant.
>
> If you are creating Products outside of createProduct then chances are you are doing it wrong anyway but all you have to do is make sure some sort of default type gets set.
>
> OFBiz is heavily dependent on the type pattern in general and I don't want to see people making a mess of that while they hack their way through custom projects.  Maybe next week you will want to create Parties without types or WorkEfforts, CommunicationEvents, Agreements, Content, it is not for OFBiz to try and accommodate every strange little thing you may decide to do.
>
> And I believe it does actually hurt, we end up with inconsistent code and if people start to think that TypeIds are optional then it opens up room for more of this type of thing.  There was no bug in trunk or 10.04, the bug is in your custom code.

I agree with Scott here.  It's one thing to add a new FooType, work
with FooType in your own custom code, and make certain that ofbiz
proper either ignores the new FooType, or that it following the type
inheritance to do basic processing.

But do not allow for the type field to be null.

>
> Regards
> Scott
>
> On 24/06/2010, at 1:47 AM, Jacques Le Roux wrote:
>
>> I think we should discuss this. Product.productTypeId is not part of Product prmary key.
>> You may create other services that may not rely on OOTB createProduct and then you get caught by this when using the UI (even an UI based on OOTB UI)
>> Also consider that this is only checking if the value is there. As OOTB it's mandatory there is no problems: it can't hurt (I know you don't like this sentence ;o).
>>
>> When I say we should discuss this, I don't think about this peculiar case but at large: should we consider only OOTB cases when enforcing NPE or such?
>>
>> Jacques
>>
>> Scott Gray wrote:
>>> Check the createProduct service definition.  productTypeId is required and the out of the box code shouldn't have to deal with
>>> any other scenario, please revert this.
>>>
>>> Thanks
>>> Scott
>>>
>>> HotWax Media
>>> http://www.hotwaxmedia.com
>>>
>>> On 23/06/2010, at 11:16 PM, [hidden email] wrote:
>>>
>>>> Author: jleroux
>>>> Date: Wed Jun 23 11:16:33 2010
>>>> New Revision: 957160
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=957160&view=rev
>>>> Log:
>>>> Product.productTypeId may not exist in a custom application
>>>>
>>>> Modified:
>>>>   ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>>
>>>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl?rev=957160&r1=957159&r2=957160&view=diff
>>>> ============================================================================== ---
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl (original) +++
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl Wed Jun 23 11:16:33 2010 @@ -156,7 +156,7 @@ under
>>>>                      the License. <span style="color: red; font-size:
>>>>                    15px;">[${backOrdered?if_exists}&nbsp;${uiLabelMap.OrderBackOrdered}]</span> </#if>
>>>>                    </#if>
>>>> -                    <#if (availableToPromiseMap.get(cartLine.getProductId()) <= 0) && (shoppingCart.getOrderType() ==
>>>> 'SALES_ORDER') && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId != "MARKETING_PKG_AUTO" &&
>>>>                      product.productTypeId != "MARKETING_PKG_PICK"> +                    <#if
>>>>                    (availableToPromiseMap.get(cartLine.getProductId()) <= 0) && (shoppingCart.getOrderType() == 'SALES_ORDER')
>>>>                      && product.productTypeId! != "DIGITAL_GOOD" && product.productTypeId! != "MARKETING_PKG_AUTO" &&
>>>> product.productTypeId! != "MARKETING_PKG_PICK"> <span style="color:
>>>> red;">[${cartLine.getQuantity()}&nbsp;${uiLabelMap.OrderBackOrdered}]</span> <#else> <#if
>>>> (availableToPromiseMap.get(cartLine.getProductId()) < cartLine.getQuantity()) && (shoppingCart.getOrderType() == 'SALES_ORDER')
>>>> && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId != "MARKETING_PKG_AUTO" && product.productTypeId !=
>>>> "MARKETING_PKG_PICK">
>>
>

[David E. Jones-2]

After thinking about this a bit I agree with Scott too.

The general issue is that there is a lot of code that requires the type to be specified, which is basically a de-facto way of making it a "required" field. In other words, I don't know if it was ever formally decided (other than through the service def) that this field is required, but that's the way it's been used for a long time and there may be a lot of code that depends on that.

If we wanted to make it optional, a LOT of code would have to be tested and changed. It would be far easier to just make sure data imported or created in other ways always has the productTypeId (or most *TypeId fields in the system).

-David


On Jun 23, 2010, at 4:21 PM, Scott Gray wrote:

> Of course it's not part of the primary key but that is irrelevant.
>
> If you are creating Products outside of createProduct then chances are you are doing it wrong anyway but all you have to do is make sure some sort of default type gets set.
>
> OFBiz is heavily dependent on the type pattern in general and I don't want to see people making a mess of that while they hack their way through custom projects.  Maybe next week you will want to create Parties without types or WorkEfforts, CommunicationEvents, Agreements, Content, it is not for OFBiz to try and accommodate every strange little thing you may decide to do.
>
> And I believe it does actually hurt, we end up with inconsistent code and if people start to think that TypeIds are optional then it opens up room for more of this type of thing.  There was no bug in trunk or 10.04, the bug is in your custom code.
>
> Regards
> Scott
>
> On 24/06/2010, at 1:47 AM, Jacques Le Roux wrote:
>
>> I think we should discuss this. Product.productTypeId is not part of Product prmary key.
>> You may create other services that may not rely on OOTB createProduct and then you get caught by this when using the UI (even an UI based on OOTB UI)
>> Also consider that this is only checking if the value is there. As OOTB it's mandatory there is no problems: it can't hurt (I know you don't like this sentence ;o).
>>
>> When I say we should discuss this, I don't think about this peculiar case but at large: should we consider only OOTB cases when enforcing NPE or such?
>>
>> Jacques
>>
>> Scott Gray wrote:
>>> Check the createProduct service definition.  productTypeId is required and the out of the box code shouldn't have to deal with
>>> any other scenario, please revert this.
>>>
>>> Thanks
>>> Scott
>>>
>>> HotWax Media
>>> http://www.hotwaxmedia.com
>>>
>>> On 23/06/2010, at 11:16 PM, [hidden email] wrote:
>>>
>>>> Author: jleroux
>>>> Date: Wed Jun 23 11:16:33 2010
>>>> New Revision: 957160
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=957160&view=rev
>>>> Log:
>>>> Product.productTypeId may not exist in a custom application
>>>>
>>>> Modified:
>>>>  ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>>
>>>> Modified: ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl?rev=957160&r1=957159&r2=957160&view=diff
>>>> ============================================================================== ---
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl (original) +++
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl Wed Jun 23 11:16:33 2010 @@ -156,7 +156,7 @@ under
>>>>                     the License. <span style="color: red; font-size:
>>>>                   15px;">[${backOrdered?if_exists}&nbsp;${uiLabelMap.OrderBackOrdered}]</span> </#if>
>>>>                   </#if>
>>>> -                    <#if (availableToPromiseMap.get(cartLine.getProductId()) <= 0) && (shoppingCart.getOrderType() ==
>>>> 'SALES_ORDER') && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId != "MARKETING_PKG_AUTO" &&
>>>>                     product.productTypeId != "MARKETING_PKG_PICK"> +                    <#if
>>>>                   (availableToPromiseMap.get(cartLine.getProductId()) <= 0) && (shoppingCart.getOrderType() == 'SALES_ORDER')
>>>>                     && product.productTypeId! != "DIGITAL_GOOD" && product.productTypeId! != "MARKETING_PKG_AUTO" &&
>>>> product.productTypeId! != "MARKETING_PKG_PICK"> <span style="color:
>>>> red;">[${cartLine.getQuantity()}&nbsp;${uiLabelMap.OrderBackOrdered}]</span> <#else> <#if
>>>> (availableToPromiseMap.get(cartLine.getProductId()) < cartLine.getQuantity()) && (shoppingCart.getOrderType() == 'SALES_ORDER')
>>>> && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId != "MARKETING_PKG_AUTO" && product.productTypeId !=
>>>> "MARKETING_PKG_PICK">
>>
>>
>

[BJ Freeman]

when coding I always use the reference
https://demo-stable.ofbiz.apache.org/webtools/control/ViewRelations?entityName=Product
as you see there is a reference  ProductTypeAttr for productTypeId
this tells me I should have a default assigned to productTypeId
if I am importing for a constant source I can assing this like in my
samples for external data importing.
if I want the user to be able to do this I check the field in the import
to make sure there is a valid value.

I think this would make a good best practice.



Jacques Le Roux sent the following on 6/23/2010 6:47 AM:


=========================
BJ Freeman
http://bjfreeman.elance.com
Strategic Power Office with Supplier Automation  <http://www.businessesnetwork.com/automation/viewforum.php?f=52>
Specialtymarket.com  <http://www.specialtymarket.com/>

Systems Integrator-- Glad to Assist

Chat  Y! messenger: bjfr33man
Linkedin
<http://www.linkedin.com/profile?viewProfile=&key=1237480&locale=en_US&trk=tab_pro>

> I think we should discuss this. Product.productTypeId is not part of
> Product prmary key.
> You may create other services that may not rely on OOTB createProduct
> and then you get caught by this when using the UI (even an UI based on
> OOTB UI)
> Also consider that this is only checking if the value is there. As OOTB
> it's mandatory there is no problems: it can't hurt (I know you don't
> like this sentence ;o).
>
> When I say we should discuss this, I don't think about this peculiar
> case but at large: should we consider only OOTB cases when enforcing NPE
> or such?
>
> Jacques
>
> Scott Gray wrote:
>> Check the createProduct service definition. productTypeId is required
>> and the out of the box code shouldn't have to deal with
>> any other scenario, please revert this.
>>
>> Thanks
>> Scott
>>
>> HotWax Media
>> http://www.hotwaxmedia.com
>>
>> On 23/06/2010, at 11:16 PM, [hidden email] wrote:
>>
>>> Author: jleroux
>>> Date: Wed Jun 23 11:16:33 2010
>>> New Revision: 957160
>>>
>>> URL: http://svn.apache.org/viewvc?rev=957160&view=rev
>>> Log:
>>> Product.productTypeId may not exist in a custom application
>>>
>>> Modified:
>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>
>>>
>>> Modified:
>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl?rev=957160&r1=957159&r2=957160&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>> (original) +++
>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>> Wed Jun 23 11:16:33 2010 @@ -156,7 +156,7 @@ under
>>> the License. <span style="color: red; font-size:
>>> 15px;">[${backOrdered?if_exists}&nbsp;${uiLabelMap.OrderBackOrdered}]</span>
>>> </#if>
>>> </#if>
>>> - <#if (availableToPromiseMap.get(cartLine.getProductId()) <= 0) &&
>>> (shoppingCart.getOrderType() ==
>>> 'SALES_ORDER') && product.productTypeId != "DIGITAL_GOOD" &&
>>> product.productTypeId != "MARKETING_PKG_AUTO" &&
>>> product.productTypeId != "MARKETING_PKG_PICK"> + <#if
>>> (availableToPromiseMap.get(cartLine.getProductId()) <= 0) &&
>>> (shoppingCart.getOrderType() == 'SALES_ORDER')
>>> && product.productTypeId! != "DIGITAL_GOOD" && product.productTypeId!
>>> != "MARKETING_PKG_AUTO" &&
>>> product.productTypeId! != "MARKETING_PKG_PICK"> <span style="color:
>>> red;">[${cartLine.getQuantity()}&nbsp;${uiLabelMap.OrderBackOrdered}]</span>
>>> <#else> <#if
>>> (availableToPromiseMap.get(cartLine.getProductId()) <
>>> cartLine.getQuantity()) && (shoppingCart.getOrderType() ==
>>> 'SALES_ORDER')
>>> && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId
>>> != "MARKETING_PKG_AUTO" && product.productTypeId !=
>>> "MARKETING_PKG_PICK">
>
>
>

[Jacques Le Roux]

Thanks Guys,

You convinced me. I have reverted r957221 at r957430  for trunk, and r957224 at r957431 for 10.04

Jacques

From: "BJ Freeman" <[hidden email]>

> when coding I always use the reference
> https://demo-stable.ofbiz.apache.org/webtools/control/ViewRelations?entityName=Product
> as you see there is a reference  ProductTypeAttr for productTypeId
> this tells me I should have a default assigned to productTypeId
> if I am importing for a constant source I can assing this like in my samples for external data importing.
> if I want the user to be able to do this I check the field in the import to make sure there is a valid value.
>
> I think this would make a good best practice.
>
>
>
> Jacques Le Roux sent the following on 6/23/2010 6:47 AM:
>
>
> =========================
> BJ Freeman
> http://bjfreeman.elance.com
> Strategic Power Office with Supplier Automation  <http://www.businessesnetwork.com/automation/viewforum.php?f=52>
> Specialtymarket.com  <http://www.specialtymarket.com/>
>
> Systems Integrator-- Glad to Assist
>
> Chat  Y! messenger: bjfr33man
> Linkedin <http://www.linkedin.com/profile?viewProfile=&key=1237480&locale=en_US&trk=tab_pro>
>
>> I think we should discuss this. Product.productTypeId is not part of
>> Product prmary key.
>> You may create other services that may not rely on OOTB createProduct
>> and then you get caught by this when using the UI (even an UI based on
>> OOTB UI)
>> Also consider that this is only checking if the value is there. As OOTB
>> it's mandatory there is no problems: it can't hurt (I know you don't
>> like this sentence ;o).
>>
>> When I say we should discuss this, I don't think about this peculiar
>> case but at large: should we consider only OOTB cases when enforcing NPE
>> or such?
>>
>> Jacques
>>
>> Scott Gray wrote:
>>> Check the createProduct service definition. productTypeId is required
>>> and the out of the box code shouldn't have to deal with
>>> any other scenario, please revert this.
>>>
>>> Thanks
>>> Scott
>>>
>>> HotWax Media
>>> http://www.hotwaxmedia.com
>>>
>>> On 23/06/2010, at 11:16 PM, [hidden email] wrote:
>>>
>>>> Author: jleroux
>>>> Date: Wed Jun 23 11:16:33 2010
>>>> New Revision: 957160
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=957160&view=rev
>>>> Log:
>>>> Product.productTypeId may not exist in a custom application
>>>>
>>>> Modified:
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>>
>>>>
>>>> Modified:
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl?rev=957160&r1=957159&r2=957160&view=diff
>>>>
>>>> ==============================================================================
>>>> ---
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>> (original) +++
>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl
>>>> Wed Jun 23 11:16:33 2010 @@ -156,7 +156,7 @@ under
>>>> the License. <span style="color: red; font-size:
>>>> 15px;">[${backOrdered?if_exists}&nbsp;${uiLabelMap.OrderBackOrdered}]</span>
>>>> </#if>
>>>> </#if>
>>>> - <#if (availableToPromiseMap.get(cartLine.getProductId()) <= 0) &&
>>>> (shoppingCart.getOrderType() ==
>>>> 'SALES_ORDER') && product.productTypeId != "DIGITAL_GOOD" &&
>>>> product.productTypeId != "MARKETING_PKG_AUTO" &&
>>>> product.productTypeId != "MARKETING_PKG_PICK"> + <#if
>>>> (availableToPromiseMap.get(cartLine.getProductId()) <= 0) &&
>>>> (shoppingCart.getOrderType() == 'SALES_ORDER')
>>>> && product.productTypeId! != "DIGITAL_GOOD" && product.productTypeId!
>>>> != "MARKETING_PKG_AUTO" &&
>>>> product.productTypeId! != "MARKETING_PKG_PICK"> <span style="color:
>>>> red;">[${cartLine.getQuantity()}&nbsp;${uiLabelMap.OrderBackOrdered}]</span>
>>>> <#else> <#if
>>>> (availableToPromiseMap.get(cartLine.getProductId()) <
>>>> cartLine.getQuantity()) && (shoppingCart.getOrderType() ==
>>>> 'SALES_ORDER')
>>>> && product.productTypeId != "DIGITAL_GOOD" && product.productTypeId
>>>> != "MARKETING_PKG_AUTO" && product.productTypeId !=
>>>> "MARKETING_PKG_PICK">
>>
>>
>>
>

[Scott Gray-2]

Thanks Jacques

Regards
Scott

On 24/06/2010, at 6:30 PM, "Jacques Le Roux" <[hidden email]
 > wrote:

> Thanks Guys,
>
> You convinced me. I have reverted r957221 at r957430  for trunk, and  
> r957224 at r957431 for 10.04
>
> Jacques
>
> From: "BJ Freeman" <[hidden email]>
>> when coding I always use the reference
>> https://demo-stable.ofbiz.apache.org/webtools/control/ViewRelations?entityName=Product
>> as you see there is a reference  ProductTypeAttr for productTypeId
>> this tells me I should have a default assigned to productTypeId
>> if I am importing for a constant source I can assing this like in  
>> my samples for external data importing.
>> if I want the user to be able to do this I check the field in the  
>> import to make sure there is a valid value.
>>
>> I think this would make a good best practice.
>>
>>
>>
>> Jacques Le Roux sent the following on 6/23/2010 6:47 AM:
>>
>>
>> =========================
>> BJ Freeman
>> http://bjfreeman.elance.com
>> Strategic Power Office with Supplier Automation  <http://www.businessesnetwork.com/automation/viewforum.php?f=52 
>> >
>> Specialtymarket.com  <http://www.specialtymarket.com/>
>>
>> Systems Integrator-- Glad to Assist
>>
>> Chat  Y! messenger: bjfr33man
>> Linkedin <http://www.linkedin.com/profile?viewProfile=&key=1237480&locale=en_US&trk=tab_pro 
>> >
>>
>>> I think we should discuss this. Product.productTypeId is not part of
>>> Product prmary key.
>>> You may create other services that may not rely on OOTB  
>>> createProduct
>>> and then you get caught by this when using the UI (even an UI  
>>> based on
>>> OOTB UI)
>>> Also consider that this is only checking if the value is there. As  
>>> OOTB
>>> it's mandatory there is no problems: it can't hurt (I know you don't
>>> like this sentence ;o).
>>>
>>> When I say we should discuss this, I don't think about this peculiar
>>> case but at large: should we consider only OOTB cases when  
>>> enforcing NPE
>>> or such?
>>>
>>> Jacques
>>>
>>> Scott Gray wrote:
>>>> Check the createProduct service definition. productTypeId is  
>>>> required
>>>> and the out of the box code shouldn't have to deal with
>>>> any other scenario, please revert this.
>>>>
>>>> Thanks
>>>> Scott
>>>>
>>>> HotWax Media
>>>> http://www.hotwaxmedia.com
>>>>
>>>> On 23/06/2010, at 11:16 PM, [hidden email] wrote:
>>>>
>>>>> Author: jleroux
>>>>> Date: Wed Jun 23 11:16:33 2010
>>>>> New Revision: 957160
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=957160&view=rev
>>>>> Log:
>>>>> Product.productTypeId may not exist in a custom application
>>>>>
>>>>> Modified:
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/
>>>>> showcartitems.ftl
>>>>>
>>>>>
>>>>> Modified:
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/
>>>>> showcartitems.ftl
>>>>>
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/showcartitems.ftl?rev=957160&r1=957159&r2=957160&view=diff
>>>>>
>>>>> ===
>>>>> ===
>>>>> ===
>>>>> ===
>>>>> ==================================================================
>>>>> ---
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/
>>>>> showcartitems.ftl
>>>>> (original) +++
>>>>> ofbiz/trunk/applications/order/webapp/ordermgr/entry/cart/
>>>>> showcartitems.ftl
>>>>> Wed Jun 23 11:16:33 2010 @@ -156,7 +156,7 @@ under
>>>>> the License. <span style="color: red; font-size:
>>>>> 15px;">[${backOrdered?if_exists}&nbsp;$
>>>>> {uiLabelMap.OrderBackOrdered}]</span>
>>>>> </#if>
>>>>> </#if>
>>>>> - <#if (availableToPromiseMap.get(cartLine.getProductId()) <= 0)  
>>>>> &&
>>>>> (shoppingCart.getOrderType() ==
>>>>> 'SALES_ORDER') && product.productTypeId != "DIGITAL_GOOD" &&
>>>>> product.productTypeId != "MARKETING_PKG_AUTO" &&
>>>>> product.productTypeId != "MARKETING_PKG_PICK"> + <#if
>>>>> (availableToPromiseMap.get(cartLine.getProductId()) <= 0) &&
>>>>> (shoppingCart.getOrderType() == 'SALES_ORDER')
>>>>> && product.productTypeId! != "DIGITAL_GOOD" &&  
>>>>> product.productTypeId!
>>>>> != "MARKETING_PKG_AUTO" &&
>>>>> product.productTypeId! != "MARKETING_PKG_PICK"> <span  
>>>>> style="color:
>>>>> red;">[${cartLine.getQuantity()}&nbsp;$
>>>>> {uiLabelMap.OrderBackOrdered}]</span>
>>>>> <#else> <#if
>>>>> (availableToPromiseMap.get(cartLine.getProductId()) <
>>>>> cartLine.getQuantity()) && (shoppingCart.getOrderType() ==
>>>>> 'SALES_ORDER')
>>>>> && product.productTypeId != "DIGITAL_GOOD" &&  
>>>>> product.productTypeId
>>>>> != "MARKETING_PKG_AUTO" && product.productTypeId !=
>>>>> "MARKETING_PKG_PICK">
>>>
>>>
>>>
>
>