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

[Jonathon -- Improov]

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" <[EMAIL PROTECTED]> 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:
[EMAIL PROTECTED]
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);