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

[David E. Jones]

-David


On Jan 19, 2007, at 10:31 PM, Chris Howe 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.

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);

smime.p7s
Description: S/MIME cryptographic signature