From: "Adrian Crum" <adrian.c...@sandglass-software.com>
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
Of course this is supposed to be a DB using OFBiz data model as it is so far,
else all this discussion is pointless.
* If the existing DB has not this field it's not concerned by this change.-
* If it has it, then this change will take it into account, nothing to worry
about .-
The user will not have to worry about a procedure like those in
https://cwiki.apache.org/confluence/display/OFBTECH/Revisions+Requiring+Data+Migration+%28upgrade+ofbiz%29.
S/he just have to
upgrade the code and run load-demo or whatever s/he needs to load or not.
Nicolas's idea is to provide a fallback for users who want to continue to use
the previous data model. But also a mean for users who
could prefer the new way to make their choice. With the fallback, users might even ignore this change, it will continue to work
silently.
It even seems to me that you are contradicting yourself.
1st you say
Wrong. An existing database has data in the serviceName field, not in the
oldServiceName field.
then
That is even worse - how do you know every database has that column name.
"that column name" must mean "serviceName field", and seems contradictory to me, hu? Did you review my commit AND Nicolas's 2nd
patch at https://issues.apache.org/jira/browse/OFBIZ-5020 which must be applied under my commit. Since he preferred this way than to
provide a way to move data, as I asked him in my Jira comment about my commit. BTW at this moment I did not remember he has no
access to this wiki page. Not a pb, this can be either a comment in the page or in the Jira that I can put in the wiki page.
I don't mind reverting this commit, I have nothing to win or lose. I just
believe this way of doing is not fair, and not respectful
to Nicolas's work and how he tried to exchange with us (OFBiz developers, and
OFBiz team - aka committers)
Adrian, you could at least answer to him, even if did not use a perfect
english. He proposed another way, the classical one,
something like in the wiki page I referred to above.
I really wonder what others think about it?
Is the way Nicolas proposed not good or worse than what has been done so far in the the wiki page. To me, it's a smarter automated
way.
If we don't want to apply the 2nd patch under the 1st, then we need to revert
my commit indeed...
Finally, this commit night seem not important. But it's just about the way we handle contributions. And I don't think we
(committers) are doing a good job at it presently. We are neglecting too much what contributors bring to us. I understand it's
easier/safer to put your own things in the repo, but we should try to have more consideration for contributors work. I tried to do
so far, not always with success, and I thank you and others who helped me sometimes, but at least I tried my best and did not gave
up when I was right. And nobody can't say some good happened from these efforts.
Hope I'm clear, and not too long (I tried to avoid that but eventually needed
to clarify my POV)
Jacques
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/
