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/20 01:40:13 UTC

[Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Hey there -

This patch is a good idea, but I think Scott Gray suggested that this 
"-" could be configured in a properties file, and I think that's a good 
idea.  Otherwise, if you have four or five features you will easily 
overrun the 20-character productId key limit.  Keeping it in properties 
file is a good way to allow it to be modified.  Otherwise it's not very 
nice to have to go into the code to do it.

Jonathon, you up for doing this and sending in another patch?

Si

Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Jonathon -- Improov <jo...@improov.com>.
David,

Yeah, using config properties for this is a sort of hardcoding.

I'd say it's more user-friendly to be able to configure this sort of thing INSIDE OFBiz, not in 
config properties.

Jonathon

David E. Jones wrote:
> 
> Here are my previous comments about it earlier on the mailing list:
> 
> =========================================
> This isn't a universal policy or anything, but I'd say for something 
> minor like this there isn't a problem with hard-coding it.
> 
> The whole point of the ID generation is to make the IDs unique. In the 
> UI you can specify an ID instead of using the default, so it only 
> matters so much.
> =========================================
> 
> In general if this sort of thing were to be configurable, would we 
> really want it in a properties file? I'd say no, especially since one of 
> the new objectives that has been discussed recently is to make it easy 
> to configure things, make things configurable more granularly, and make 
> it easier to use different seed data files for OOTB industry-specific 
> configurations.
> 
> What to do...
> 
> -David
> 
> 
> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
> 
>> Hi
>>
>> I actually said that we should do it if anyone had objections, but no 
>> one did so that's why it went in as is.  But yeah if over-running the 
>> limit is a possibility then it should probably be changed.  Perhaps 
>> there should be some code in there anyway for coping with that 
>> situation, if someones running that many features saving 1 character 
>> might not be much of a bonus.
>>
>> Regards
>> Scott
>>
>> Jacopo Cappellato wrote:
>>> I agree with Si.
>>>
>>> Jacopo
>>>
>>> Si Chen wrote:
>>>> Hey there -
>>>>
>>>> This patch is a good idea, but I think Scott Gray suggested that 
>>>> this "-" could be configured in a properties file, and I think 
>>>> that's a good idea.  Otherwise, if you have four or five features 
>>>> you will easily overrun the 20-character productId key limit.  
>>>> Keeping it in properties file is a good way to allow it to be 
>>>> modified.  Otherwise it's not very nice to have to go into the code 
>>>> to do it.
>>>>
>>>> Jonathon, you up for doing this and sending in another patch?
>>>>
>>>> Si
>>>>
>>>> ------------------------------------------------------------------------ 
>>>>
>>>>
>>>> Subject:
>>>> svn commit: r495891 - 
>>>> /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>
>>>> From:
>>>> jleroux@apache.org
>>>> Date:
>>>> Sat, 13 Jan 2007 12:48:56 -0000
>>>> To:
>>>> commits@ofbiz.apache.org
>>>>
>>>> To:
>>>> commits@ofbiz.apache.org
>>>>
>>>>
>>>> Author: jleroux
>>>> Date: Sat Jan 13 04:48:55 2007
>>>> New Revision: 495891
>>>>
>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
>>>> Log:
>>>> A patch from Jonathon Wong "Prepend feature idCodes with '-'" 
>>>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>>
>>>> Modified:
>>>>     
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>
>>>>
>>>> Modified: 
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>
>>>> URL: 
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java?view=diff&rev=495891&r1=495890&r2=495891 
>>>>
>>>> ============================================================================== 
>>>>
>>>> --- 
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>> (original)
>>>> +++ 
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>> Sat Jan 13 04:48:55 2007
>>>> @@ -227,7 +227,7 @@
>>>>                                 List newFeatures = new LinkedList();
>>>>                                 List newFeatureIds = new LinkedList();
>>>>                                 if 
>>>> (currentFeature.getString("idCode") != null)
>>>> -                                
>>>> newCombination.put("defaultVariantProductId", productId + 
>>>> currentFeature.getString("idCode"));
>>>> +                                
>>>> newCombination.put("defaultVariantProductId", productId + "-" + 
>>>> currentFeature.getString("idCode"));
>>>>                              else
>>>>                                  
>>>> newCombination.put("defaultVariantProductId", productId);
>>>>                              newFeatures.add(currentFeature);
>>>>
>>>>
>>>
>>>
>>>
>>
> 


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Jonathon -- Improov <jo...@improov.com>.
Scott,

 > 2.  "as David's lack of management of SVN trunk" what is this supposed to
 > mean?  If I were David I would be offended.  Management of the trunk is not
 > a one man job, it is the responibility of the commiters and it is unfair to
 > try and blame anything like this on someone who has contributed so much.

Relax. Reread my post:

 >> says OFBiz folks (including yourself) are busy; I say I am busy ("almost
 >> getting fired" kind of busy).

Very often, to push the responsible parties to act, we take it first upon ourselves. Hopefully, 
the committers will start thinking "Was David around when I committed this, or was he at some 
conference I was too busy to attend myself?".

I heard from somewhere that David's recently been through a lot. Good that you're looking out for him.

Jonathon

Scott Gray wrote:
> Only 2 quick points here:
> 1.  Contributors should have no obligation to their contribution once it is
> in the trunk, if it has been passed by a committer then the code becomes 
> the
> responsibility of the community.  My bet is the "getVariantCombinations"
> service was an improvement on what was there previously (probably nothing)
> and we should be grateful that there is some functionality rather than 
> none.
> 
> 2.  "as David's lack of management of SVN trunk" what is this supposed to
> mean?  If I were David I would be offended.  Management of the trunk is not
> a one man job, it is the responibility of the commiters and it is unfair to
> try and blame anything like this on someone who has contributed so much.
> 
> Regards
> Scott
> 
> On 23/01/07, Jonathon -- Improov <jo...@improov.com> wrote:
>>
>> Si Chen,
>>
>> Question. Is there any way to know who did the QuickAddVariants flow in
>> the first place? Maybe the
>> original author should be responsible for correcting this MVC-related
>> problem, for refactoring the
>> "getVariantCombinations" service?
>>
>> The original service "getVariantCombinations" already "hardcodes" the
>> construction procedure of
>> the variants' product IDs.
>>
>> Yes, I agree my patch should not have added a new hardcode (the "-"
>> separator) into that service.
>>
>> However, if I were to avoid that "evil", I'd have to refactor the service
>> itself, plus the
>> QuickAddVariants screen, the whole works.
>>
>> Sigh. Whether you take it as my fault (selfish change to serve my needs)
>> or as David's lack of
>> management of SVN trunk, I have only 1 answer: time constraint. David 
>> says
>> OFBiz folks (including
>> yourself) are busy; I say I am busy ("almost getting fired" kind of 
>> busy).
>>
>> Only solution is to pour in more resources. I have no ideas regarding
>> where to find billionaires.
>>
>> I still disagree with David that this is a trivial change. My boss
>> requires the auto-generated
>> variants' product IDs for hundreds, no less, of variants per product. No
>> reason for me to think
>> nobody else needs the same functionality, but different separator.
>>
>> I guess this discussion is more a management issue than a coding issue?
>> For example, who audited
>> the commit of the service "getVariantCombinations"?
>>
>> This management issue does worry me (as does you, since your flagship
>> CRMSFA/Financials rely on
>> OFBiz heavily), so I'll risk taking a retort from David here.
>>
>> Jonathon
>>
>> Si Chen wrote:
>> > David E. Jones wrote:
>> >>
>> >> Here are my previous comments about it earlier on the mailing list:
>> >>
>> >> =========================================
>> >> This isn't a universal policy or anything, but I'd say for something
>> >> minor like this there isn't a problem with hard-coding it.
>> >>
>> >> The whole point of the ID generation is to make the IDs unique. In the
>> >> UI you can specify an ID instead of using the default, so it only
>> >> matters so much.
>> >> =========================================
>> >>
>> >> In general if this sort of thing were to be configurable, would we
>> >> really want it in a properties file? I'd say no, especially since one
>> >> of the new objectives that has been discussed recently is to make it
>> >> easy to configure things, make things configurable more granularly,
>> >> and make it easier to use different seed data files for OOTB
>> >> industry-specific configurations.
>> >>
>> >> What to do...
>> >>
>> >> -David
>> >>
>> >>
>> >> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
>> >>
>> >>> Hi
>> >>>
>> >>> I actually said that we should do it if anyone had objections, but no
>> >>> one did so that's why it went in as is.  But yeah if over-running the
>> >>> limit is a possibility then it should probably be changed.  Perhaps
>> >>> there should be some code in there anyway for coping with that
>> >>> situation, if someones running that many features saving 1 character
>> >>> might not be much of a bonus.
>> >>>
>> >>> Regards
>> >>> Scott
>> >>>
>> >>> Jacopo Cappellato wrote:
>> >>>> I agree with Si.
>> >>>>
>> >>>> Jacopo
>> >>>>
>> >>>> Si Chen wrote:
>> >>>>> Hey there -
>> >>>>>
>> >>>>> This patch is a good idea, but I think Scott Gray suggested that
>> >>>>> this "-" could be configured in a properties file, and I think
>> >>>>> that's a good idea.  Otherwise, if you have four or five features
>> >>>>> you will easily overrun the 20-character productId key limit.
>> >>>>> Keeping it in properties file is a good way to allow it to be
>> >>>>> modified.  Otherwise it's not very nice to have to go into the code
>> >>>>> to do it.
>> >>>>>
>> >>>>> Jonathon, you up for doing this and sending in another patch?
>> >>>>>
>> >>>>> Si
>> >>>>>
>> >>>>>
>> ------------------------------------------------------------------------
>> >>>>>
>> >>>>>
>> >>>>> Subject:
>> >>>>> svn commit: r495891 -
>> >>>>>
>> /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>
>> >>>>>
>> >>>>> From:
>> >>>>> jleroux@apache.org
>> >>>>> Date:
>> >>>>> Sat, 13 Jan 2007 12:48:56 -0000
>> >>>>> To:
>> >>>>> commits@ofbiz.apache.org
>> >>>>>
>> >>>>> To:
>> >>>>> commits@ofbiz.apache.org
>> >>>>>
>> >>>>>
>> >>>>> Author: jleroux
>> >>>>> Date: Sat Jan 13 04:48:55 2007
>> >>>>> New Revision: 495891
>> >>>>>
>> >>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
>> >>>>> Log:
>> >>>>> A patch from Jonathon Wong "Prepend feature idCodes with '-'"
>> >>>>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>> >>>>>
>> >>>>> Modified:
>> >>>>>
>> >>>>>
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>
>> >>>>>
>> >>>>>
>> >>>>> Modified:
>> >>>>>
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>
>> >>>>>
>> >>>>> URL:
>> >>>>>
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java?view=diff&rev=495891&r1=495890&r2=495891 
>>
>> >>>>>
>> >>>>>
>> ============================================================================== 
>>
>> >>>>>
>> >>>>> ---
>> >>>>>
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>
>> >>>>> (original)
>> >>>>> +++
>> >>>>>
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>
>> >>>>> Sat Jan 13 04:48:55 2007
>> >>>>> @@ -227,7 +227,7 @@
>> >>>>>                                 List newFeatures = new 
>> LinkedList();
>> >>>>>                                 List newFeatureIds = new
>> LinkedList();
>> >>>>>                                 if
>> >>>>> (currentFeature.getString("idCode") != null)
>> >>>>> -
>> >>>>> newCombination.put("defaultVariantProductId", productId +
>> >>>>> currentFeature.getString("idCode"));
>> >>>>> +
>> >>>>> newCombination.put("defaultVariantProductId", productId + "-" +
>> >>>>> currentFeature.getString("idCode"));
>> >>>>>                              else
>> >>>>>
>> >>>>> newCombination.put("defaultVariantProductId", productId);
>> >>>>>                              newFeatures.add(currentFeature);
>> >>>>>
>> >>>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>
>> >>
>> > David,
>> >
>> > Unfortunately I find this to be somewhat unsatisfactory for two 
>> reasons:
>> >
>> > 1.  Any potential problem when present should be addressed or at least
>> > considered.  This bit of code possibly adds a trivial enhancement in 
>> the
>> > eyes of some users but definitely potential for bugs in the case of
>> > other users.
>> >
>> > 2.  While it is true that somebody can just correct those on the data
>> > entry screen, the fact that this is at the service level rather than
>> > just on the screen in a .bsh file means that somebody else might be
>> > relying on this service to do things in a way that cannot be corrected
>> > on a screen.  To the extent the service layer offers a set of APIs in
>> > the form of services, such services should work without resorting to
>> > corrections in the view layer.  One of my favorite things about 
>> OFBIZ is
>> > the separation of the layers.
>> >
>> > Still, I agree with you--let's not have a long discussion about small
>> > things like this.  If I find problems with it down the road with this
>> > code I'll fix them myself in a way that's acceptable to all.
>> >
>> > Si
>> >
>> >
>>
>>
> 
> 
> ------------------------------------------------------------------------
> 
> No virus found in this incoming message.
> Checked by AVG Free Edition.
> Version: 7.5.432 / Virus Database: 268.17.5/645 - Release Date: 1/22/2007 4:10 PM


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Scott Gray <le...@gmail.com>.
Only 2 quick points here:
1.  Contributors should have no obligation to their contribution once it is
in the trunk, if it has been passed by a committer then the code becomes the
responsibility of the community.  My bet is the "getVariantCombinations"
service was an improvement on what was there previously (probably nothing)
and we should be grateful that there is some functionality rather than none.

2.  "as David's lack of management of SVN trunk" what is this supposed to
mean?  If I were David I would be offended.  Management of the trunk is not
a one man job, it is the responibility of the commiters and it is unfair to
try and blame anything like this on someone who has contributed so much.

Regards
Scott

On 23/01/07, Jonathon -- Improov <jo...@improov.com> wrote:
>
> Si Chen,
>
> Question. Is there any way to know who did the QuickAddVariants flow in
> the first place? Maybe the
> original author should be responsible for correcting this MVC-related
> problem, for refactoring the
> "getVariantCombinations" service?
>
> The original service "getVariantCombinations" already "hardcodes" the
> construction procedure of
> the variants' product IDs.
>
> Yes, I agree my patch should not have added a new hardcode (the "-"
> separator) into that service.
>
> However, if I were to avoid that "evil", I'd have to refactor the service
> itself, plus the
> QuickAddVariants screen, the whole works.
>
> Sigh. Whether you take it as my fault (selfish change to serve my needs)
> or as David's lack of
> management of SVN trunk, I have only 1 answer: time constraint. David says
> OFBiz folks (including
> yourself) are busy; I say I am busy ("almost getting fired" kind of busy).
>
> Only solution is to pour in more resources. I have no ideas regarding
> where to find billionaires.
>
> I still disagree with David that this is a trivial change. My boss
> requires the auto-generated
> variants' product IDs for hundreds, no less, of variants per product. No
> reason for me to think
> nobody else needs the same functionality, but different separator.
>
> I guess this discussion is more a management issue than a coding issue?
> For example, who audited
> the commit of the service "getVariantCombinations"?
>
> This management issue does worry me (as does you, since your flagship
> CRMSFA/Financials rely on
> OFBiz heavily), so I'll risk taking a retort from David here.
>
> Jonathon
>
> Si Chen wrote:
> > David E. Jones wrote:
> >>
> >> Here are my previous comments about it earlier on the mailing list:
> >>
> >> =========================================
> >> This isn't a universal policy or anything, but I'd say for something
> >> minor like this there isn't a problem with hard-coding it.
> >>
> >> The whole point of the ID generation is to make the IDs unique. In the
> >> UI you can specify an ID instead of using the default, so it only
> >> matters so much.
> >> =========================================
> >>
> >> In general if this sort of thing were to be configurable, would we
> >> really want it in a properties file? I'd say no, especially since one
> >> of the new objectives that has been discussed recently is to make it
> >> easy to configure things, make things configurable more granularly,
> >> and make it easier to use different seed data files for OOTB
> >> industry-specific configurations.
> >>
> >> What to do...
> >>
> >> -David
> >>
> >>
> >> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
> >>
> >>> Hi
> >>>
> >>> I actually said that we should do it if anyone had objections, but no
> >>> one did so that's why it went in as is.  But yeah if over-running the
> >>> limit is a possibility then it should probably be changed.  Perhaps
> >>> there should be some code in there anyway for coping with that
> >>> situation, if someones running that many features saving 1 character
> >>> might not be much of a bonus.
> >>>
> >>> Regards
> >>> Scott
> >>>
> >>> Jacopo Cappellato wrote:
> >>>> I agree with Si.
> >>>>
> >>>> Jacopo
> >>>>
> >>>> Si Chen wrote:
> >>>>> Hey there -
> >>>>>
> >>>>> This patch is a good idea, but I think Scott Gray suggested that
> >>>>> this "-" could be configured in a properties file, and I think
> >>>>> that's a good idea.  Otherwise, if you have four or five features
> >>>>> you will easily overrun the 20-character productId key limit.
> >>>>> Keeping it in properties file is a good way to allow it to be
> >>>>> modified.  Otherwise it's not very nice to have to go into the code
> >>>>> to do it.
> >>>>>
> >>>>> Jonathon, you up for doing this and sending in another patch?
> >>>>>
> >>>>> Si
> >>>>>
> >>>>>
> ------------------------------------------------------------------------
> >>>>>
> >>>>>
> >>>>> Subject:
> >>>>> svn commit: r495891 -
> >>>>>
> /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java
> >>>>>
> >>>>> From:
> >>>>> jleroux@apache.org
> >>>>> Date:
> >>>>> Sat, 13 Jan 2007 12:48:56 -0000
> >>>>> To:
> >>>>> commits@ofbiz.apache.org
> >>>>>
> >>>>> To:
> >>>>> commits@ofbiz.apache.org
> >>>>>
> >>>>>
> >>>>> Author: jleroux
> >>>>> Date: Sat Jan 13 04:48:55 2007
> >>>>> New Revision: 495891
> >>>>>
> >>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
> >>>>> Log:
> >>>>> A patch from Jonathon Wong "Prepend feature idCodes with '-'"
> >>>>> (https://issues.apache.org/jira/browse/OFBIZ-620)
> >>>>>
> >>>>> Modified:
> >>>>>
> >>>>>
> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java
> >>>>>
> >>>>>
> >>>>> Modified:
> >>>>>
> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java
> >>>>>
> >>>>> URL:
> >>>>>
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java?view=diff&rev=495891&r1=495890&r2=495891
> >>>>>
> >>>>>
> ==============================================================================
> >>>>>
> >>>>> ---
> >>>>>
> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java
> >>>>> (original)
> >>>>> +++
> >>>>>
> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java
> >>>>> Sat Jan 13 04:48:55 2007
> >>>>> @@ -227,7 +227,7 @@
> >>>>>                                 List newFeatures = new LinkedList();
> >>>>>                                 List newFeatureIds = new
> LinkedList();
> >>>>>                                 if
> >>>>> (currentFeature.getString("idCode") != null)
> >>>>> -
> >>>>> newCombination.put("defaultVariantProductId", productId +
> >>>>> currentFeature.getString("idCode"));
> >>>>> +
> >>>>> newCombination.put("defaultVariantProductId", productId + "-" +
> >>>>> currentFeature.getString("idCode"));
> >>>>>                              else
> >>>>>
> >>>>> newCombination.put("defaultVariantProductId", productId);
> >>>>>                              newFeatures.add(currentFeature);
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>
> > David,
> >
> > Unfortunately I find this to be somewhat unsatisfactory for two reasons:
> >
> > 1.  Any potential problem when present should be addressed or at least
> > considered.  This bit of code possibly adds a trivial enhancement in the
> > eyes of some users but definitely potential for bugs in the case of
> > other users.
> >
> > 2.  While it is true that somebody can just correct those on the data
> > entry screen, the fact that this is at the service level rather than
> > just on the screen in a .bsh file means that somebody else might be
> > relying on this service to do things in a way that cannot be corrected
> > on a screen.  To the extent the service layer offers a set of APIs in
> > the form of services, such services should work without resorting to
> > corrections in the view layer.  One of my favorite things about OFBIZ is
> > the separation of the layers.
> >
> > Still, I agree with you--let's not have a long discussion about small
> > things like this.  If I find problems with it down the road with this
> > code I'll fix them myself in a way that's acceptable to all.
> >
> > Si
> >
> >
>
>

Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Jonathon -- Improov <jo...@improov.com>.
Si Chen,

Question. Is there any way to know who did the QuickAddVariants flow in the first place? Maybe the 
original author should be responsible for correcting this MVC-related problem, for refactoring the 
"getVariantCombinations" service?

The original service "getVariantCombinations" already "hardcodes" the construction procedure of 
the variants' product IDs.

Yes, I agree my patch should not have added a new hardcode (the "-" separator) into that service.

However, if I were to avoid that "evil", I'd have to refactor the service itself, plus the 
QuickAddVariants screen, the whole works.

Sigh. Whether you take it as my fault (selfish change to serve my needs) or as David's lack of 
management of SVN trunk, I have only 1 answer: time constraint. David says OFBiz folks (including 
yourself) are busy; I say I am busy ("almost getting fired" kind of busy).

Only solution is to pour in more resources. I have no ideas regarding where to find billionaires.

I still disagree with David that this is a trivial change. My boss requires the auto-generated 
variants' product IDs for hundreds, no less, of variants per product. No reason for me to think 
nobody else needs the same functionality, but different separator.

I guess this discussion is more a management issue than a coding issue? For example, who audited 
the commit of the service "getVariantCombinations"?

This management issue does worry me (as does you, since your flagship CRMSFA/Financials rely on 
OFBiz heavily), so I'll risk taking a retort from David here.

Jonathon

Si Chen wrote:
> David E. Jones wrote:
>>
>> Here are my previous comments about it earlier on the mailing list:
>>
>> =========================================
>> This isn't a universal policy or anything, but I'd say for something 
>> minor like this there isn't a problem with hard-coding it.
>>
>> The whole point of the ID generation is to make the IDs unique. In the 
>> UI you can specify an ID instead of using the default, so it only 
>> matters so much.
>> =========================================
>>
>> In general if this sort of thing were to be configurable, would we 
>> really want it in a properties file? I'd say no, especially since one 
>> of the new objectives that has been discussed recently is to make it 
>> easy to configure things, make things configurable more granularly, 
>> and make it easier to use different seed data files for OOTB 
>> industry-specific configurations.
>>
>> What to do...
>>
>> -David
>>
>>
>> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
>>
>>> Hi
>>>
>>> I actually said that we should do it if anyone had objections, but no 
>>> one did so that's why it went in as is.  But yeah if over-running the 
>>> limit is a possibility then it should probably be changed.  Perhaps 
>>> there should be some code in there anyway for coping with that 
>>> situation, if someones running that many features saving 1 character 
>>> might not be much of a bonus.
>>>
>>> Regards
>>> Scott
>>>
>>> Jacopo Cappellato wrote:
>>>> I agree with Si.
>>>>
>>>> Jacopo
>>>>
>>>> Si Chen wrote:
>>>>> Hey there -
>>>>>
>>>>> This patch is a good idea, but I think Scott Gray suggested that 
>>>>> this "-" could be configured in a properties file, and I think 
>>>>> that's a good idea.  Otherwise, if you have four or five features 
>>>>> you will easily overrun the 20-character productId key limit.  
>>>>> Keeping it in properties file is a good way to allow it to be 
>>>>> modified.  Otherwise it's not very nice to have to go into the code 
>>>>> to do it.
>>>>>
>>>>> Jonathon, you up for doing this and sending in another patch?
>>>>>
>>>>> Si
>>>>>
>>>>> ------------------------------------------------------------------------ 
>>>>>
>>>>>
>>>>> Subject:
>>>>> svn commit: r495891 - 
>>>>> /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>>
>>>>> From:
>>>>> jleroux@apache.org
>>>>> Date:
>>>>> Sat, 13 Jan 2007 12:48:56 -0000
>>>>> To:
>>>>> commits@ofbiz.apache.org
>>>>>
>>>>> To:
>>>>> commits@ofbiz.apache.org
>>>>>
>>>>>
>>>>> Author: jleroux
>>>>> Date: Sat Jan 13 04:48:55 2007
>>>>> New Revision: 495891
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
>>>>> Log:
>>>>> A patch from Jonathon Wong "Prepend feature idCodes with '-'" 
>>>>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>>>
>>>>> Modified:
>>>>>     
>>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>>
>>>>>
>>>>> Modified: 
>>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>>
>>>>> URL: 
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java?view=diff&rev=495891&r1=495890&r2=495891 
>>>>>
>>>>> ============================================================================== 
>>>>>
>>>>> --- 
>>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>> (original)
>>>>> +++ 
>>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>> Sat Jan 13 04:48:55 2007
>>>>> @@ -227,7 +227,7 @@
>>>>>                                 List newFeatures = new LinkedList();
>>>>>                                 List newFeatureIds = new LinkedList();
>>>>>                                 if 
>>>>> (currentFeature.getString("idCode") != null)
>>>>> -                                
>>>>> newCombination.put("defaultVariantProductId", productId + 
>>>>> currentFeature.getString("idCode"));
>>>>> +                                
>>>>> newCombination.put("defaultVariantProductId", productId + "-" + 
>>>>> currentFeature.getString("idCode"));
>>>>>                              else
>>>>>                                  
>>>>> newCombination.put("defaultVariantProductId", productId);
>>>>>                              newFeatures.add(currentFeature);
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>
> David,
> 
> Unfortunately I find this to be somewhat unsatisfactory for two reasons:
> 
> 1.  Any potential problem when present should be addressed or at least 
> considered.  This bit of code possibly adds a trivial enhancement in the 
> eyes of some users but definitely potential for bugs in the case of 
> other users.
> 
> 2.  While it is true that somebody can just correct those on the data 
> entry screen, the fact that this is at the service level rather than 
> just on the screen in a .bsh file means that somebody else might be 
> relying on this service to do things in a way that cannot be corrected 
> on a screen.  To the extent the service layer offers a set of APIs in 
> the form of services, such services should work without resorting to 
> corrections in the view layer.  One of my favorite things about OFBIZ is 
> the separation of the layers.
> 
> Still, I agree with you--let's not have a long discussion about small 
> things like this.  If I find problems with it down the road with this 
> code I'll fix them myself in a way that's acceptable to all.
> 
> Si
> 
> 


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
On Jan 22, 2007, at 11:45 AM, Si Chen wrote:

> David,
>
> Unfortunately I find this to be somewhat unsatisfactory for two  
> reasons:
>
> 1.  Any potential problem when present should be addressed or at  
> least considered.  This bit of code possibly adds a trivial  
> enhancement in the eyes of some users but definitely potential for  
> bugs in the case of other users.
>
> 2.  While it is true that somebody can just correct those on the  
> data entry screen, the fact that this is at the service level  
> rather than just on the screen in a .bsh file means that somebody  
> else might be relying on this service to do things in a way that  
> cannot be corrected on a screen.  To the extent the service layer  
> offers a set of APIs in the form of services, such services should  
> work without resorting to corrections in the view layer.  One of my  
> favorite things about OFBIZ is the separation of the layers.
>
> Still, I agree with you--let's not have a long discussion about  
> small things like this.  If I find problems with it down the road  
> with this code I'll fix them myself in a way that's acceptable to all.

Sounds fine to me.

-David


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Si Chen <si...@opensourcestrategies.com>.
David E. Jones wrote:
>
> Here are my previous comments about it earlier on the mailing list:
>
> =========================================
> This isn't a universal policy or anything, but I'd say for something 
> minor like this there isn't a problem with hard-coding it.
>
> The whole point of the ID generation is to make the IDs unique. In the 
> UI you can specify an ID instead of using the default, so it only 
> matters so much.
> =========================================
>
> In general if this sort of thing were to be configurable, would we 
> really want it in a properties file? I'd say no, especially since one 
> of the new objectives that has been discussed recently is to make it 
> easy to configure things, make things configurable more granularly, 
> and make it easier to use different seed data files for OOTB 
> industry-specific configurations.
>
> What to do...
>
> -David
>
>
> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
>
>> Hi
>>
>> I actually said that we should do it if anyone had objections, but no 
>> one did so that's why it went in as is.  But yeah if over-running the 
>> limit is a possibility then it should probably be changed.  Perhaps 
>> there should be some code in there anyway for coping with that 
>> situation, if someones running that many features saving 1 character 
>> might not be much of a bonus.
>>
>> Regards
>> Scott
>>
>> Jacopo Cappellato wrote:
>>> I agree with Si.
>>>
>>> Jacopo
>>>
>>> Si Chen wrote:
>>>> Hey there -
>>>>
>>>> This patch is a good idea, but I think Scott Gray suggested that 
>>>> this "-" could be configured in a properties file, and I think 
>>>> that's a good idea.  Otherwise, if you have four or five features 
>>>> you will easily overrun the 20-character productId key limit.  
>>>> Keeping it in properties file is a good way to allow it to be 
>>>> modified.  Otherwise it's not very nice to have to go into the code 
>>>> to do it.
>>>>
>>>> Jonathon, you up for doing this and sending in another patch?
>>>>
>>>> Si
>>>>
>>>> ------------------------------------------------------------------------ 
>>>>
>>>>
>>>> Subject:
>>>> svn commit: r495891 - 
>>>> /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>
>>>> From:
>>>> jleroux@apache.org
>>>> Date:
>>>> Sat, 13 Jan 2007 12:48:56 -0000
>>>> To:
>>>> commits@ofbiz.apache.org
>>>>
>>>> To:
>>>> commits@ofbiz.apache.org
>>>>
>>>>
>>>> Author: jleroux
>>>> Date: Sat Jan 13 04:48:55 2007
>>>> New Revision: 495891
>>>>
>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
>>>> Log:
>>>> A patch from Jonathon Wong "Prepend feature idCodes with '-'" 
>>>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>>
>>>> Modified:
>>>>     
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>
>>>>
>>>> Modified: 
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>
>>>> URL: 
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java?view=diff&rev=495891&r1=495890&r2=495891 
>>>>
>>>> ============================================================================== 
>>>>
>>>> --- 
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>> (original)
>>>> +++ 
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>> Sat Jan 13 04:48:55 2007
>>>> @@ -227,7 +227,7 @@
>>>>                                 List newFeatures = new LinkedList();
>>>>                                 List newFeatureIds = new LinkedList();
>>>>                                 if 
>>>> (currentFeature.getString("idCode") != null)
>>>> -                                
>>>> newCombination.put("defaultVariantProductId", productId + 
>>>> currentFeature.getString("idCode"));
>>>> +                                
>>>> newCombination.put("defaultVariantProductId", productId + "-" + 
>>>> currentFeature.getString("idCode"));
>>>>                              else
>>>>                                  
>>>> newCombination.put("defaultVariantProductId", productId);
>>>>                              newFeatures.add(currentFeature);
>>>>
>>>>
>>>
>>>
>>>
>>
>
David,

Unfortunately I find this to be somewhat unsatisfactory for two reasons:

1.  Any potential problem when present should be addressed or at least 
considered.  This bit of code possibly adds a trivial enhancement in the 
eyes of some users but definitely potential for bugs in the case of 
other users.

2.  While it is true that somebody can just correct those on the data 
entry screen, the fact that this is at the service level rather than 
just on the screen in a .bsh file means that somebody else might be 
relying on this service to do things in a way that cannot be corrected 
on a screen.  To the extent the service layer offers a set of APIs in 
the form of services, such services should work without resorting to 
corrections in the view layer.  One of my favorite things about OFBIZ is 
the separation of the layers.

Still, I agree with you--let's not have a long discussion about small 
things like this.  If I find problems with it down the road with this 
code I'll fix them myself in a way that's acceptable to all.

Si

Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Chris Howe <cj...@yahoo.com>.
Maybe this example will help clarify.

You sell a t-shirt, it has variants Red(Rd), Blue (Bl)
and Green (Gr) and sizes Small(S), Medium(M) and Large
(L)

S Rd T-Shirt - productId = 1000
M Rd T-Shirt - productId = 1001
L Rd T-Shirt - productId = 1002
S Bl T-Shirt - productId = 1003
M Bl T-Shirt - productId = 1004
L Bl T-Shirt - productId = 1005
S Gr T-Shirt - productId = 1006
M Gr T-Shirt - productId = 1007
L Gr T-Shirt - productId = 1008

All GoodIdentification.goodIdentificationTypeId  =
MARKETING_PROD_ID

S Rd T-Shirt - 
productId = 1000
GoodIdentification.idValue = Shirt-Rd-S

M Rd T-Shirt - 
productId = 1001
GoodIdentification.idValue = Shirt-Rd-M

L Rd T-Shirt - 
productId = 1002
GoodIdentification.idValue = Shirt-Rd-L

S Bl T-Shirt - 
productId = 1003
GoodIdentification.idValue = Shirt-Bl-S

M Bl T-Shirt - 
productId = 1004
GoodIdentification.idValue = Shirt-Bl-M

L Bl T-Shirt - 
productId = 1005
GoodIdentification.idValue = Shirt-Bl-L

S Gr T-Shirt - 
productId = 1006
GoodIdentification.idValue = Shirt-Gr-S

M Gr T-Shirt - 
productId = 1007
GoodIdentification.idValue = Shirt-Gr-M

L Gr T-Shirt - 
productId = 1008
GoodIdentification.idValue = Shirt-Gr-L


Foreign keys regarding this product are stored with
productId.  Products are displayed to users using the
idValue.


**Note: This is perhaps better modeled by removing
productId from GoodIdentification and instead using an
associative table between GoodIdentification and
Product, but that may be over-designing as the only
real downside to the current implementation is
duplicate GoodIdentification.idValue.

**BTW, this solution solves your field length issue as
well as the idValue is arbitrarily set at 60
characters and because it has no meaning to the
database, can be expanded to handle more characters if
necessary.

**Keep in mind storing the autogenerated id as
Product.productId with international characters may
reduce your potential field length to 5 characters as
unicode characters may take up to 4 places per
character.

--- Jonathon -- Improov <jo...@improov.com> wrote:

> Chris,
> 
> I agree with using GoodIdentification value. Recall
> our discussion about changing branding and 
> model numbers we show to customers.
> 
> But then, what would we use for the variants'
> productId? The productId field seems to have been 
> used so extensively now for product ID purposes as
> well as for display purposes (ecommerce).
> 
> I hope you're not saying we use a single Product
> that has many GoodIdentification values. Then I'd 
> be really confused. I just got the Variants modeling
> (virtual BOMs and all) down pat for my boss!
> 
> Jonathon
> 
> Chris Howe wrote:
> > I think this is best handled by reverting r495891
> and
> > simply giving instruction for the modification for
> > those that wish to use it. With a move towards
> > granularity, we should be discouraging this type
> of
> > use as a primary key to begin with.  This auto
> > generation should be creating a GoodIdentification
> > value instead of a primary key.  If the primary
> key is
> > a surrogate key, let it truly be a surrogate and
> > remove the application data meaning surrounding
> it. 
> > Then keep it hidden from the user of the
> application. 
> > 
> > Once we've finished successfully hiding the
> > Product.productId from the user and possibly
> scripts
> > to regenerate legacy Product Entities, then set up
> the
> > delimiter as a ProductCatalog setting.
> > 
> > 
> > --- "David E. Jones" <jo...@hotwaxmedia.com>
> wrote:
> > 
> >> Here are my previous comments about it earlier on
> >> the mailing list:
> >>
> >> =========================================
> >> This isn't a universal policy or anything, but
> I'd
> >> say for something  
> >> minor like this there isn't a problem with
> >> hard-coding it.
> >>
> >> The whole point of the ID generation is to make
> the
> >> IDs unique. In  
> >> the UI you can specify an ID instead of using the
> >> default, so it only  
> >> matters so much.
> >> =========================================
> >>
> >> In general if this sort of thing were to be
> >> configurable, would we  
> >> really want it in a properties file? I'd say no,
> >> especially since one  
> >> of the new objectives that has been discussed
> >> recently is to make it  
> >> easy to configure things, make things
> configurable
> >> more granularly,  
> >> and make it easier to use different seed data
> files
> >> for OOTB industry- 
> >> specific configurations.
> >>
> >> What to do...
> >>
> >> -David
> >>
> >>
> >> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
> >>
> >>> Hi
> >>>
> >>> I actually said that we should do it if anyone
> had
> >> objections, but  
> >>> no one did so that's why it went in as is.  But
> >> yeah if over- 
> >>> running the limit is a possibility then it
> should
> >> probably be  
> >>> changed.  Perhaps there should be some code in
> >> there anyway for  
> >>> coping with that situation, if someones running
> >> that many features  
> >>> saving 1 character might not be much of a bonus.
> >>>
> >>> Regards
> >>> Scott
> >>>
> >>> Jacopo Cappellato wrote:
> >>>> I agree with Si.
> >>>>
> >>>> Jacopo
> >>>>
> >>>> Si Chen wrote:
> >>>>> Hey there -
> >>>>>
> >>>>> This patch is a good idea, but I think Scott
> >> Gray suggested that  
> >>>>> this "-" could be configured in a properties
> >> file, and I think  
> >>>>> that's a good idea.  Otherwise, if you have
> four
> >> or five features  
> >>>>> you will easily overrun the 20-character
> >> productId key limit.   
> >>>>> Keeping it in properties file is a good way to
> >> allow it to be  
> >>>>> modified.  Otherwise it's not very nice to
> have
> >> to go into the  
> >>>>> code to do it.
> >>>>>
> >>>>> Jonathon, you up for doing this and sending in
> >> another patch?
> >>>>> Si
> >>>>>
> >>>>>
> >
>
--------------------------------------------------------------------
> >>>>> ----
> >>>>>
> >>>>> Subject:
> >>>>> svn commit: r495891 -
> >> /ofbiz/trunk/applications/product/src/org/ 
> >> ofbiz/product/feature/ProductFeatureServices.java
> >>>>> From:
> >>>>> jleroux@apache.org
> >>>>> Date:
> >>>>> Sat, 13 Jan 2007 12:48:56 -0000
> >>>>> To:
> >>>>> commits@ofbiz.apache.org
> >>>>>
> >>>>> To:
> >>>>> commits@ofbiz.apache.org
> >>>>>
> >>>>>
> >>>>> Author: jleroux
> >>>>> Date: Sat Jan 13 04:48:55 2007
> >>>>> New Revision: 495891
> >>>>>
> >>>>> URL:
> >> http://svn.apache.org/viewvc?view=rev&rev=495891
> >>>>> Log:
> >>>>> A patch from Jonathon Wong "Prepend feature
> >> idCodes with  
> >>>>> '-'"
> >> (https://issues.apache.org/jira/browse/OFBIZ-620)
> >>>>> Modified:
> >>>>>    
> >
>
ofbiz/trunk/applications/product/src/org/ofbiz/product/
> >>>>> feature/ProductFeatureServices.java
> >>>>>
> >>>>> Modified:
> >
>
ofbiz/trunk/applications/product/src/org/ofbiz/product/
> >>>>> feature/ProductFeatureServices.java
> >>>>> URL:
> >
>
http://svn.apache.org/viewvc/ofbiz/trunk/applications/
> >
>
product/src/org/ofbiz/product/feature/ProductFeatureServices.java?
> >>>>> view=diff&rev=495891&r1=495890&r2=495891
> >>>>>
> >
>
====================================================================
> >>>>> ==========
> >>>>> ---
> >
>
ofbiz/trunk/applications/product/src/org/ofbiz/product/
> >>>>> feature/ProductFeatureServices.java (original)
> >>>>> +++
> >
>
ofbiz/trunk/applications/product/src/org/ofbiz/product/
> >>>>> feature/ProductFeatureServices.java Sat Jan 13
> >> 04:48:55 2007
> >>>>> @@ -227,7 +227,7 @@
> >>>>>                                 List
> newFeatures
> >> = new LinkedList();
> >>>>>                                 List
> >> newFeatureIds = new  
> >>>>> LinkedList();
> >>>>>                                 if
> >> (currentFeature.getString 
> 
=== message truncated ===


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Jonathon -- Improov <jo...@improov.com>.
Chris,

I agree with using GoodIdentification value. Recall our discussion about changing branding and 
model numbers we show to customers.

But then, what would we use for the variants' productId? The productId field seems to have been 
used so extensively now for product ID purposes as well as for display purposes (ecommerce).

I hope you're not saying we use a single Product that has many GoodIdentification values. Then I'd 
be really confused. I just got the Variants modeling (virtual BOMs and all) down pat for my boss!

Jonathon

Chris Howe wrote:
> I think this is best handled by reverting r495891 and
> simply giving instruction for the modification for
> those that wish to use it. With a move towards
> granularity, we should be discouraging this type of
> use as a primary key to begin with.  This auto
> generation should be creating a GoodIdentification
> value instead of a primary key.  If the primary key is
> a surrogate key, let it truly be a surrogate and
> remove the application data meaning surrounding it. 
> Then keep it hidden from the user of the application. 
> 
> Once we've finished successfully hiding the
> Product.productId from the user and possibly scripts
> to regenerate legacy Product Entities, then set up the
> delimiter as a ProductCatalog setting.
> 
> 
> --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
> 
>> Here are my previous comments about it earlier on
>> the mailing list:
>>
>> =========================================
>> This isn't a universal policy or anything, but I'd
>> say for something  
>> minor like this there isn't a problem with
>> hard-coding it.
>>
>> The whole point of the ID generation is to make the
>> IDs unique. In  
>> the UI you can specify an ID instead of using the
>> default, so it only  
>> matters so much.
>> =========================================
>>
>> In general if this sort of thing were to be
>> configurable, would we  
>> really want it in a properties file? I'd say no,
>> especially since one  
>> of the new objectives that has been discussed
>> recently is to make it  
>> easy to configure things, make things configurable
>> more granularly,  
>> and make it easier to use different seed data files
>> for OOTB industry- 
>> specific configurations.
>>
>> What to do...
>>
>> -David
>>
>>
>> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
>>
>>> Hi
>>>
>>> I actually said that we should do it if anyone had
>> objections, but  
>>> no one did so that's why it went in as is.  But
>> yeah if over- 
>>> running the limit is a possibility then it should
>> probably be  
>>> changed.  Perhaps there should be some code in
>> there anyway for  
>>> coping with that situation, if someones running
>> that many features  
>>> saving 1 character might not be much of a bonus.
>>>
>>> Regards
>>> Scott
>>>
>>> Jacopo Cappellato wrote:
>>>> I agree with Si.
>>>>
>>>> Jacopo
>>>>
>>>> Si Chen wrote:
>>>>> Hey there -
>>>>>
>>>>> This patch is a good idea, but I think Scott
>> Gray suggested that  
>>>>> this "-" could be configured in a properties
>> file, and I think  
>>>>> that's a good idea.  Otherwise, if you have four
>> or five features  
>>>>> you will easily overrun the 20-character
>> productId key limit.   
>>>>> Keeping it in properties file is a good way to
>> allow it to be  
>>>>> modified.  Otherwise it's not very nice to have
>> to go into the  
>>>>> code to do it.
>>>>>
>>>>> Jonathon, you up for doing this and sending in
>> another patch?
>>>>> Si
>>>>>
>>>>>
> --------------------------------------------------------------------
>>>>> ----
>>>>>
>>>>> Subject:
>>>>> svn commit: r495891 -
>> /ofbiz/trunk/applications/product/src/org/ 
>> ofbiz/product/feature/ProductFeatureServices.java
>>>>> From:
>>>>> jleroux@apache.org
>>>>> Date:
>>>>> Sat, 13 Jan 2007 12:48:56 -0000
>>>>> To:
>>>>> commits@ofbiz.apache.org
>>>>>
>>>>> To:
>>>>> commits@ofbiz.apache.org
>>>>>
>>>>>
>>>>> Author: jleroux
>>>>> Date: Sat Jan 13 04:48:55 2007
>>>>> New Revision: 495891
>>>>>
>>>>> URL:
>> http://svn.apache.org/viewvc?view=rev&rev=495891
>>>>> Log:
>>>>> A patch from Jonathon Wong "Prepend feature
>> idCodes with  
>>>>> '-'"
>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>>> Modified:
>>>>>    
> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>>>> feature/ProductFeatureServices.java
>>>>>
>>>>> Modified:
> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>>>> feature/ProductFeatureServices.java
>>>>> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/
> product/src/org/ofbiz/product/feature/ProductFeatureServices.java?
>>>>> view=diff&rev=495891&r1=495890&r2=495891
>>>>>
> ====================================================================
>>>>> ==========
>>>>> ---
> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>>>> feature/ProductFeatureServices.java (original)
>>>>> +++
> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>>>> feature/ProductFeatureServices.java Sat Jan 13
>> 04:48:55 2007
>>>>> @@ -227,7 +227,7 @@
>>>>>                                 List newFeatures
>> = new LinkedList();
>>>>>                                 List
>> newFeatureIds = new  
>>>>> LinkedList();
>>>>>                                 if
>> (currentFeature.getString 
>>>>> ("idCode") != null)
>>>>> -                               
>> newCombination.put 
>>>>> ("defaultVariantProductId", productId +
>> currentFeature.getString 
>>>>> ("idCode"));
>>>>> +                               
>> newCombination.put 
>>>>> ("defaultVariantProductId", productId + "-" +  
>>>>> currentFeature.getString("idCode"));
>>>>>                              else
>>>>>                                 
>> newCombination.put 
>>>>> ("defaultVariantProductId", productId);
>>>>>                             
>> newFeatures.add(currentFeature);
>>>>>
>>>>
>>>>
>>
> 
> 


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Jonathon -- Improov <jo...@improov.com>.
David,

Yes, I agree with continuing to use the productId as it's always been used. I'm not going fork a 
OFBiz-VeryProperERD branch that has a total rework of all data modeling.

As for this change being minor, like I said before, it's not minor to some of us. I need to 
auto-generate hundreds of defaultVariantProductId(s) per product (very close to built-to-order 
business model). Say if I hardcoded it to '-' now, and Si Chen has same requirements but needs a 
'_' for separator instead, poor Si Chen would messing his local code branch with a rather 
unnecessary "customization". I do agree with Si Chen that this should be left configurable, either 
in config files or preferably within OFBiz itself.

Yeah, sure, Si Chen could manually override all the default '-' separators by hand, in the Html UI 
as a user. But for hundreds of variants? It's the computer age! (Or have we passed it?)

Jonathon

David E. Jones wrote:
> 
> I think this is a little extreme, and I'm not sure hiding the productId 
> is a good idea. The way it is now works really well for most companies 
> I've worked with. And for the rest, having the productIds visible is not 
> too much a problem, and they can be sequenced just fine.
> 
> I should maybe say it again: this change it SO SO SO MINOR it is hardly 
> worth reading the commit, let along discussing it. It is a default 
> setting for a fairly special purpose UI and the ID can be manually set 
> in the UI anyway.
> 
> -David
> 
> 
> On Jan 19, 2007, at 10:31 PM, Chris Howe wrote:
> 
>> I think this is best handled by reverting r495891 and
>> simply giving instruction for the modification for
>> those that wish to use it. With a move towards
>> granularity, we should be discouraging this type of
>> use as a primary key to begin with.  This auto
>> generation should be creating a GoodIdentification
>> value instead of a primary key.  If the primary key is
>> a surrogate key, let it truly be a surrogate and
>> remove the application data meaning surrounding it.
>> Then keep it hidden from the user of the application.
>>
>> Once we've finished successfully hiding the
>> Product.productId from the user and possibly scripts
>> to regenerate legacy Product Entities, then set up the
>> delimiter as a ProductCatalog setting.
>>
>>
>> --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
>>
>>>
>>> Here are my previous comments about it earlier on
>>> the mailing list:
>>>
>>> =========================================
>>> This isn't a universal policy or anything, but I'd
>>> say for something
>>> minor like this there isn't a problem with
>>> hard-coding it.
>>>
>>> The whole point of the ID generation is to make the
>>> IDs unique. In
>>> the UI you can specify an ID instead of using the
>>> default, so it only
>>> matters so much.
>>> =========================================
>>>
>>> In general if this sort of thing were to be
>>> configurable, would we
>>> really want it in a properties file? I'd say no,
>>> especially since one
>>> of the new objectives that has been discussed
>>> recently is to make it
>>> easy to configure things, make things configurable
>>> more granularly,
>>> and make it easier to use different seed data files
>>> for OOTB industry-
>>> specific configurations.
>>>
>>> What to do...
>>>
>>> -David
>>>
>>>
>>> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
>>>
>>>> Hi
>>>>
>>>> I actually said that we should do it if anyone had
>>> objections, but
>>>> no one did so that's why it went in as is.  But
>>> yeah if over-
>>>> running the limit is a possibility then it should
>>> probably be
>>>> changed.  Perhaps there should be some code in
>>> there anyway for
>>>> coping with that situation, if someones running
>>> that many features
>>>> saving 1 character might not be much of a bonus.
>>>>
>>>> Regards
>>>> Scott
>>>>
>>>> Jacopo Cappellato wrote:
>>>>> I agree with Si.
>>>>>
>>>>> Jacopo
>>>>>
>>>>> Si Chen wrote:
>>>>>> Hey there -
>>>>>>
>>>>>> This patch is a good idea, but I think Scott
>>> Gray suggested that
>>>>>> this "-" could be configured in a properties
>>> file, and I think
>>>>>> that's a good idea.  Otherwise, if you have four
>>> or five features
>>>>>> you will easily overrun the 20-character
>>> productId key limit.
>>>>>> Keeping it in properties file is a good way to
>>> allow it to be
>>>>>> modified.  Otherwise it's not very nice to have
>>> to go into the
>>>>>> code to do it.
>>>>>>
>>>>>> Jonathon, you up for doing this and sending in
>>> another patch?
>>>>>>
>>>>>> Si
>>>>>>
>>>>>>
>>>
>> --------------------------------------------------------------------
>>>
>>>>>> ----
>>>>>>
>>>>>> Subject:
>>>>>> svn commit: r495891 -
>>> /ofbiz/trunk/applications/product/src/org/
>>>>>>
>>> ofbiz/product/feature/ProductFeatureServices.java
>>>>>> From:
>>>>>> jleroux@apache.org
>>>>>> Date:
>>>>>> Sat, 13 Jan 2007 12:48:56 -0000
>>>>>> To:
>>>>>> commits@ofbiz.apache.org
>>>>>>
>>>>>> To:
>>>>>> commits@ofbiz.apache.org
>>>>>>
>>>>>>
>>>>>> Author: jleroux
>>>>>> Date: Sat Jan 13 04:48:55 2007
>>>>>> New Revision: 495891
>>>>>>
>>>>>> URL:
>>> http://svn.apache.org/viewvc?view=rev&rev=495891
>>>>>> Log:
>>>>>> A patch from Jonathon Wong "Prepend feature
>>> idCodes with
>>>>>> '-'"
>>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>>>>
>>>>>> Modified:
>>>>>>
>>>
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>>
>>>>>> feature/ProductFeatureServices.java
>>>>>>
>>>>>> Modified:
>>>
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>>
>>>>>> feature/ProductFeatureServices.java
>>>>>> URL:
>>>
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/
>>>
>>>>>>
>>>
>> product/src/org/ofbiz/product/feature/ProductFeatureServices.java?
>>>
>>>>>> view=diff&rev=495891&r1=495890&r2=495891
>>>>>>
>>>
>> ====================================================================
>>>
>>>>>> ==========
>>>>>> ---
>>>
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>>
>>>>>> feature/ProductFeatureServices.java (original)
>>>>>> +++
>>>
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>>
>>>>>> feature/ProductFeatureServices.java Sat Jan 13
>>> 04:48:55 2007
>>>>>> @@ -227,7 +227,7 @@
>>>>>>                                 List newFeatures
>>> = new LinkedList();
>>>>>>                                 List
>>> newFeatureIds = new
>>>>>> LinkedList();
>>>>>>                                 if
>>> (currentFeature.getString
>>>>>> ("idCode") != null)
>>>>>> -
>>> newCombination.put
>>>>>> ("defaultVariantProductId", productId +
>>> currentFeature.getString
>>>>>> ("idCode"));
>>>>>> +
>>> newCombination.put
>>>>>> ("defaultVariantProductId", productId + "-" +
>>>>>> currentFeature.getString("idCode"));
>>>>>>                              else
>>>>>>
>>> newCombination.put
>>>>>> ("defaultVariantProductId", productId);
>>>>>>
>>> newFeatures.add(currentFeature);
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
> 


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Chris Howe <cj...@yahoo.com>.
David,
Whether OOTB Ofbiz will support both ways or not
doesn't make exposing the primary key to the user any
less dangerous.

The generated Id in the variant example should be the
marketing  product id in GoodIdentification, not the
primary key Product.productId.  The marketing product
id you show to the user, the primary key
Product.productId, you hide.  As the user of the data,
the customer is concerned with the marketing product
id, not the primary key productId.  The primary key
product Id is [should be] a surrogate key (
http://en.wikipedia.org/wiki/Surrogate_key ).  It has
no meaning except to uniquely identify a product in
the database, not in real life.  

As your foreign key in your OrderHeader entity, you
would have your Product.productId.  What shows up on
the UI for the sales order is the marketing product
Id.  When displaying a phone number, do you show the
user the contactMechId?


--- "David E. Jones" <jo...@hotwaxmedia.com> wrote:

> 
> Chris,
> 
> The point is that in the system we're going to
> support both ways.  
> That's it. I see no good reason to write a bunch of
> stuff to force  
> one way or another, especially when we've already
> written stuff  
> (that's fairly simple really) to allow both.
> 
> Another thing to consider: even if a productId is
> generated it  
> doesn't mean you necessarily want it hidden. People
> may want to write  
> it down, have fields where they can enter with a
> barcode scanner,  
> etc, much like an order or invoice ID.
> 
> -David
> 
> 
> On Jan 20, 2007, at 12:15 AM, Chris Howe wrote:
> 
> > I agree that the delimiter issue is very, very
> small,
> > however the manner in which the order and
> ecommerce
> > component utilize the productId is not unique in a
> > company's life cycle.  Granted, you won't run into
> > this problem on initial deployment or if your
> product
> > never changes.  But very few companies keep their
> > product the exact same over the span of several
> years.
> >
> > To give you a real life example of the problem
> this
> > practice exposes.
> > We do business with a company that exposes their
> > primary key as their productId that we order with.
> > They manufacture power wheelchairs.  Their vendor
> for
> > the seat kit (upholstery and mounting hardware) on
> the
> > chairs started becoming unreliable with delivery
> and
> > they had to get the upholstery from another vendor
> > (began as a mixed utilization before going
> completely
> > with the new vendor).  In order to keep up their
> > quality control tracking, they had to issue a new
> > productId to distinguish the two chairs apart as
> seat
> > kits are not serialized.  They allow their
> customers
> > to view real time inventory.  They weren't able to
> > effectively communicate this change of productId
> to
> > their customers, so their customers kept getting
> "out
> > of stock" messages on the power wheelchairs. 
> Their
> > customers ended up meeting their chair needs from
> > other suppliers.  They experienced a 25% decline
> in
> > power wheelchair sales in the quarter this
> occurred,
> > had to discount their overstock the following
> quarter
> > and they have yet to win back many of the
> customers
> > that fulfilled their initial orders with another
> > supplier as well as the customers who now have the
> > perception of a "new" untested chair on the
> market.
> >
> > Because of a change in upholstery they had to
> consider
> > the ramifications on marketing, sales force
> training,
> > and inventory.  This really isn't necessary.  To
> the
> > customers these are the exact same chairs, they
> should
> > be ordering them with the exact same marketed
> > productId.  Their inventory counts should be
> > consolidating the separate counts.  Their sales
> force
> > should not be worried about how to explain the
> > differences in the two chairs.  This could be a
> lot
> > simpler.
> >
> > This may be an extreme change, but it's extremely
> more
> > flexible and more accurately describes a
> business's
> > product line.
> >
> > ,Chris
> >
> > --- "David E. Jones" <jo...@hotwaxmedia.com>
> wrote:
> >
> >>
> >> I think this is a little extreme, and I'm not
> sure
> >> hiding the
> >> productId is a good idea. The way it is now works
> >> really well for
> >> most companies I've worked with. And for the
> rest,
> >> having the
> >> productIds visible is not too much a problem, and
> >> they can be
> >> sequenced just fine.
> >>
> >> I should maybe say it again: this change it SO SO
> SO
> >> MINOR it is
> >> hardly worth reading the commit, let along
> >> discussing it. It is a
> >> default setting for a fairly special purpose UI
> and
> >> the ID can be
> >> manually set in the UI anyway.
> >>
> >> -David
> >>
> >>
> >> On Jan 19, 2007, at 10:31 PM, Chris Howe wrote:
> >>
> >>> I think this is best handled by reverting
> r495891
> >> and
> >>> simply giving instruction for the modification
> for
> >>> those that wish to use it. With a move towards
> >>> granularity, we should be discouraging this type
> >> of
> >>> use as a primary key to begin with.  This auto
> >>> generation should be creating a
> GoodIdentification
> >>> value instead of a primary key.  If the primary
> >> key is
> >>> a surrogate key, let it truly be a surrogate and
> >>> remove the application data meaning surrounding
> >> it.
> >>> Then keep it hidden from the user of the
> >> application.
> >>>
> >>> Once we've finished successfully hiding the
> >>> Product.productId from the user and possibly
> >> scripts
> >>> to regenerate legacy Product Entities, then set
> up
> >> the
> >>> delimiter as a ProductCatalog setting.
> >>>
> >>>
> >>> --- "David E. Jones" <jo...@hotwaxmedia.com>
> >> wrote:
> >>>
> >>>>
> >>>> Here are my previous comments about it earlier
> on
> >>>> the mailing list:
> >>>>
> >>>> =========================================
> >>>> This isn't a universal policy or anything, but
> >> I'd
> >>>> say for something
> >>>> minor like this there isn't a problem with
> >>>> hard-coding it.
> >>>>
> >>>> The whole point of the ID generation is to make
> >> the
> >>>> IDs unique. In
> >>>> the UI you can specify an ID instead of using
> the
> >>>> default, so it only
> >>>> matters so much.
> >>>> =========================================
> >>>>
> >>>> In general if this sort of thing were to be
> >>>> configurable, would we
> >>>> really want it in a properties file? I'd say
> no,
> >>>> especially since one
> >>>> of the new objectives that has been discussed
> >>>> recently is to make it
> >>>> easy to configure things, make things
> >> configurable
> >>>> more granularly,
> >>>> and make it easier to use different seed data
> >> files
> >>>> for OOTB industry-
> >>>> specific configurations.
> >>>>
> >>>> What to do...
> 
=== message truncated ===


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
Chris,

The point is that in the system we're going to support both ways.  
That's it. I see no good reason to write a bunch of stuff to force  
one way or another, especially when we've already written stuff  
(that's fairly simple really) to allow both.

Another thing to consider: even if a productId is generated it  
doesn't mean you necessarily want it hidden. People may want to write  
it down, have fields where they can enter with a barcode scanner,  
etc, much like an order or invoice ID.

-David


On Jan 20, 2007, at 12:15 AM, Chris Howe wrote:

> I agree that the delimiter issue is very, very small,
> however the manner in which the order and ecommerce
> component utilize the productId is not unique in a
> company's life cycle.  Granted, you won't run into
> this problem on initial deployment or if your product
> never changes.  But very few companies keep their
> product the exact same over the span of several years.
>
> To give you a real life example of the problem this
> practice exposes.
> We do business with a company that exposes their
> primary key as their productId that we order with.
> They manufacture power wheelchairs.  Their vendor for
> the seat kit (upholstery and mounting hardware) on the
> chairs started becoming unreliable with delivery and
> they had to get the upholstery from another vendor
> (began as a mixed utilization before going completely
> with the new vendor).  In order to keep up their
> quality control tracking, they had to issue a new
> productId to distinguish the two chairs apart as seat
> kits are not serialized.  They allow their customers
> to view real time inventory.  They weren't able to
> effectively communicate this change of productId to
> their customers, so their customers kept getting "out
> of stock" messages on the power wheelchairs.  Their
> customers ended up meeting their chair needs from
> other suppliers.  They experienced a 25% decline in
> power wheelchair sales in the quarter this occurred,
> had to discount their overstock the following quarter
> and they have yet to win back many of the customers
> that fulfilled their initial orders with another
> supplier as well as the customers who now have the
> perception of a "new" untested chair on the market.
>
> Because of a change in upholstery they had to consider
> the ramifications on marketing, sales force training,
> and inventory.  This really isn't necessary.  To the
> customers these are the exact same chairs, they should
> be ordering them with the exact same marketed
> productId.  Their inventory counts should be
> consolidating the separate counts.  Their sales force
> should not be worried about how to explain the
> differences in the two chairs.  This could be a lot
> simpler.
>
> This may be an extreme change, but it's extremely more
> flexible and more accurately describes a business's
> product line.
>
> ,Chris
>
> --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
>
>>
>> I think this is a little extreme, and I'm not sure
>> hiding the
>> productId is a good idea. The way it is now works
>> really well for
>> most companies I've worked with. And for the rest,
>> having the
>> productIds visible is not too much a problem, and
>> they can be
>> sequenced just fine.
>>
>> I should maybe say it again: this change it SO SO SO
>> MINOR it is
>> hardly worth reading the commit, let along
>> discussing it. It is a
>> default setting for a fairly special purpose UI and
>> the ID can be
>> manually set in the UI anyway.
>>
>> -David
>>
>>
>> On Jan 19, 2007, at 10:31 PM, Chris Howe wrote:
>>
>>> I think this is best handled by reverting r495891
>> and
>>> simply giving instruction for the modification for
>>> those that wish to use it. With a move towards
>>> granularity, we should be discouraging this type
>> of
>>> use as a primary key to begin with.  This auto
>>> generation should be creating a GoodIdentification
>>> value instead of a primary key.  If the primary
>> key is
>>> a surrogate key, let it truly be a surrogate and
>>> remove the application data meaning surrounding
>> it.
>>> Then keep it hidden from the user of the
>> application.
>>>
>>> Once we've finished successfully hiding the
>>> Product.productId from the user and possibly
>> scripts
>>> to regenerate legacy Product Entities, then set up
>> the
>>> delimiter as a ProductCatalog setting.
>>>
>>>
>>> --- "David E. Jones" <jo...@hotwaxmedia.com>
>> wrote:
>>>
>>>>
>>>> Here are my previous comments about it earlier on
>>>> the mailing list:
>>>>
>>>> =========================================
>>>> This isn't a universal policy or anything, but
>> I'd
>>>> say for something
>>>> minor like this there isn't a problem with
>>>> hard-coding it.
>>>>
>>>> The whole point of the ID generation is to make
>> the
>>>> IDs unique. In
>>>> the UI you can specify an ID instead of using the
>>>> default, so it only
>>>> matters so much.
>>>> =========================================
>>>>
>>>> In general if this sort of thing were to be
>>>> configurable, would we
>>>> really want it in a properties file? I'd say no,
>>>> especially since one
>>>> of the new objectives that has been discussed
>>>> recently is to make it
>>>> easy to configure things, make things
>> configurable
>>>> more granularly,
>>>> and make it easier to use different seed data
>> files
>>>> for OOTB industry-
>>>> specific configurations.
>>>>
>>>> What to do...
>>>>
>>>> -David
>>>>
>>>>
>>>> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> I actually said that we should do it if anyone
>> had
>>>> objections, but
>>>>> no one did so that's why it went in as is.  But
>>>> yeah if over-
>>>>> running the limit is a possibility then it
>> should
>>>> probably be
>>>>> changed.  Perhaps there should be some code in
>>>> there anyway for
>>>>> coping with that situation, if someones running
>>>> that many features
>>>>> saving 1 character might not be much of a bonus.
>>>>>
>>>>> Regards
>>>>> Scott
>>>>>
>>>>> Jacopo Cappellato wrote:
>>>>>> I agree with Si.
>>>>>>
>>>>>> Jacopo
>>>>>>
>>>>>> Si Chen wrote:
>>>>>>> Hey there -
>>>>>>>
>>>>>>> This patch is a good idea, but I think Scott
>>>> Gray suggested that
>>>>>>> this "-" could be configured in a properties
>>>> file, and I think
>>>>>>> that's a good idea.  Otherwise, if you have
>> four
>>>> or five features
>>>>>>> you will easily overrun the 20-character
>>>> productId key limit.
>>>>>>> Keeping it in properties file is a good way to
>>>> allow it to be
>>>>>>> modified.  Otherwise it's not very nice to
>> have
>>>> to go into the
>>>>>>> code to do it.
>>>>>>>
>>>>>>> Jonathon, you up for doing this and sending in
>>>> another patch?
>>>>>>>
>>>>>>> Si
>>>>>>>
>>>>>>>
>>>>
>>>
>>
> --------------------------------------------------------------------
>>>>
>>>>>>> ----
>>>>>>>
>>>>>>> Subject:
>>>>>>> svn commit: r495891 -
>>>> /ofbiz/trunk/applications/product/src/org/
>>>>>>>
>>>> ofbiz/product/feature/ProductFeatureServices.java
>>>>>>> From:
>>>>>>> jleroux@apache.org
>>>>>>> Date:
>>>>>>> Sat, 13 Jan 2007 12:48:56 -0000
>>>>>>> To:
>>>>>>> commits@ofbiz.apache.org
>>>>>>>
>>>>>>> To:
>>>>>>> commits@ofbiz.apache.org
>>>>>>>
>>>>>>>
>>>>>>> Author: jleroux
>>>>>>> Date: Sat Jan 13 04:48:55 2007
>>>>>>> New Revision: 495891
>>>>>>>
>>>>>>> URL:
>>>> http://svn.apache.org/viewvc?view=rev&rev=495891
>>>>>>> Log:
>>>>>>> A patch from Jonathon Wong "Prepend feature
>>>> idCodes with
>>>>>>> '-'"
>>>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>>>>>
>>>>>>> Modified:
>>>>>>>
>>>>
>>>
>>
> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>>>
>>>>>>> feature/ProductFeatureServices.java
>>>>>>>
>>>>>>> Modified:
>>>>
>>>
>>
> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>>>
>>>>>>> feature/ProductFeatureServices.java
>>>>>>> URL:
>>>>
>>>
>>
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/
>>>>
>>>>>>>
>>>>
>>>
>>
> product/src/org/ofbiz/product/feature/ProductFeatureServices.java?
>>>>
>>>>>>> view=diff&rev=495891&r1=495890&r2=495891
>>>>>>>
>>>>
>>
> === message truncated ===
>


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Chris Howe <cj...@yahoo.com>.
I agree that the delimiter issue is very, very small,
however the manner in which the order and ecommerce
component utilize the productId is not unique in a
company's life cycle.  Granted, you won't run into
this problem on initial deployment or if your product
never changes.  But very few companies keep their
product the exact same over the span of several years.

To give you a real life example of the problem this
practice exposes.
We do business with a company that exposes their
primary key as their productId that we order with. 
They manufacture power wheelchairs.  Their vendor for
the seat kit (upholstery and mounting hardware) on the
chairs started becoming unreliable with delivery and
they had to get the upholstery from another vendor
(began as a mixed utilization before going completely
with the new vendor).  In order to keep up their
quality control tracking, they had to issue a new
productId to distinguish the two chairs apart as seat
kits are not serialized.  They allow their customers
to view real time inventory.  They weren't able to
effectively communicate this change of productId to
their customers, so their customers kept getting "out
of stock" messages on the power wheelchairs.  Their
customers ended up meeting their chair needs from
other suppliers.  They experienced a 25% decline in
power wheelchair sales in the quarter this occurred,
had to discount their overstock the following quarter
and they have yet to win back many of the customers
that fulfilled their initial orders with another
supplier as well as the customers who now have the
perception of a "new" untested chair on the market.

Because of a change in upholstery they had to consider
the ramifications on marketing, sales force training,
and inventory.  This really isn't necessary.  To the
customers these are the exact same chairs, they should
be ordering them with the exact same marketed
productId.  Their inventory counts should be
consolidating the separate counts.  Their sales force
should not be worried about how to explain the
differences in the two chairs.  This could be a lot
simpler.

This may be an extreme change, but it's extremely more
flexible and more accurately describes a business's
product line.

,Chris

--- "David E. Jones" <jo...@hotwaxmedia.com> wrote:

> 
> I think this is a little extreme, and I'm not sure
> hiding the  
> productId is a good idea. The way it is now works
> really well for  
> most companies I've worked with. And for the rest,
> having the  
> productIds visible is not too much a problem, and
> they can be  
> sequenced just fine.
> 
> I should maybe say it again: this change it SO SO SO
> MINOR it is  
> hardly worth reading the commit, let along
> discussing it. It is a  
> default setting for a fairly special purpose UI and
> the ID can be  
> manually set in the UI anyway.
> 
> -David
> 
> 
> On Jan 19, 2007, at 10:31 PM, Chris Howe wrote:
> 
> > I think this is best handled by reverting r495891
> and
> > simply giving instruction for the modification for
> > those that wish to use it. With a move towards
> > granularity, we should be discouraging this type
> of
> > use as a primary key to begin with.  This auto
> > generation should be creating a GoodIdentification
> > value instead of a primary key.  If the primary
> key is
> > a surrogate key, let it truly be a surrogate and
> > remove the application data meaning surrounding
> it.
> > Then keep it hidden from the user of the
> application.
> >
> > Once we've finished successfully hiding the
> > Product.productId from the user and possibly
> scripts
> > to regenerate legacy Product Entities, then set up
> the
> > delimiter as a ProductCatalog setting.
> >
> >
> > --- "David E. Jones" <jo...@hotwaxmedia.com>
> wrote:
> >
> >>
> >> Here are my previous comments about it earlier on
> >> the mailing list:
> >>
> >> =========================================
> >> This isn't a universal policy or anything, but
> I'd
> >> say for something
> >> minor like this there isn't a problem with
> >> hard-coding it.
> >>
> >> The whole point of the ID generation is to make
> the
> >> IDs unique. In
> >> the UI you can specify an ID instead of using the
> >> default, so it only
> >> matters so much.
> >> =========================================
> >>
> >> In general if this sort of thing were to be
> >> configurable, would we
> >> really want it in a properties file? I'd say no,
> >> especially since one
> >> of the new objectives that has been discussed
> >> recently is to make it
> >> easy to configure things, make things
> configurable
> >> more granularly,
> >> and make it easier to use different seed data
> files
> >> for OOTB industry-
> >> specific configurations.
> >>
> >> What to do...
> >>
> >> -David
> >>
> >>
> >> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
> >>
> >>> Hi
> >>>
> >>> I actually said that we should do it if anyone
> had
> >> objections, but
> >>> no one did so that's why it went in as is.  But
> >> yeah if over-
> >>> running the limit is a possibility then it
> should
> >> probably be
> >>> changed.  Perhaps there should be some code in
> >> there anyway for
> >>> coping with that situation, if someones running
> >> that many features
> >>> saving 1 character might not be much of a bonus.
> >>>
> >>> Regards
> >>> Scott
> >>>
> >>> Jacopo Cappellato wrote:
> >>>> I agree with Si.
> >>>>
> >>>> Jacopo
> >>>>
> >>>> Si Chen wrote:
> >>>>> Hey there -
> >>>>>
> >>>>> This patch is a good idea, but I think Scott
> >> Gray suggested that
> >>>>> this "-" could be configured in a properties
> >> file, and I think
> >>>>> that's a good idea.  Otherwise, if you have
> four
> >> or five features
> >>>>> you will easily overrun the 20-character
> >> productId key limit.
> >>>>> Keeping it in properties file is a good way to
> >> allow it to be
> >>>>> modified.  Otherwise it's not very nice to
> have
> >> to go into the
> >>>>> code to do it.
> >>>>>
> >>>>> Jonathon, you up for doing this and sending in
> >> another patch?
> >>>>>
> >>>>> Si
> >>>>>
> >>>>>
> >>
> >
>
--------------------------------------------------------------------
> >>
> >>>>> ----
> >>>>>
> >>>>> Subject:
> >>>>> svn commit: r495891 -
> >> /ofbiz/trunk/applications/product/src/org/
> >>>>>
> >> ofbiz/product/feature/ProductFeatureServices.java
> >>>>> From:
> >>>>> jleroux@apache.org
> >>>>> Date:
> >>>>> Sat, 13 Jan 2007 12:48:56 -0000
> >>>>> To:
> >>>>> commits@ofbiz.apache.org
> >>>>>
> >>>>> To:
> >>>>> commits@ofbiz.apache.org
> >>>>>
> >>>>>
> >>>>> Author: jleroux
> >>>>> Date: Sat Jan 13 04:48:55 2007
> >>>>> New Revision: 495891
> >>>>>
> >>>>> URL:
> >> http://svn.apache.org/viewvc?view=rev&rev=495891
> >>>>> Log:
> >>>>> A patch from Jonathon Wong "Prepend feature
> >> idCodes with
> >>>>> '-'"
> >> (https://issues.apache.org/jira/browse/OFBIZ-620)
> >>>>>
> >>>>> Modified:
> >>>>>
> >>
> >
>
ofbiz/trunk/applications/product/src/org/ofbiz/product/
> >>
> >>>>> feature/ProductFeatureServices.java
> >>>>>
> >>>>> Modified:
> >>
> >
>
ofbiz/trunk/applications/product/src/org/ofbiz/product/
> >>
> >>>>> feature/ProductFeatureServices.java
> >>>>> URL:
> >>
> >
>
http://svn.apache.org/viewvc/ofbiz/trunk/applications/
> >>
> >>>>>
> >>
> >
>
product/src/org/ofbiz/product/feature/ProductFeatureServices.java?
> >>
> >>>>> view=diff&rev=495891&r1=495890&r2=495891
> >>>>>
> >>
> 
=== message truncated ===


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
I think this is a little extreme, and I'm not sure hiding the  
productId is a good idea. The way it is now works really well for  
most companies I've worked with. And for the rest, having the  
productIds visible is not too much a problem, and they can be  
sequenced just fine.

I should maybe say it again: this change it SO SO SO MINOR it is  
hardly worth reading the commit, let along discussing it. It is a  
default setting for a fairly special purpose UI and the ID can be  
manually set in the UI anyway.

-David


On Jan 19, 2007, at 10:31 PM, Chris Howe wrote:

> I think this is best handled by reverting r495891 and
> simply giving instruction for the modification for
> those that wish to use it. With a move towards
> granularity, we should be discouraging this type of
> use as a primary key to begin with.  This auto
> generation should be creating a GoodIdentification
> value instead of a primary key.  If the primary key is
> a surrogate key, let it truly be a surrogate and
> remove the application data meaning surrounding it.
> Then keep it hidden from the user of the application.
>
> Once we've finished successfully hiding the
> Product.productId from the user and possibly scripts
> to regenerate legacy Product Entities, then set up the
> delimiter as a ProductCatalog setting.
>
>
> --- "David E. Jones" <jo...@hotwaxmedia.com> wrote:
>
>>
>> Here are my previous comments about it earlier on
>> the mailing list:
>>
>> =========================================
>> This isn't a universal policy or anything, but I'd
>> say for something
>> minor like this there isn't a problem with
>> hard-coding it.
>>
>> The whole point of the ID generation is to make the
>> IDs unique. In
>> the UI you can specify an ID instead of using the
>> default, so it only
>> matters so much.
>> =========================================
>>
>> In general if this sort of thing were to be
>> configurable, would we
>> really want it in a properties file? I'd say no,
>> especially since one
>> of the new objectives that has been discussed
>> recently is to make it
>> easy to configure things, make things configurable
>> more granularly,
>> and make it easier to use different seed data files
>> for OOTB industry-
>> specific configurations.
>>
>> What to do...
>>
>> -David
>>
>>
>> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
>>
>>> Hi
>>>
>>> I actually said that we should do it if anyone had
>> objections, but
>>> no one did so that's why it went in as is.  But
>> yeah if over-
>>> running the limit is a possibility then it should
>> probably be
>>> changed.  Perhaps there should be some code in
>> there anyway for
>>> coping with that situation, if someones running
>> that many features
>>> saving 1 character might not be much of a bonus.
>>>
>>> Regards
>>> Scott
>>>
>>> Jacopo Cappellato wrote:
>>>> I agree with Si.
>>>>
>>>> Jacopo
>>>>
>>>> Si Chen wrote:
>>>>> Hey there -
>>>>>
>>>>> This patch is a good idea, but I think Scott
>> Gray suggested that
>>>>> this "-" could be configured in a properties
>> file, and I think
>>>>> that's a good idea.  Otherwise, if you have four
>> or five features
>>>>> you will easily overrun the 20-character
>> productId key limit.
>>>>> Keeping it in properties file is a good way to
>> allow it to be
>>>>> modified.  Otherwise it's not very nice to have
>> to go into the
>>>>> code to do it.
>>>>>
>>>>> Jonathon, you up for doing this and sending in
>> another patch?
>>>>>
>>>>> Si
>>>>>
>>>>>
>>
> --------------------------------------------------------------------
>>
>>>>> ----
>>>>>
>>>>> Subject:
>>>>> svn commit: r495891 -
>> /ofbiz/trunk/applications/product/src/org/
>>>>>
>> ofbiz/product/feature/ProductFeatureServices.java
>>>>> From:
>>>>> jleroux@apache.org
>>>>> Date:
>>>>> Sat, 13 Jan 2007 12:48:56 -0000
>>>>> To:
>>>>> commits@ofbiz.apache.org
>>>>>
>>>>> To:
>>>>> commits@ofbiz.apache.org
>>>>>
>>>>>
>>>>> Author: jleroux
>>>>> Date: Sat Jan 13 04:48:55 2007
>>>>> New Revision: 495891
>>>>>
>>>>> URL:
>> http://svn.apache.org/viewvc?view=rev&rev=495891
>>>>> Log:
>>>>> A patch from Jonathon Wong "Prepend feature
>> idCodes with
>>>>> '-'"
>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>>>
>>>>> Modified:
>>>>>
>>
> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>
>>>>> feature/ProductFeatureServices.java
>>>>>
>>>>> Modified:
>>
> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>
>>>>> feature/ProductFeatureServices.java
>>>>> URL:
>>
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/
>>
>>>>>
>>
> product/src/org/ofbiz/product/feature/ProductFeatureServices.java?
>>
>>>>> view=diff&rev=495891&r1=495890&r2=495891
>>>>>
>>
> ====================================================================
>>
>>>>> ==========
>>>>> ---
>>
> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>
>>>>> feature/ProductFeatureServices.java (original)
>>>>> +++
>>
> ofbiz/trunk/applications/product/src/org/ofbiz/product/
>>
>>>>> feature/ProductFeatureServices.java Sat Jan 13
>> 04:48:55 2007
>>>>> @@ -227,7 +227,7 @@
>>>>>                                 List newFeatures
>> = new LinkedList();
>>>>>                                 List
>> newFeatureIds = new
>>>>> LinkedList();
>>>>>                                 if
>> (currentFeature.getString
>>>>> ("idCode") != null)
>>>>> -
>> newCombination.put
>>>>> ("defaultVariantProductId", productId +
>> currentFeature.getString
>>>>> ("idCode"));
>>>>> +
>> newCombination.put
>>>>> ("defaultVariantProductId", productId + "-" +
>>>>> currentFeature.getString("idCode"));
>>>>>                              else
>>>>>
>> newCombination.put
>>>>> ("defaultVariantProductId", productId);
>>>>>
>> newFeatures.add(currentFeature);
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Chris Howe <cj...@yahoo.com>.
I think this is best handled by reverting r495891 and
simply giving instruction for the modification for
those that wish to use it. With a move towards
granularity, we should be discouraging this type of
use as a primary key to begin with.  This auto
generation should be creating a GoodIdentification
value instead of a primary key.  If the primary key is
a surrogate key, let it truly be a surrogate and
remove the application data meaning surrounding it. 
Then keep it hidden from the user of the application. 

Once we've finished successfully hiding the
Product.productId from the user and possibly scripts
to regenerate legacy Product Entities, then set up the
delimiter as a ProductCatalog setting.


--- "David E. Jones" <jo...@hotwaxmedia.com> wrote:

> 
> Here are my previous comments about it earlier on
> the mailing list:
> 
> =========================================
> This isn't a universal policy or anything, but I'd
> say for something  
> minor like this there isn't a problem with
> hard-coding it.
> 
> The whole point of the ID generation is to make the
> IDs unique. In  
> the UI you can specify an ID instead of using the
> default, so it only  
> matters so much.
> =========================================
> 
> In general if this sort of thing were to be
> configurable, would we  
> really want it in a properties file? I'd say no,
> especially since one  
> of the new objectives that has been discussed
> recently is to make it  
> easy to configure things, make things configurable
> more granularly,  
> and make it easier to use different seed data files
> for OOTB industry- 
> specific configurations.
> 
> What to do...
> 
> -David
> 
> 
> On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:
> 
> > Hi
> >
> > I actually said that we should do it if anyone had
> objections, but  
> > no one did so that's why it went in as is.  But
> yeah if over- 
> > running the limit is a possibility then it should
> probably be  
> > changed.  Perhaps there should be some code in
> there anyway for  
> > coping with that situation, if someones running
> that many features  
> > saving 1 character might not be much of a bonus.
> >
> > Regards
> > Scott
> >
> > Jacopo Cappellato wrote:
> >> I agree with Si.
> >>
> >> Jacopo
> >>
> >> Si Chen wrote:
> >>> Hey there -
> >>>
> >>> This patch is a good idea, but I think Scott
> Gray suggested that  
> >>> this "-" could be configured in a properties
> file, and I think  
> >>> that's a good idea.  Otherwise, if you have four
> or five features  
> >>> you will easily overrun the 20-character
> productId key limit.   
> >>> Keeping it in properties file is a good way to
> allow it to be  
> >>> modified.  Otherwise it's not very nice to have
> to go into the  
> >>> code to do it.
> >>>
> >>> Jonathon, you up for doing this and sending in
> another patch?
> >>>
> >>> Si
> >>>
> >>>
>
--------------------------------------------------------------------
> 
> >>> ----
> >>>
> >>> Subject:
> >>> svn commit: r495891 -
> /ofbiz/trunk/applications/product/src/org/ 
> >>>
> ofbiz/product/feature/ProductFeatureServices.java
> >>> From:
> >>> jleroux@apache.org
> >>> Date:
> >>> Sat, 13 Jan 2007 12:48:56 -0000
> >>> To:
> >>> commits@ofbiz.apache.org
> >>>
> >>> To:
> >>> commits@ofbiz.apache.org
> >>>
> >>>
> >>> Author: jleroux
> >>> Date: Sat Jan 13 04:48:55 2007
> >>> New Revision: 495891
> >>>
> >>> URL:
> http://svn.apache.org/viewvc?view=rev&rev=495891
> >>> Log:
> >>> A patch from Jonathon Wong "Prepend feature
> idCodes with  
> >>> '-'"
> (https://issues.apache.org/jira/browse/OFBIZ-620)
> >>>
> >>> Modified:
> >>>    
>
ofbiz/trunk/applications/product/src/org/ofbiz/product/
> 
> >>> feature/ProductFeatureServices.java
> >>>
> >>> Modified:
>
ofbiz/trunk/applications/product/src/org/ofbiz/product/
> 
> >>> feature/ProductFeatureServices.java
> >>> URL:
>
http://svn.apache.org/viewvc/ofbiz/trunk/applications/
> 
> >>>
>
product/src/org/ofbiz/product/feature/ProductFeatureServices.java?
> 
> >>> view=diff&rev=495891&r1=495890&r2=495891
> >>>
>
====================================================================
> 
> >>> ==========
> >>> ---
>
ofbiz/trunk/applications/product/src/org/ofbiz/product/
> 
> >>> feature/ProductFeatureServices.java (original)
> >>> +++
>
ofbiz/trunk/applications/product/src/org/ofbiz/product/
> 
> >>> feature/ProductFeatureServices.java Sat Jan 13
> 04:48:55 2007
> >>> @@ -227,7 +227,7 @@
> >>>                                 List newFeatures
> = new LinkedList();
> >>>                                 List
> newFeatureIds = new  
> >>> LinkedList();
> >>>                                 if
> (currentFeature.getString 
> >>> ("idCode") != null)
> >>> -                               
> newCombination.put 
> >>> ("defaultVariantProductId", productId +
> currentFeature.getString 
> >>> ("idCode"));
> >>> +                               
> newCombination.put 
> >>> ("defaultVariantProductId", productId + "-" +  
> >>> currentFeature.getString("idCode"));
> >>>                              else
> >>>                                 
> newCombination.put 
> >>> ("defaultVariantProductId", productId);
> >>>                             
> newFeatures.add(currentFeature);
> >>>
> >>>
> >>
> >>
> >>
> >
> 
> 


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
Here are my previous comments about it earlier on the mailing list:

=========================================
This isn't a universal policy or anything, but I'd say for something  
minor like this there isn't a problem with hard-coding it.

The whole point of the ID generation is to make the IDs unique. In  
the UI you can specify an ID instead of using the default, so it only  
matters so much.
=========================================

In general if this sort of thing were to be configurable, would we  
really want it in a properties file? I'd say no, especially since one  
of the new objectives that has been discussed recently is to make it  
easy to configure things, make things configurable more granularly,  
and make it easier to use different seed data files for OOTB industry- 
specific configurations.

What to do...

-David


On Jan 19, 2007, at 8:59 PM, Scott Gray wrote:

> Hi
>
> I actually said that we should do it if anyone had objections, but  
> no one did so that's why it went in as is.  But yeah if over- 
> running the limit is a possibility then it should probably be  
> changed.  Perhaps there should be some code in there anyway for  
> coping with that situation, if someones running that many features  
> saving 1 character might not be much of a bonus.
>
> Regards
> Scott
>
> Jacopo Cappellato wrote:
>> I agree with Si.
>>
>> Jacopo
>>
>> Si Chen wrote:
>>> Hey there -
>>>
>>> This patch is a good idea, but I think Scott Gray suggested that  
>>> this "-" could be configured in a properties file, and I think  
>>> that's a good idea.  Otherwise, if you have four or five features  
>>> you will easily overrun the 20-character productId key limit.   
>>> Keeping it in properties file is a good way to allow it to be  
>>> modified.  Otherwise it's not very nice to have to go into the  
>>> code to do it.
>>>
>>> Jonathon, you up for doing this and sending in another patch?
>>>
>>> Si
>>>
>>> -------------------------------------------------------------------- 
>>> ----
>>>
>>> Subject:
>>> svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ 
>>> ofbiz/product/feature/ProductFeatureServices.java
>>> From:
>>> jleroux@apache.org
>>> Date:
>>> Sat, 13 Jan 2007 12:48:56 -0000
>>> To:
>>> commits@ofbiz.apache.org
>>>
>>> To:
>>> commits@ofbiz.apache.org
>>>
>>>
>>> Author: jleroux
>>> Date: Sat Jan 13 04:48:55 2007
>>> New Revision: 495891
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
>>> Log:
>>> A patch from Jonathon Wong "Prepend feature idCodes with  
>>> '-'" (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>
>>> Modified:
>>>     ofbiz/trunk/applications/product/src/org/ofbiz/product/ 
>>> feature/ProductFeatureServices.java
>>>
>>> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/product/ 
>>> feature/ProductFeatureServices.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/ 
>>> product/src/org/ofbiz/product/feature/ProductFeatureServices.java? 
>>> view=diff&rev=495891&r1=495890&r2=495891
>>> ==================================================================== 
>>> ==========
>>> --- ofbiz/trunk/applications/product/src/org/ofbiz/product/ 
>>> feature/ProductFeatureServices.java (original)
>>> +++ ofbiz/trunk/applications/product/src/org/ofbiz/product/ 
>>> feature/ProductFeatureServices.java Sat Jan 13 04:48:55 2007
>>> @@ -227,7 +227,7 @@
>>>                                 List newFeatures = new LinkedList();
>>>                                 List newFeatureIds = new  
>>> LinkedList();
>>>                                 if (currentFeature.getString 
>>> ("idCode") != null)
>>> -                                newCombination.put 
>>> ("defaultVariantProductId", productId + currentFeature.getString 
>>> ("idCode"));
>>> +                                newCombination.put 
>>> ("defaultVariantProductId", productId + "-" +  
>>> currentFeature.getString("idCode"));
>>>                              else
>>>                                  newCombination.put 
>>> ("defaultVariantProductId", productId);
>>>                              newFeatures.add(currentFeature);
>>>
>>>
>>
>>
>>
>


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Jonathon -- Improov <jo...@improov.com>.
Scott,

All theory, I believe. But I could some day decide I want to sell a product named 
SUPER­CALI­FRAGI­LISTIC­EXPI­ALI­DOCIOUS. I know, that's too long already, but you get the picture.

I'd say this is a real problem. We need to first stop user action from even reaching database. We 
then need to at least warn users that the resultant auto-generated defaultVariantProductId(s) are 
non-unique, and further tell them it's because they either have overly long base (virtual) product 
Id or too many feature idCodes.

I'll put that in my TODO ASAP list.

Which is something I've been meaning to say to us folks in OFBiz community. Error messages in 
OFBiz are probably too difficult for a hacker to decipher, let alone users. Slight exaggeration 
there, but you get picture. :)

I'm trying to start a properly written (tech docs standard) "average driver's handbook (or 
compendium if you got time)" with some "average drivers". Any interested, please let me know. 
Cryptic error messages will be "corrected" along the way.

Jonathon

Scott Gray wrote:
> Hi Jonathan
> 
> If the features are extending the product Id past 20 characters, a 
> different strategy would probably be required for auto id generation as 
> truncating the product id would most likely cause duplicate id errors.  
> I don't know how to best to approach this, but is this all theory or is 
> someone actually having problems with variant product Id lengths?
> 
> Regards
> Scott
> 
> Jonathon -- Improov wrote:
>> Scott,
>>
>> Oh hey, I think there's a "buffer overrun" bug of sorts here. The 
>> codes I submitted doesn't confine the defaultVariantProductId to a max 
>> length of 20? Not that the original codes does such a constraint. (Do 
>> we do a delegator.getEntityFieldType("Product", 
>> "id-ne").stringLength() to get max length?). Anyway, it's still pumped 
>> back into database where it's truncated to max-length.
>>
>> Still, it'll be good for the QuickAddVariants.ftl javascript to 
>> display only defaultVariantProductId(s) of valid lengths.
>>
>> Spin-off thought here. Currently, the defaultVariantProductId is of a 
>> simplistic form <productId><separator><featureIdCodes>. Would anyone 
>> want to further split up the <featureIdCodes>? I don't need to. If 
>> anyone needs that, let me know.
>>
>> Jonathon
>>
>> Scott Gray wrote:
>>> Hi
>>>
>>> I actually said that we should do it if anyone had objections, but no 
>>> one did so that's why it went in as is.  But yeah if over-running the 
>>> limit is a possibility then it should probably be changed.  Perhaps 
>>> there should be some code in there anyway for coping with that 
>>> situation, if someones running that many features saving 1 character 
>>> might not be much of a bonus.
>>>
>>> Regards
>>> Scott
>>>
>>> Jacopo Cappellato wrote:
>>>> I agree with Si.
>>>>
>>>> Jacopo
>>>>
>>>> Si Chen wrote:
>>>>> Hey there -
>>>>>
>>>>> This patch is a good idea, but I think Scott Gray suggested that 
>>>>> this "-" could be configured in a properties file, and I think 
>>>>> that's a good idea.  Otherwise, if you have four or five features 
>>>>> you will easily overrun the 20-character productId key limit.  
>>>>> Keeping it in properties file is a good way to allow it to be 
>>>>> modified.  Otherwise it's not very nice to have to go into the code 
>>>>> to do it.
>>>>>
>>>>> Jonathon, you up for doing this and sending in another patch?
>>>>>
>>>>> Si
>>>>>
>>>>> ------------------------------------------------------------------------ 
>>>>>
>>>>>
>>>>> Subject:
>>>>> svn commit: r495891 - 
>>>>> /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>>
>>>>> From:
>>>>> jleroux@apache.org
>>>>> Date:
>>>>> Sat, 13 Jan 2007 12:48:56 -0000
>>>>> To:
>>>>> commits@ofbiz.apache.org
>>>>>
>>>>> To:
>>>>> commits@ofbiz.apache.org
>>>>>
>>>>>
>>>>> Author: jleroux
>>>>> Date: Sat Jan 13 04:48:55 2007
>>>>> New Revision: 495891
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
>>>>> Log:
>>>>> A patch from Jonathon Wong "Prepend feature idCodes with '-'" 
>>>>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>>>
>>>>> Modified:
>>>>>     
>>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>>
>>>>>
>>>>> Modified: 
>>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>>
>>>>> URL: 
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java?view=diff&rev=495891&r1=495890&r2=495891 
>>>>>
>>>>> ============================================================================== 
>>>>>
>>>>> --- 
>>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>> (original)
>>>>> +++ 
>>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>> Sat Jan 13 04:48:55 2007
>>>>> @@ -227,7 +227,7 @@
>>>>>                                 List newFeatures = new LinkedList();
>>>>>                                 List newFeatureIds = new LinkedList();
>>>>>                                 if 
>>>>> (currentFeature.getString("idCode") != null)
>>>>> -                                
>>>>> newCombination.put("defaultVariantProductId", productId + 
>>>>> currentFeature.getString("idCode"));
>>>>> +                                
>>>>> newCombination.put("defaultVariantProductId", productId + "-" + 
>>>>> currentFeature.getString("idCode"));
>>>>>                              else
>>>>>                                  
>>>>> newCombination.put("defaultVariantProductId", productId);
>>>>>                              newFeatures.add(currentFeature);
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
> 
> 


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Scott Gray <le...@gmail.com>.
Hi Jonathan

If the features are extending the product Id past 20 characters, a 
different strategy would probably be required for auto id generation as 
truncating the product id would most likely cause duplicate id errors.  
I don't know how to best to approach this, but is this all theory or is 
someone actually having problems with variant product Id lengths?

Regards
Scott

Jonathon -- Improov wrote:
> Scott,
>
> Oh hey, I think there's a "buffer overrun" bug of sorts here. The 
> codes I submitted doesn't confine the defaultVariantProductId to a max 
> length of 20? Not that the original codes does such a constraint. (Do 
> we do a delegator.getEntityFieldType("Product", 
> "id-ne").stringLength() to get max length?). Anyway, it's still pumped 
> back into database where it's truncated to max-length.
>
> Still, it'll be good for the QuickAddVariants.ftl javascript to 
> display only defaultVariantProductId(s) of valid lengths.
>
> Spin-off thought here. Currently, the defaultVariantProductId is of a 
> simplistic form <productId><separator><featureIdCodes>. Would anyone 
> want to further split up the <featureIdCodes>? I don't need to. If 
> anyone needs that, let me know.
>
> Jonathon
>
> Scott Gray wrote:
>> Hi
>>
>> I actually said that we should do it if anyone had objections, but no 
>> one did so that's why it went in as is.  But yeah if over-running the 
>> limit is a possibility then it should probably be changed.  Perhaps 
>> there should be some code in there anyway for coping with that 
>> situation, if someones running that many features saving 1 character 
>> might not be much of a bonus.
>>
>> Regards
>> Scott
>>
>> Jacopo Cappellato wrote:
>>> I agree with Si.
>>>
>>> Jacopo
>>>
>>> Si Chen wrote:
>>>> Hey there -
>>>>
>>>> This patch is a good idea, but I think Scott Gray suggested that 
>>>> this "-" could be configured in a properties file, and I think 
>>>> that's a good idea.  Otherwise, if you have four or five features 
>>>> you will easily overrun the 20-character productId key limit.  
>>>> Keeping it in properties file is a good way to allow it to be 
>>>> modified.  Otherwise it's not very nice to have to go into the code 
>>>> to do it.
>>>>
>>>> Jonathon, you up for doing this and sending in another patch?
>>>>
>>>> Si
>>>>
>>>> ------------------------------------------------------------------------ 
>>>>
>>>>
>>>> Subject:
>>>> svn commit: r495891 - 
>>>> /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>
>>>> From:
>>>> jleroux@apache.org
>>>> Date:
>>>> Sat, 13 Jan 2007 12:48:56 -0000
>>>> To:
>>>> commits@ofbiz.apache.org
>>>>
>>>> To:
>>>> commits@ofbiz.apache.org
>>>>
>>>>
>>>> Author: jleroux
>>>> Date: Sat Jan 13 04:48:55 2007
>>>> New Revision: 495891
>>>>
>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
>>>> Log:
>>>> A patch from Jonathon Wong "Prepend feature idCodes with '-'" 
>>>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>>
>>>> Modified:
>>>>     
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>
>>>>
>>>> Modified: 
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>>
>>>> URL: 
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java?view=diff&rev=495891&r1=495890&r2=495891 
>>>>
>>>> ============================================================================== 
>>>>
>>>> --- 
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>> (original)
>>>> +++ 
>>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>> Sat Jan 13 04:48:55 2007
>>>> @@ -227,7 +227,7 @@
>>>>                                 List newFeatures = new LinkedList();
>>>>                                 List newFeatureIds = new LinkedList();
>>>>                                 if 
>>>> (currentFeature.getString("idCode") != null)
>>>> -                                
>>>> newCombination.put("defaultVariantProductId", productId + 
>>>> currentFeature.getString("idCode"));
>>>> +                                
>>>> newCombination.put("defaultVariantProductId", productId + "-" + 
>>>> currentFeature.getString("idCode"));
>>>>                              else
>>>>                                  
>>>> newCombination.put("defaultVariantProductId", productId);
>>>>                              newFeatures.add(currentFeature);
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Jonathon -- Improov <jo...@improov.com>.
Scott,

Oh hey, I think there's a "buffer overrun" bug of sorts here. The codes I submitted doesn't 
confine the defaultVariantProductId to a max length of 20? Not that the original codes does such a 
constraint. (Do we do a delegator.getEntityFieldType("Product", "id-ne").stringLength() to get max 
length?). Anyway, it's still pumped back into database where it's truncated to max-length.

Still, it'll be good for the QuickAddVariants.ftl javascript to display only 
defaultVariantProductId(s) of valid lengths.

Spin-off thought here. Currently, the defaultVariantProductId is of a simplistic form 
<productId><separator><featureIdCodes>. Would anyone want to further split up the 
<featureIdCodes>? I don't need to. If anyone needs that, let me know.

Jonathon

Scott Gray wrote:
> Hi
> 
> I actually said that we should do it if anyone had objections, but no 
> one did so that's why it went in as is.  But yeah if over-running the 
> limit is a possibility then it should probably be changed.  Perhaps 
> there should be some code in there anyway for coping with that 
> situation, if someones running that many features saving 1 character 
> might not be much of a bonus.
> 
> Regards
> Scott
> 
> Jacopo Cappellato wrote:
>> I agree with Si.
>>
>> Jacopo
>>
>> Si Chen wrote:
>>> Hey there -
>>>
>>> This patch is a good idea, but I think Scott Gray suggested that this 
>>> "-" could be configured in a properties file, and I think that's a 
>>> good idea.  Otherwise, if you have four or five features you will 
>>> easily overrun the 20-character productId key limit.  Keeping it in 
>>> properties file is a good way to allow it to be modified.  Otherwise 
>>> it's not very nice to have to go into the code to do it.
>>>
>>> Jonathon, you up for doing this and sending in another patch?
>>>
>>> Si
>>>
>>> ------------------------------------------------------------------------
>>>
>>> Subject:
>>> svn commit: r495891 - 
>>> /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>
>>> From:
>>> jleroux@apache.org
>>> Date:
>>> Sat, 13 Jan 2007 12:48:56 -0000
>>> To:
>>> commits@ofbiz.apache.org
>>>
>>> To:
>>> commits@ofbiz.apache.org
>>>
>>>
>>> Author: jleroux
>>> Date: Sat Jan 13 04:48:55 2007
>>> New Revision: 495891
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
>>> Log:
>>> A patch from Jonathon Wong "Prepend feature idCodes with '-'" 
>>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>>
>>> Modified:
>>>     
>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>
>>>
>>> Modified: 
>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>>
>>> URL: 
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java?view=diff&rev=495891&r1=495890&r2=495891 
>>>
>>> ============================================================================== 
>>>
>>> --- 
>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>> (original)
>>> +++ 
>>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>> Sat Jan 13 04:48:55 2007
>>> @@ -227,7 +227,7 @@
>>>                                 List newFeatures = new LinkedList();
>>>                                 List newFeatureIds = new LinkedList();
>>>                                 if 
>>> (currentFeature.getString("idCode") != null)
>>> -                                
>>> newCombination.put("defaultVariantProductId", productId + 
>>> currentFeature.getString("idCode"));
>>> +                                
>>> newCombination.put("defaultVariantProductId", productId + "-" + 
>>> currentFeature.getString("idCode"));
>>>                              else
>>>                                  
>>> newCombination.put("defaultVariantProductId", productId);
>>>                              newFeatures.add(currentFeature);
>>>
>>>
>>
>>
>>
> 
> 


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Scott Gray <le...@gmail.com>.
Hi

I actually said that we should do it if anyone had objections, but no 
one did so that's why it went in as is.  But yeah if over-running the 
limit is a possibility then it should probably be changed.  Perhaps 
there should be some code in there anyway for coping with that 
situation, if someones running that many features saving 1 character 
might not be much of a bonus.

Regards
Scott

Jacopo Cappellato wrote:
> I agree with Si.
>
> Jacopo
>
> Si Chen wrote:
>> Hey there -
>>
>> This patch is a good idea, but I think Scott Gray suggested that this 
>> "-" could be configured in a properties file, and I think that's a 
>> good idea.  Otherwise, if you have four or five features you will 
>> easily overrun the 20-character productId key limit.  Keeping it in 
>> properties file is a good way to allow it to be modified.  Otherwise 
>> it's not very nice to have to go into the code to do it.
>>
>> Jonathon, you up for doing this and sending in another patch?
>>
>> Si
>>
>> ------------------------------------------------------------------------
>>
>> Subject:
>> svn commit: r495891 - 
>> /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>
>> From:
>> jleroux@apache.org
>> Date:
>> Sat, 13 Jan 2007 12:48:56 -0000
>> To:
>> commits@ofbiz.apache.org
>>
>> To:
>> commits@ofbiz.apache.org
>>
>>
>> Author: jleroux
>> Date: Sat Jan 13 04:48:55 2007
>> New Revision: 495891
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
>> Log:
>> A patch from Jonathon Wong "Prepend feature idCodes with '-'" 
>> (https://issues.apache.org/jira/browse/OFBIZ-620)
>>
>> Modified:
>>     
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>
>>
>> Modified: 
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>>
>> URL: 
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java?view=diff&rev=495891&r1=495890&r2=495891 
>>
>> ============================================================================== 
>>
>> --- 
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>> (original)
>> +++ 
>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java 
>> Sat Jan 13 04:48:55 2007
>> @@ -227,7 +227,7 @@
>>                                 List newFeatures = new LinkedList();
>>                                 List newFeatureIds = new LinkedList();
>>                                 if 
>> (currentFeature.getString("idCode") != null)
>> -                                
>> newCombination.put("defaultVariantProductId", productId + 
>> currentFeature.getString("idCode"));
>> +                                
>> newCombination.put("defaultVariantProductId", productId + "-" + 
>> currentFeature.getString("idCode"));
>>                              else
>>                                  
>> newCombination.put("defaultVariantProductId", productId);
>>                              newFeatures.add(currentFeature);
>>
>>
>
>
>


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Jacopo Cappellato <ti...@sastau.it>.
I agree with Si.

Jacopo

Si Chen wrote:
> Hey there -
> 
> This patch is a good idea, but I think Scott Gray suggested that this 
> "-" could be configured in a properties file, and I think that's a good 
> idea.  Otherwise, if you have four or five features you will easily 
> overrun the 20-character productId key limit.  Keeping it in properties 
> file is a good way to allow it to be modified.  Otherwise it's not very 
> nice to have to go into the code to do it.
> 
> Jonathon, you up for doing this and sending in another patch?
> 
> Si
> 
> ------------------------------------------------------------------------
> 
> Subject:
> svn commit: r495891 - 
> /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java
> From:
> jleroux@apache.org
> Date:
> Sat, 13 Jan 2007 12:48:56 -0000
> To:
> commits@ofbiz.apache.org
> 
> To:
> commits@ofbiz.apache.org
> 
> 
> Author: jleroux
> Date: Sat Jan 13 04:48:55 2007
> New Revision: 495891
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=495891
> Log:
> A patch from Jonathon Wong "Prepend feature idCodes with '-'" (https://issues.apache.org/jira/browse/OFBIZ-620)
> 
> Modified:
>     ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java
> 
> Modified: ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java?view=diff&rev=495891&r1=495890&r2=495891
> ==============================================================================
> --- ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java (original)
> +++ ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java Sat Jan 13 04:48:55 2007
> @@ -227,7 +227,7 @@
>                                 List newFeatures = new LinkedList();
>                                 List newFeatureIds = new LinkedList();
>                                 if (currentFeature.getString("idCode") != null)
> -                                newCombination.put("defaultVariantProductId", productId + currentFeature.getString("idCode"));
> +                                newCombination.put("defaultVariantProductId", productId + "-" + currentFeature.getString("idCode"));
>                              else
>                                  newCombination.put("defaultVariantProductId", productId);
>                              newFeatures.add(currentFeature);
> 
> 



Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Jacques Le Roux <ja...@les7arts.com>.
+1

Jacques

----- Original Message ----- 
From: "David E. Jones" <jo...@hotwaxmedia.com>
To: <de...@ofbiz.apache.org>
Cc: "Tom Anderson" <to...@spaceagecontrol.com>
Sent: Saturday, January 20, 2007 8:15 AM
Subject: Re: [Fwd: svn commit: r495891 -
/ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]


>
> Maybe, as a compromise, to avoid the properties file issue you could
> just put an extra field in the UI for the character(s) to use to
> separate the base ID from the feature idCode(s).
>
> -David
>
>
> On Jan 20, 2007, at 12:04 AM, Jonathon -- Improov wrote:
>
> > Si,
> >
> > > This patch is a good idea, but I think Scott Gray suggested that
> > this
> > > "-" could be configured in a properties file, and I think that's
> > a good
> > > idea.  Otherwise, if you have four or five features you will easily
> > > overrun the 20-character productId key limit.  Keeping it in
> > properties
> > > file is a good way to allow it to be modified.  Otherwise it's
> > not very
> > > nice to have to go into the code to do it.
> >
> > David said it's ok to hardcode it for now. I was arguing, but of
> > course it struck me that it wouldn't be difficult to
> > "customization" the hardcoding it folks needed to place say '--' or
> > '*' instead of '-'.
> >
> > I was of the idea that we put a field on the QuickAddVariants page
> > to allow users to specify the separator ('-' or '*' or 'whatever').
> >
> > I was thinking we have '-' as the default. Then we allow config
> > properties to override this default.
> >
> > > Jonathon, you up for doing this and sending in another patch?
> >
> > Ok, sure. Which config file and what config param name you want it in?
> >
> > Jonathon
> >
> > Si Chen wrote:
> >> Hey there -
> >> This patch is a good idea, but I think Scott Gray suggested that
> >> this "-" could be configured in a properties file, and I think
> >> that's a good idea.  Otherwise, if you have four or five features
> >> you will easily overrun the 20-character productId key limit.
> >> Keeping it in properties file is a good way to allow it to be
> >> modified.  Otherwise it's not very nice to have to go into the
> >> code to do it.
> >> Jonathon, you up for doing this and sending in another patch?
> >> Si
> >> --------------------------------------------------------------------- 
> >> ---
> >> No virus found in this incoming message.
> >> Checked by AVG Free Edition.
> >> Version: 7.5.432 / Virus Database: 268.17.0/639 - Release Date:
> >> 1/18/2007 6:47 PM
> >
>
>


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
Maybe, as a compromise, to avoid the properties file issue you could  
just put an extra field in the UI for the character(s) to use to  
separate the base ID from the feature idCode(s).

-David


On Jan 20, 2007, at 12:04 AM, Jonathon -- Improov wrote:

> Si,
>
> > This patch is a good idea, but I think Scott Gray suggested that  
> this
> > "-" could be configured in a properties file, and I think that's  
> a good
> > idea.  Otherwise, if you have four or five features you will easily
> > overrun the 20-character productId key limit.  Keeping it in  
> properties
> > file is a good way to allow it to be modified.  Otherwise it's  
> not very
> > nice to have to go into the code to do it.
>
> David said it's ok to hardcode it for now. I was arguing, but of  
> course it struck me that it wouldn't be difficult to  
> "customization" the hardcoding it folks needed to place say '--' or  
> '*' instead of '-'.
>
> I was of the idea that we put a field on the QuickAddVariants page  
> to allow users to specify the separator ('-' or '*' or 'whatever').
>
> I was thinking we have '-' as the default. Then we allow config  
> properties to override this default.
>
> > Jonathon, you up for doing this and sending in another patch?
>
> Ok, sure. Which config file and what config param name you want it in?
>
> Jonathon
>
> Si Chen wrote:
>> Hey there -
>> This patch is a good idea, but I think Scott Gray suggested that  
>> this "-" could be configured in a properties file, and I think  
>> that's a good idea.  Otherwise, if you have four or five features  
>> you will easily overrun the 20-character productId key limit.   
>> Keeping it in properties file is a good way to allow it to be  
>> modified.  Otherwise it's not very nice to have to go into the  
>> code to do it.
>> Jonathon, you up for doing this and sending in another patch?
>> Si
>> --------------------------------------------------------------------- 
>> ---
>> No virus found in this incoming message.
>> Checked by AVG Free Edition.
>> Version: 7.5.432 / Virus Database: 268.17.0/639 - Release Date:  
>> 1/18/2007 6:47 PM
>


Re: [Fwd: svn commit: r495891 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java]

Posted by Jonathon -- Improov <jo...@improov.com>.
Si,

 > This patch is a good idea, but I think Scott Gray suggested that this
 > "-" could be configured in a properties file, and I think that's a good
 > idea.  Otherwise, if you have four or five features you will easily
 > overrun the 20-character productId key limit.  Keeping it in properties
 > file is a good way to allow it to be modified.  Otherwise it's not very
 > nice to have to go into the code to do it.

David said it's ok to hardcode it for now. I was arguing, but of course it struck me that it 
wouldn't be difficult to "customization" the hardcoding it folks needed to place say '--' or '*' 
instead of '-'.

I was of the idea that we put a field on the QuickAddVariants page to allow users to specify the 
separator ('-' or '*' or 'whatever').

I was thinking we have '-' as the default. Then we allow config properties to override this default.

 > Jonathon, you up for doing this and sending in another patch?

Ok, sure. Which config file and what config param name you want it in?

Jonathon

Si Chen wrote:
> Hey there -
> 
> This patch is a good idea, but I think Scott Gray suggested that this 
> "-" could be configured in a properties file, and I think that's a good 
> idea.  Otherwise, if you have four or five features you will easily 
> overrun the 20-character productId key limit.  Keeping it in properties 
> file is a good way to allow it to be modified.  Otherwise it's not very 
> nice to have to go into the code to do it.
> 
> Jonathon, you up for doing this and sending in another patch?
> 
> Si
> 
> 
> ------------------------------------------------------------------------
> 
> No virus found in this incoming message.
> Checked by AVG Free Edition.
> Version: 7.5.432 / Virus Database: 268.17.0/639 - Release Date: 1/18/2007 6:47 PM