That is even worse - how do you know every database has that column name.
I'm done discussing this - here is my veto:
-1
Please revert this and think about what you are doing.
-Adrian
On 8/31/2012 5:53 PM, Jacques Le Roux wrote:
From: "Adrian Crum" <adrian.c...@sandglass-software.com>
Wrong. An existing database has data in the serviceName field, not in
the oldServiceName field.
Not wrong: by using col-name="service_name" you allow the entity
engine to use this DB field, still named service_name, to be used as
if it was oldServiceName
Hence this change in Nicolas's 2nd patch is needed!
- String fulfillmentService = (String)
content.get("serviceName");
+ // external service fulfillment
+ String fulfillmentService = (String)
content.get("oldServiceName"); //keep for backward compatibility
Jacques
-Adrian
On 8/31/2012 4:57 PM, Jacques Le Roux wrote:
Yes, the point is Nicolas is using the col-name="service_name" trick in
+ <field name="oldServiceName" type="long-varchar"
col-name="service_name">
+ <description>Deprecated : use customMethod pattern
instead of. Keep for backward compatibility</description>
+ </field>
So no migration tool is needed, it's done automatically. What others
think, should Nicolas really provide a migration script?
Jacques
From: "Nicolas Malin" <malin.nico...@librenberry.net>
Le 31/08/2012 15:18, Adrian Crum a écrit :
I think you are missing the point. Documenting the change is fine,
but there should be a migration path. This commit just ignores
existing data. In other words, the commit should include a service
that copies data in the deleted field to the new structure.
Sorry adrian, I'm not agree. This commit has been validate on
existing data with template content fusion without data migration.
I have spot the backward compatibility.
[Quote]
On my initial proposition, I keep serviceName on oldServiceName for
transparent evolution without script migration. An other solution
will be convert all serviceName present on Content to customMethod
data by conversion script.
If you prefer a complete migration (exclude serviceName from
services), I can correct it today.
[/Quote]
I open to all suggestion. I reopen the issue to correct the
fulfillDigitalItems order process raised by Scott, I wait the
run-tests end.
Nicolas
And no, if there is a problem with your commit, YOU should revert it.
-Adrian
On 8/31/2012 1:24 PM, Jacques Le Roux wrote:
You can revert if you want, I thought documenting in wiki would
be enough as we did before
Jacques
From: "Adrian Crum" <adrian.c...@sandglass-software.com>
The problem with this commit is it hurts existing users because
their data is being ignored (or thrown away).
-Adrian
On 8/31/2012 12:49 PM, Jacques Le Roux wrote:
Yes, I wondered about using both also. I asked Nicolas to
clarify at
https://cwiki.apache.org/confluence/display/OFBTECH/Revisions+Requiring+Data+Migration+%28upgrade+ofbiz%29
It would be really easy to have both and could then simply
documented by a sentence.
But do we want to keep the old way? The new way allow more...
Nicolas asked opinions at
http://markmail.org/message/uqbvio76dulkzvnd BJ suggested both...
Jacques
From: "Adrian Crum" <adrian.c...@sandglass-software.com>
Why can't we use both? What happens to users who are using the
serviceName field? Is there a migration service?
-Adrian
On 8/31/2012 10:55 AM, jler...@apache.org wrote:
Author: jleroux
Date: Fri Aug 31 09:55:34 2012
New Revision: 1379389
URL: http://svn.apache.org/viewvc?rev=1379389&view=rev
Log:
A patch from Nicolas Malin "change serviceName by
customMethod on Content "
https://issues.apache.org/jira/browse/OFBIZ-5020
When you used a content as template, the content.serviceName
value used to call the context populate service before
rendering.
I propose to replace serviceName field by customMethodId and
use customMethod pattern for more flexibility.
serviceName field is renamed to oldServiceName field for
backward compatibility
Modified:
ofbiz/trunk/applications/content/entitydef/entitymodel.xml
ofbiz/trunk/applications/content/script/org/ofbiz/content/ContentManagementMapProcessors.xml
ofbiz/trunk/applications/content/src/org/ofbiz/content/content/ContentWorker.java
Modified:
ofbiz/trunk/applications/content/entitydef/entitymodel.xml
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/content/entitydef/entitymodel.xml?rev=1379389&r1=1379388&r2=1379389&view=diff
==============================================================================
---
ofbiz/trunk/applications/content/entitydef/entitymodel.xml
(original)
+++
ofbiz/trunk/applications/content/entitydef/entitymodel.xml
Fri Aug 31 09:55:34 2012
@@ -159,7 +159,10 @@ under the License.
<field name="dataSourceId" type="id"></field>
<field name="statusId" type="id"></field>
<field name="privilegeEnumId" type="id"></field>
- <field name="serviceName" type="long-varchar"></field>
+ <field name="oldServiceName" type="long-varchar"
col-name="service_name">
+ <description>Deprecated : use customMethod pattern
instead of. Keep for backward compatibility</description>
+ </field>
+ <field name="customMethodId" type="id"></field>
<field name="contentName" type="name"></field>
<field name="description" type="description"></field>
<field name="localeString" type="very-short"></field>
@@ -187,6 +190,9 @@ under the License.
<relation type="one" fk-name="CONTENT_PRIVENM"
title="Privilege" rel-entity-name="Enumeration">
<key-map field-name="privilegeEnumId"
rel-field-name="enumId"/>
</relation>
+ <relation type="one" fk-name="CONTENT_CUSTMET"
rel-entity-name="CustomMethod">
+ <key-map field-name="customMethodId"/>
+ </relation>
<!-- the relationship to MimeType is one-nofk so that
you can still do a lookup on MimeType but a new
and unexpected mime type would not cause a foreign
key constraint violation, so MimeType can store the
most common mime types instead of an exhaustive list
of all possible mime types -->
Modified:
ofbiz/trunk/applications/content/script/org/ofbiz/content/ContentManagementMapProcessors.xml
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/content/script/org/ofbiz/content/ContentManagementMapProcessors.xml?rev=1379389&r1=1379388&r2=1379389&view=diff
==============================================================================
---
ofbiz/trunk/applications/content/script/org/ofbiz/content/ContentManagementMapProcessors.xml
(original)
+++
ofbiz/trunk/applications/content/script/org/ofbiz/content/ContentManagementMapProcessors.xml
Fri Aug 31 09:55:34 2012
@@ -27,7 +27,8 @@ under the License.
<process field="dataResourceId"><copy
to-field="dataResourceId" replace="true"
set-if-null="false"/></process>
<process field="dataSourceId"><copy
to-field="dataSourceId" replace="true"
set-if-null="false"/></process>
<process field="statusId"><copy to-field="statusId"
replace="true" set-if-null="false"/></process>
- <process field="serviceName"><copy
to-field="serviceName" replace="true"
set-if-null="false"/></process>
+ <process field="customMethodId"><copy
to-field="customMethodId" replace="true"
set-if-null="false"/></process>
+ <process field="oldServiceName"><copy
to-field="oldServiceName" replace="true"
set-if-null="false"/></process>
<process field="contentName"><copy
to-field="contentName" replace="true"
set-if-null="false"/></process>
<process field="description"><copy
to-field="description" replace="true"
set-if-null="false"/></process>
<process field="localeString"><copy
to-field="localeString" replace="true"
set-if-null="false"/></process>
@@ -112,7 +113,8 @@ under the License.
<process field="dataResourceId"><copy
to-field="dataResourceId" replace="true"
set-if-null="false"/></process>
<process field="dataSourceId"><copy
to-field="dataSourceId" replace="true"
set-if-null="false"/></process>
<process field="statusId"><copy to-field="statusId"
replace="true" set-if-null="false"/></process>
- <process field="serviceName"><copy
to-field="serviceName" replace="true"
set-if-null="false"/></process>
+ <process field="customMethodId"><copy
to-field="customMethodId" replace="true"
set-if-null="false"/></process>
+ <process field="oldServiceName"><copy
to-field="oldServiceName" replace="true"
set-if-null="false"/></process>
<process field="contentName"><copy
to-field="contentName" replace="true"
set-if-null="false"/></process>
<process field="description"><copy
to-field="description" replace="true"
set-if-null="false"/></process>
<process field="localeString"><copy
to-field="localeString" replace="true"
set-if-null="false"/></process>
Modified:
ofbiz/trunk/applications/content/src/org/ofbiz/content/content/ContentWorker.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/content/src/org/ofbiz/content/content/ContentWorker.java?rev=1379389&r1=1379388&r2=1379389&view=diff
==============================================================================
---
ofbiz/trunk/applications/content/src/org/ofbiz/content/content/ContentWorker.java
(original)
+++
ofbiz/trunk/applications/content/src/org/ofbiz/content/content/ContentWorker.java
Fri Aug 31 09:55:34 2012
@@ -177,7 +177,10 @@ public class ContentWorker implements or
Map<String,Object>templateContext, Locale locale, String
mimeTypeId, boolean cache, List<GenericValue> webAnalytics)
throws GeneralException, IOException {
// if the content has a service attached run the
service
- String serviceName =
content.getString("serviceName");
+ //search serviceName to call on associate
customMethod and if empty get value from old serviceName field
+ String serviceName =
content.getString("oldServiceName");
+ GenericValue custMethod =
content.getRelatedOne("CustomMethod", true);
+ if (custMethod != null) serviceName =
custMethod.getString("customMethodName");
if (dispatcher != null &&
UtilValidate.isNotEmpty(serviceName)) {
DispatchContext dctx =
dispatcher.getDispatchContext();
ModelService service =
dctx.getModelService(serviceName);
--
Nicolas MALIN
Consultant
Tél : 06.17.66.40.06
Site projet : http://www.neogia.org/
-------
Société LibrenBerry
Tél : 02.48.02.56.12
Site : http://www.librenberry.net/