Re: svn commit: r695268 - /ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r695268 - /ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml

Jacopo Cappellato-4
Jacques,

I am a bit confused.... why should the "admin" user be hardcoded into  
a service?

Jacopo

On Sep 14, 2008, at 9:20 PM, [hidden email] wrote:

> Author: jleroux
> Date: Sun Sep 14 12:20:12 2008
> New Revision: 695268
>
> URL: http://svn.apache.org/viewvc?rev=695268&view=rev
> Log:
> A modified patch from Patrick Antivackis "Create a party  
> relationship between a new customer and the product store in the  
> eCommerce application" (https://issues.apache.org/jira/browse/OFBIZ-1955 
> ) - OFBIZ-1955
> I modified since we need a party to relate not a store, hence admin  
> as SALES_REP
>
> Modified:
>    ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/
> customer/CustomerEvents.xml
>
> Modified: ofbiz/trunk/applications/ecommerce/script/org/ofbiz/
> ecommerce/customer/CustomerEvents.xml
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml?rev=695268&r1=695267&r2=695268&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/
> customer/CustomerEvents.xml (original)
> +++ ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/
> customer/CustomerEvents.xml Sun Sep 14 12:20:12 2008
> @@ -307,6 +307,13 @@
>         <create-value value-name="partyDataSource"/>
>         <call-service service-name="createPartyRole" in-map-
> name="partyRoleContext" include-user-login="true"/>
>
> +        <!--  Create party relationship between the new customer  
> and the product store sales representative -->
> +        <set field="partyRelationshipCtx.partyIdTo" value="admin"/>
> +        <set field="partyRelationshipCtx.roleTypeIdTo"  
> value="SALES_REP"/>
> +        <set field="partyRelationshipCtx.roleTypeIdFrom"  
> value="CUSTOMER"/>
> +        <set field="partyRelationshipCtx.partyRelationshipTypeId"  
> value="CUSTOMER_REL"/>
> +        <call-service service-name="createPartyRelationship" in-map-
> name="partyRelationshipCtx"/>
> +
>         <!-- shipping address -->
>         <if-compare field="parameters.USE_ADDRESS" operator="equals"  
> value="false">
>             <!-- address not used, do nothing -->
> @@ -1368,4 +1375,4 @@
>             <store-value value-name="loggedInUser"/>
>         </if-compare>
>     </simple-method>
> -</simple-methods>
> \ No newline at end of file
> +</simple-methods>
>
>


smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r695268 - /ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml

David E Jones

Sorry I didn't get to this sooner. I flagged this issue as one needing  
more review when I saw the notice come through but I haven't had a  
chance to actually look at it until now.

Aside from the problem Jacopo mentioned, why would we EVER want to  
have this functionality in OOTB, generic OFBiz? This appears to have  
come from a very specific end-user requirement that is not mentioned  
here or in the issue and I can't guess at why anyone would want this,  
even in their custom system (even if the Party to associate with was  
configurable and not just "admin").

In other words, this should be reverted and the Jira issue should  
probably be changed to rejected. This is way to specific for generic  
OFBiz, and even if it wasn't the reason for doing it is really unclear  
and actually seems to be a bad thing to do by default OOTB.

-David


On Sep 14, 2008, at 1:31 PM, Jacopo Cappellato wrote:

> Jacques,
>
> I am a bit confused.... why should the "admin" user be hardcoded  
> into a service?
>
> Jacopo
>
> On Sep 14, 2008, at 9:20 PM, [hidden email] wrote:
>
>> Author: jleroux
>> Date: Sun Sep 14 12:20:12 2008
>> New Revision: 695268
>>
>> URL: http://svn.apache.org/viewvc?rev=695268&view=rev
>> Log:
>> A modified patch from Patrick Antivackis "Create a party  
>> relationship between a new customer and the product store in the  
>> eCommerce application" (https://issues.apache.org/jira/browse/OFBIZ-1955 
>> ) - OFBIZ-1955
>> I modified since we need a party to relate not a store, hence admin  
>> as SALES_REP
>>
>> Modified:
>>   ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/
>> customer/CustomerEvents.xml
>>
>> Modified: ofbiz/trunk/applications/ecommerce/script/org/ofbiz/
>> ecommerce/customer/CustomerEvents.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml?rev=695268&r1=695267&r2=695268&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/
>> customer/CustomerEvents.xml (original)
>> +++ ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/
>> customer/CustomerEvents.xml Sun Sep 14 12:20:12 2008
>> @@ -307,6 +307,13 @@
>>        <create-value value-name="partyDataSource"/>
>>        <call-service service-name="createPartyRole" in-map-
>> name="partyRoleContext" include-user-login="true"/>
>>
>> +        <!--  Create party relationship between the new customer  
>> and the product store sales representative -->
>> +        <set field="partyRelationshipCtx.partyIdTo" value="admin"/>
>> +        <set field="partyRelationshipCtx.roleTypeIdTo"  
>> value="SALES_REP"/>
>> +        <set field="partyRelationshipCtx.roleTypeIdFrom"  
>> value="CUSTOMER"/>
>> +        <set field="partyRelationshipCtx.partyRelationshipTypeId"  
>> value="CUSTOMER_REL"/>
>> +        <call-service service-name="createPartyRelationship" in-
>> map-name="partyRelationshipCtx"/>
>> +
>>        <!-- shipping address -->
>>        <if-compare field="parameters.USE_ADDRESS" operator="equals"  
>> value="false">
>>            <!-- address not used, do nothing -->
>> @@ -1368,4 +1375,4 @@
>>            <store-value value-name="loggedInUser"/>
>>        </if-compare>
>>    </simple-method>
>> -</simple-methods>
>> \ No newline at end of file
>> +</simple-methods>
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r695268 - /ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml

Jacques Le Roux
Administrator
In reply to this post by Jacopo Cappellato-4
Jacopo,

Yes you are right I should have used ProductStoreRole to retrieve the SALES_REP of the store. I will do if we do not reject all the
patch as David seems to be inclined...

Thanks

Jacques

From: "Jacopo Cappellato" <[hidden email]>

> Jacques,
>
> I am a bit confused.... why should the "admin" user be hardcoded into  a service?
>
> Jacopo
>
> On Sep 14, 2008, at 9:20 PM, [hidden email] wrote:
>
>> Author: jleroux
>> Date: Sun Sep 14 12:20:12 2008
>> New Revision: 695268
>>
>> URL: http://svn.apache.org/viewvc?rev=695268&view=rev
>> Log:
>> A modified patch from Patrick Antivackis "Create a party  relationship between a new customer and the product store in the
>> eCommerce application" (https://issues.apache.org/jira/browse/OFBIZ-1955 ) - OFBIZ-1955
>> I modified since we need a party to relate not a store, hence admin  as SALES_REP
>>
>> Modified:
>>    ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/ customer/CustomerEvents.xml
>>
>> Modified: ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ ecommerce/customer/CustomerEvents.xml
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml?rev=695268&r1=695267&r2=695268&view=diff
>> = = = = = = = = ======================================================================
>> --- ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/ customer/CustomerEvents.xml (original)
>> +++ ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/ customer/CustomerEvents.xml Sun Sep 14 12:20:12 2008
>> @@ -307,6 +307,13 @@
>>         <create-value value-name="partyDataSource"/>
>>         <call-service service-name="createPartyRole" in-map- name="partyRoleContext" include-user-login="true"/>
>>
>> +        <!--  Create party relationship between the new customer  and the product store sales representative -->
>> +        <set field="partyRelationshipCtx.partyIdTo" value="admin"/>
>> +        <set field="partyRelationshipCtx.roleTypeIdTo"  value="SALES_REP"/>
>> +        <set field="partyRelationshipCtx.roleTypeIdFrom"  value="CUSTOMER"/>
>> +        <set field="partyRelationshipCtx.partyRelationshipTypeId"  value="CUSTOMER_REL"/>
>> +        <call-service service-name="createPartyRelationship" in-map- name="partyRelationshipCtx"/>
>> +
>>         <!-- shipping address -->
>>         <if-compare field="parameters.USE_ADDRESS" operator="equals"  value="false">
>>             <!-- address not used, do nothing -->
>> @@ -1368,4 +1375,4 @@
>>             <store-value value-name="loggedInUser"/>
>>         </if-compare>
>>     </simple-method>
>> -</simple-methods>
>> \ No newline at end of file
>> +</simple-methods>
>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r695268 - /ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml

Jacques Le Roux
Administrator
In reply to this post by David E Jones

David,

I just answered to Jacopo and suggested to use ProducStoreRole to retrieve the sales representative (SALES_REP).

I thouhgt it could be good to create a such relation between the sales representative and customers as an example and I can"t see
really why it's harmful. But I'm sure you have good reasons for that. Could you explain please ? For instance why it"s too generic
and where (or how) to put the frontier between acceptable OOTB and "too generic" ?

Please note that in a second patch I have reverted the relationship and explained why.

Thanks

Jacques

From: "David E Jones" <[hidden email]>

>
> Sorry I didn't get to this sooner. I flagged this issue as one needing  more review when I saw the notice come through but I
> haven't had a  chance to actually look at it until now.
>
> Aside from the problem Jacopo mentioned, why would we EVER want to  have this functionality in OOTB, generic OFBiz? This appears
> to have  come from a very specific end-user requirement that is not mentioned  here or in the issue and I can't guess at why
> anyone would want this,  even in their custom system (even if the Party to associate with was  configurable and not just "admin").
>
> In other words, this should be reverted and the Jira issue should  probably be changed to rejected. This is way to specific for
> generic  OFBiz, and even if it wasn't the reason for doing it is really unclear  and actually seems to be a bad thing to do by
> default OOTB.
>
> -David
>
>
> On Sep 14, 2008, at 1:31 PM, Jacopo Cappellato wrote:
>
>> Jacques,
>>
>> I am a bit confused.... why should the "admin" user be hardcoded  into a service?
>>
>> Jacopo
>>
>> On Sep 14, 2008, at 9:20 PM, [hidden email] wrote:
>>
>>> Author: jleroux
>>> Date: Sun Sep 14 12:20:12 2008
>>> New Revision: 695268
>>>
>>> URL: http://svn.apache.org/viewvc?rev=695268&view=rev
>>> Log:
>>> A modified patch from Patrick Antivackis "Create a party  relationship between a new customer and the product store in the
>>> eCommerce application" (https://issues.apache.org/jira/browse/OFBIZ-1955 ) - OFBIZ-1955
>>> I modified since we need a party to relate not a store, hence admin  as SALES_REP
>>>
>>> Modified:
>>>   ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/ customer/CustomerEvents.xml
>>>
>>> Modified: ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ ecommerce/customer/CustomerEvents.xml
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml?rev=695268&r1=695267&r2=695268&view=diff
>>> = = = = = = = = = =====================================================================
>>> --- ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/ customer/CustomerEvents.xml (original)
>>> +++ ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/ customer/CustomerEvents.xml Sun Sep 14 12:20:12 2008
>>> @@ -307,6 +307,13 @@
>>>        <create-value value-name="partyDataSource"/>
>>>        <call-service service-name="createPartyRole" in-map- name="partyRoleContext" include-user-login="true"/>
>>>
>>> +        <!--  Create party relationship between the new customer  and the product store sales representative -->
>>> +        <set field="partyRelationshipCtx.partyIdTo" value="admin"/>
>>> +        <set field="partyRelationshipCtx.roleTypeIdTo"  value="SALES_REP"/>
>>> +        <set field="partyRelationshipCtx.roleTypeIdFrom"  value="CUSTOMER"/>
>>> +        <set field="partyRelationshipCtx.partyRelationshipTypeId"  value="CUSTOMER_REL"/>
>>> +        <call-service service-name="createPartyRelationship" in- map-name="partyRelationshipCtx"/>
>>> +
>>>        <!-- shipping address -->
>>>        <if-compare field="parameters.USE_ADDRESS" operator="equals"  value="false">
>>>            <!-- address not used, do nothing -->
>>> @@ -1368,4 +1375,4 @@
>>>            <store-value value-name="loggedInUser"/>
>>>        </if-compare>
>>>    </simple-method>
>>> -</simple-methods>
>>> \ No newline at end of file
>>> +</simple-methods>
>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r695268 - /ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml

Jacopo Cappellato-4
In reply to this post by Jacques Le Roux
Maybe the best option is to revert the commit, discuss and improve the  
patch (in Jira) and then possibly recommit it.

Cheers,

Jacopo

On Sep 14, 2008, at 10:24 PM, Jacques Le Roux wrote:

> Jacopo,
>
> Yes you are right I should have used ProductStoreRole to retrieve  
> the SALES_REP of the store. I will do if we do not reject all the  
> patch as David seems to be inclined...
>
> Thanks
>
> Jacques
>
> From: "Jacopo Cappellato" <[hidden email]>
>> Jacques,
>>
>> I am a bit confused.... why should the "admin" user be hardcoded  
>> into  a service?
>>
>> Jacopo
>>
>> On Sep 14, 2008, at 9:20 PM, [hidden email] wrote:
>>
>>> Author: jleroux
>>> Date: Sun Sep 14 12:20:12 2008
>>> New Revision: 695268
>>>
>>> URL: http://svn.apache.org/viewvc?rev=695268&view=rev
>>> Log:
>>> A modified patch from Patrick Antivackis "Create a party  
>>> relationship between a new customer and the product store in the  
>>> eCommerce application" (https://issues.apache.org/jira/browse/OFBIZ-1955 
>>>  ) - OFBIZ-1955
>>> I modified since we need a party to relate not a store, hence  
>>> admin  as SALES_REP
>>>
>>> Modified:
>>>   ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/  
>>> customer/CustomerEvents.xml
>>>
>>> Modified: ofbiz/trunk/applications/ecommerce/script/org/ofbiz/  
>>> ecommerce/customer/CustomerEvents.xml
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml?rev=695268&r1=695267&r2=695268&view=diff
>>> = = = = = = = =  
>>> =
>>> =
>>> ====================================================================
>>> --- ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/  
>>> customer/CustomerEvents.xml (original)
>>> +++ ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/  
>>> customer/CustomerEvents.xml Sun Sep 14 12:20:12 2008
>>> @@ -307,6 +307,13 @@
>>>        <create-value value-name="partyDataSource"/>
>>>        <call-service service-name="createPartyRole" in-map-  
>>> name="partyRoleContext" include-user-login="true"/>
>>>
>>> +        <!--  Create party relationship between the new customer  
>>> and the product store sales representative -->
>>> +        <set field="partyRelationshipCtx.partyIdTo" value="admin"/>
>>> +        <set field="partyRelationshipCtx.roleTypeIdTo"  
>>> value="SALES_REP"/>
>>> +        <set field="partyRelationshipCtx.roleTypeIdFrom"  
>>> value="CUSTOMER"/>
>>> +        <set  
>>> field="partyRelationshipCtx.partyRelationshipTypeId"  
>>> value="CUSTOMER_REL"/>
>>> +        <call-service service-name="createPartyRelationship" in-
>>> map- name="partyRelationshipCtx"/>
>>> +
>>>        <!-- shipping address -->
>>>        <if-compare field="parameters.USE_ADDRESS"  
>>> operator="equals"  value="false">
>>>            <!-- address not used, do nothing -->
>>> @@ -1368,4 +1375,4 @@
>>>            <store-value value-name="loggedInUser"/>
>>>        </if-compare>
>>>    </simple-method>
>>> -</simple-methods>
>>> \ No newline at end of file
>>> +</simple-methods>
>>>
>>>
>>
>


smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r695268 - /ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml

Jacques Le Roux
Administrator
OK Jacopo,

I reverted in revision 695286. Ready for discussion, I'm certainly missing something here since  I still can't see the problem (if I
use ProductStoreRole of course and not admin harcoded)

Jacques

From: "Jacopo Cappellato" <[hidden email]>

> Maybe the best option is to revert the commit, discuss and improve the  patch (in Jira) and then possibly recommit it.
>
> Cheers,
>
> Jacopo
>
> On Sep 14, 2008, at 10:24 PM, Jacques Le Roux wrote:
>
>> Jacopo,
>>
>> Yes you are right I should have used ProductStoreRole to retrieve  the SALES_REP of the store. I will do if we do not reject all
>> the  patch as David seems to be inclined...
>>
>> Thanks
>>
>> Jacques
>>
>> From: "Jacopo Cappellato" <[hidden email]>
>>> Jacques,
>>>
>>> I am a bit confused.... why should the "admin" user be hardcoded  into  a service?
>>>
>>> Jacopo
>>>
>>> On Sep 14, 2008, at 9:20 PM, [hidden email] wrote:
>>>
>>>> Author: jleroux
>>>> Date: Sun Sep 14 12:20:12 2008
>>>> New Revision: 695268
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=695268&view=rev
>>>> Log:
>>>> A modified patch from Patrick Antivackis "Create a party   relationship between a new customer and the product store in the
>>>> eCommerce application" (https://issues.apache.org/jira/browse/OFBIZ-1955 ) - OFBIZ-1955
>>>> I modified since we need a party to relate not a store, hence  admin  as SALES_REP
>>>>
>>>> Modified:
>>>>   ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/  customer/CustomerEvents.xml
>>>>
>>>> Modified: ofbiz/trunk/applications/ecommerce/script/org/ofbiz/  ecommerce/customer/CustomerEvents.xml
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml?rev=695268&r1=695267&r2=695268&view=diff
>>>> = = = = = = = =  = = ====================================================================
>>>> --- ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/  customer/CustomerEvents.xml (original)
>>>> +++ ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/  customer/CustomerEvents.xml Sun Sep 14 12:20:12 2008
>>>> @@ -307,6 +307,13 @@
>>>>        <create-value value-name="partyDataSource"/>
>>>>        <call-service service-name="createPartyRole" in-map-  name="partyRoleContext" include-user-login="true"/>
>>>>
>>>> +        <!--  Create party relationship between the new customer   and the product store sales representative -->
>>>> +        <set field="partyRelationshipCtx.partyIdTo" value="admin"/>
>>>> +        <set field="partyRelationshipCtx.roleTypeIdTo"   value="SALES_REP"/>
>>>> +        <set field="partyRelationshipCtx.roleTypeIdFrom"   value="CUSTOMER"/>
>>>> +        <set  field="partyRelationshipCtx.partyRelationshipTypeId"   value="CUSTOMER_REL"/>
>>>> +        <call-service service-name="createPartyRelationship" in- map- name="partyRelationshipCtx"/>
>>>> +
>>>>        <!-- shipping address -->
>>>>        <if-compare field="parameters.USE_ADDRESS"  operator="equals"  value="false">
>>>>            <!-- address not used, do nothing -->
>>>> @@ -1368,4 +1375,4 @@
>>>>            <store-value value-name="loggedInUser"/>
>>>>        </if-compare>
>>>>    </simple-method>
>>>> -</simple-methods>
>>>> \ No newline at end of file
>>>> +</simple-methods>
>>>>
>>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Generic versus Specific and Requirements (was Re: svn commit: r695268 - /ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml)

David E Jones
In reply to this post by Jacques Le Roux

It's not too "generic", it's too "specific". In other words, it's not  
generic enough.

There is nothing in the issue to describe why we would want this  
relationship, no discussion of the "requirement" in other words, and  
this doesn't seem like anything useful in any way as OOTB  
functionality. It does harm in that it makes a bunch of seemingly  
meaningless PartyRelationship records that will take up space and make  
it hard to distinguish from more meaningful information.

The patch as-is has issues anyway, and really without knowing what  
we're trying to accomplish all those are is symptoms of bad design  
because we don't know the requirement.

It doesn't even make sense to say let's use this or that relationship  
type or look for this or that role, because there is no definition of  
what we're trying to accomplish or enable, and based on the code and  
the stuff in the Jira issue I can't figure out what that requirement  
might be (ie it's weird and seems to improve nothing).

-David


On Sep 14, 2008, at 2:31 PM, Jacques Le Roux wrote:

>
> David,
>
> I just answered to Jacopo and suggested to use ProducStoreRole to  
> retrieve the sales representative (SALES_REP).
>
> I thouhgt it could be good to create a such relation between the  
> sales representative and customers as an example and I can"t see  
> really why it's harmful. But I'm sure you have good reasons for  
> that. Could you explain please ? For instance why it"s too generic  
> and where (or how) to put the frontier between acceptable OOTB and  
> "too generic" ?
>
> Please note that in a second patch I have reverted the relationship  
> and explained why.
>
> Thanks
>
> Jacques
>
> From: "David E Jones" <[hidden email]>
>>
>> Sorry I didn't get to this sooner. I flagged this issue as one  
>> needing  more review when I saw the notice come through but I  
>> haven't had a  chance to actually look at it until now.
>>
>> Aside from the problem Jacopo mentioned, why would we EVER want to  
>> have this functionality in OOTB, generic OFBiz? This appears to  
>> have  come from a very specific end-user requirement that is not  
>> mentioned  here or in the issue and I can't guess at why anyone  
>> would want this,  even in their custom system (even if the Party to  
>> associate with was  configurable and not just "admin").
>>
>> In other words, this should be reverted and the Jira issue should  
>> probably be changed to rejected. This is way to specific for  
>> generic  OFBiz, and even if it wasn't the reason for doing it is  
>> really unclear  and actually seems to be a bad thing to do by  
>> default OOTB.
>>
>> -David
>>
>>
>> On Sep 14, 2008, at 1:31 PM, Jacopo Cappellato wrote:
>>
>>> Jacques,
>>>
>>> I am a bit confused.... why should the "admin" user be hardcoded  
>>> into a service?
>>>
>>> Jacopo
>>>
>>> On Sep 14, 2008, at 9:20 PM, [hidden email] wrote:
>>>
>>>> Author: jleroux
>>>> Date: Sun Sep 14 12:20:12 2008
>>>> New Revision: 695268
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=695268&view=rev
>>>> Log:
>>>> A modified patch from Patrick Antivackis "Create a party  
>>>> relationship between a new customer and the product store in the  
>>>> eCommerce application" (https://issues.apache.org/jira/browse/OFBIZ-1955 
>>>>  ) - OFBIZ-1955
>>>> I modified since we need a party to relate not a store, hence  
>>>> admin  as SALES_REP
>>>>
>>>> Modified:
>>>>  ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/  
>>>> customer/CustomerEvents.xml
>>>>
>>>> Modified: ofbiz/trunk/applications/ecommerce/script/org/ofbiz/  
>>>> ecommerce/customer/CustomerEvents.xml
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml?rev=695268&r1=695267&r2=695268&view=diff
>>>> = = = = = = = = =  
>>>> =
>>>> =
>>>> ===================================================================
>>>> --- ofbiz/trunk/applications/ecommerce/script/org/ofbiz/
>>>> ecommerce/ customer/CustomerEvents.xml (original)
>>>> +++ ofbiz/trunk/applications/ecommerce/script/org/ofbiz/
>>>> ecommerce/ customer/CustomerEvents.xml Sun Sep 14 12:20:12 2008
>>>> @@ -307,6 +307,13 @@
>>>>       <create-value value-name="partyDataSource"/>
>>>>       <call-service service-name="createPartyRole" in-map-  
>>>> name="partyRoleContext" include-user-login="true"/>
>>>>
>>>> +        <!--  Create party relationship between the new  
>>>> customer  and the product store sales representative -->
>>>> +        <set field="partyRelationshipCtx.partyIdTo"  
>>>> value="admin"/>
>>>> +        <set field="partyRelationshipCtx.roleTypeIdTo"  
>>>> value="SALES_REP"/>
>>>> +        <set field="partyRelationshipCtx.roleTypeIdFrom"  
>>>> value="CUSTOMER"/>
>>>> +        <set  
>>>> field="partyRelationshipCtx.partyRelationshipTypeId"  
>>>> value="CUSTOMER_REL"/>
>>>> +        <call-service service-name="createPartyRelationship" in-  
>>>> map-name="partyRelationshipCtx"/>
>>>> +
>>>>       <!-- shipping address -->
>>>>       <if-compare field="parameters.USE_ADDRESS"  
>>>> operator="equals"  value="false">
>>>>           <!-- address not used, do nothing -->
>>>> @@ -1368,4 +1375,4 @@
>>>>           <store-value value-name="loggedInUser"/>
>>>>       </if-compare>
>>>>   </simple-method>
>>>> -</simple-methods>
>>>> \ No newline at end of file
>>>> +</simple-methods>
>>>>
>>>>
>>>
>

Reply | Threaded
Open this post in threaded view
|

Re: Generic versus Specific and Requirements (was Re: svn commit: r695268 - /ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml)

Jacques Le Roux
Administrator
Yes too specific of course. Hopefully, Patrick will give us some information regarding his requirements. What do you think about the
comparaison with CreateEmploye in my last message ?

Jacques

From: "David E Jones" <[hidden email]>

>
> It's not too "generic", it's too "specific". In other words, it's not  generic enough.
>
> There is nothing in the issue to describe why we would want this  relationship, no discussion of the "requirement" in other words,
> and  this doesn't seem like anything useful in any way as OOTB  functionality. It does harm in that it makes a bunch of seemingly
> meaningless PartyRelationship records that will take up space and make  it hard to distinguish from more meaningful information.
>
> The patch as-is has issues anyway, and really without knowing what  we're trying to accomplish all those are is symptoms of bad
> design  because we don't know the requirement.
>
> It doesn't even make sense to say let's use this or that relationship  type or look for this or that role, because there is no
> definition of  what we're trying to accomplish or enable, and based on the code and  the stuff in the Jira issue I can't figure
> out what that requirement  might be (ie it's weird and seems to improve nothing).
>
> -David
>
>
> On Sep 14, 2008, at 2:31 PM, Jacques Le Roux wrote:
>
>>
>> David,
>>
>> I just answered to Jacopo and suggested to use ProducStoreRole to  retrieve the sales representative (SALES_REP).
>>
>> I thouhgt it could be good to create a such relation between the  sales representative and customers as an example and I can"t
>> see  really why it's harmful. But I'm sure you have good reasons for  that. Could you explain please ? For instance why it"s too
>> generic  and where (or how) to put the frontier between acceptable OOTB and  "too generic" ?
>>
>> Please note that in a second patch I have reverted the relationship  and explained why.
>>
>> Thanks
>>
>> Jacques
>>
>> From: "David E Jones" <[hidden email]>
>>>
>>> Sorry I didn't get to this sooner. I flagged this issue as one  needing  more review when I saw the notice come through but I
>>> haven't had a  chance to actually look at it until now.
>>>
>>> Aside from the problem Jacopo mentioned, why would we EVER want to   have this functionality in OOTB, generic OFBiz? This
>>> appears to  have  come from a very specific end-user requirement that is not  mentioned  here or in the issue and I can't guess
>>> at why anyone  would want this,  even in their custom system (even if the Party to  associate with was  configurable and not
>>> just "admin").
>>>
>>> In other words, this should be reverted and the Jira issue should   probably be changed to rejected. This is way to specific for
>>> generic  OFBiz, and even if it wasn't the reason for doing it is  really unclear  and actually seems to be a bad thing to do by
>>> default OOTB.
>>>
>>> -David
>>>
>>>
>>> On Sep 14, 2008, at 1:31 PM, Jacopo Cappellato wrote:
>>>
>>>> Jacques,
>>>>
>>>> I am a bit confused.... why should the "admin" user be hardcoded   into a service?
>>>>
>>>> Jacopo
>>>>
>>>> On Sep 14, 2008, at 9:20 PM, [hidden email] wrote:
>>>>
>>>>> Author: jleroux
>>>>> Date: Sun Sep 14 12:20:12 2008
>>>>> New Revision: 695268
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=695268&view=rev
>>>>> Log:
>>>>> A modified patch from Patrick Antivackis "Create a party   relationship between a new customer and the product store in the
>>>>> eCommerce application" (https://issues.apache.org/jira/browse/OFBIZ-1955 ) - OFBIZ-1955
>>>>> I modified since we need a party to relate not a store, hence  admin  as SALES_REP
>>>>>
>>>>> Modified:
>>>>>  ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/  customer/CustomerEvents.xml
>>>>>
>>>>> Modified: ofbiz/trunk/applications/ecommerce/script/org/ofbiz/  ecommerce/customer/CustomerEvents.xml
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml?rev=695268&r1=695267&r2=695268&view=diff
>>>>> = = = = = = = = =  = = ===================================================================
>>>>> --- ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ ecommerce/ customer/CustomerEvents.xml (original)
>>>>> +++ ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ ecommerce/ customer/CustomerEvents.xml Sun Sep 14 12:20:12 2008
>>>>> @@ -307,6 +307,13 @@
>>>>>       <create-value value-name="partyDataSource"/>
>>>>>       <call-service service-name="createPartyRole" in-map-  name="partyRoleContext" include-user-login="true"/>
>>>>>
>>>>> +        <!--  Create party relationship between the new  customer  and the product store sales representative -->
>>>>> +        <set field="partyRelationshipCtx.partyIdTo"  value="admin"/>
>>>>> +        <set field="partyRelationshipCtx.roleTypeIdTo"   value="SALES_REP"/>
>>>>> +        <set field="partyRelationshipCtx.roleTypeIdFrom"   value="CUSTOMER"/>
>>>>> +        <set  field="partyRelationshipCtx.partyRelationshipTypeId"   value="CUSTOMER_REL"/>
>>>>> +        <call-service service-name="createPartyRelationship" in-  map-name="partyRelationshipCtx"/>
>>>>> +
>>>>>       <!-- shipping address -->
>>>>>       <if-compare field="parameters.USE_ADDRESS"  operator="equals"  value="false">
>>>>>           <!-- address not used, do nothing -->
>>>>> @@ -1368,4 +1375,4 @@
>>>>>           <store-value value-name="loggedInUser"/>
>>>>>       </if-compare>
>>>>>   </simple-method>
>>>>> -</simple-methods>
>>>>> \ No newline at end of file
>>>>> +</simple-methods>
>>>>>
>>>>>
>>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r695268 - /ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml

Patrick Antivackis
In reply to this post by Jacques Le Roux
Hello,
I updated Jira
https://issues.apache.org/jira/browse/OFBIZ-1955
with the following :

Indeed there is an error in my patch,it should be :
<set field="partyRelationshipCtx.partyIdTo"
from-field="productStore.payToPartyId"/>
instead of
<set field="partyRelationshipCtx.partyIdTo"
from-field="productStore.productStoreId"/>

The aim of this patch is to know to which productStore Company a customer
create an account in.

Today, the only information available after new account are :
PartyProfile set to CUSTOMER for the new account partyId
PartyDatasource set to ECOMMERCE_SITE for the partyId

May be my patch is too light, and in fact a relationship should be created
between the account and the productStore. It's of course open to discussion.


Patrick


2008/9/14 Jacques Le Roux <[hidden email]>

> OK Jacopo,
>
> I reverted in revision 695286. Ready for discussion, I'm certainly missing
> something here since  I still can't see the problem (if I use
> ProductStoreRole of course and not admin harcoded)
>
>
> Jacques
>
> From: "Jacopo Cappellato" <[hidden email]>
>
>> Maybe the best option is to revert the commit, discuss and improve the
>>  patch (in Jira) and then possibly recommit it.
>>
>> Cheers,
>>
>> Jacopo
>>
>> On Sep 14, 2008, at 10:24 PM, Jacques Le Roux wrote:
>>
>>  Jacopo,
>>>
>>> Yes you are right I should have used ProductStoreRole to retrieve  the
>>> SALES_REP of the store. I will do if we do not reject all the  patch as
>>> David seems to be inclined...
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>> From: "Jacopo Cappellato" <[hidden email]>
>>>
>>>> Jacques,
>>>>
>>>> I am a bit confused.... why should the "admin" user be hardcoded  into
>>>>  a service?
>>>>
>>>> Jacopo
>>>>
>>>> On Sep 14, 2008, at 9:20 PM, [hidden email] wrote:
>>>>
>>>>  Author: jleroux
>>>>> Date: Sun Sep 14 12:20:12 2008
>>>>> New Revision: 695268
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=695268&view=rev
>>>>> Log:
>>>>> A modified patch from Patrick Antivackis "Create a party   relationship
>>>>> between a new customer and the product store in the eCommerce application" (
>>>>> https://issues.apache.org/jira/browse/OFBIZ-1955 ) - OFBIZ-1955
>>>>> I modified since we need a party to relate not a store, hence  admin
>>>>>  as SALES_REP
>>>>>
>>>>> Modified:
>>>>>  ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/
>>>>>  customer/CustomerEvents.xml
>>>>>
>>>>> Modified: ofbiz/trunk/applications/ecommerce/script/org/ofbiz/
>>>>>  ecommerce/customer/CustomerEvents.xml
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/customer/CustomerEvents.xml?rev=695268&r1=695267&r2=695268&view=diff
>>>>> = = = = = = = =  = =
>>>>> ====================================================================
>>>>> --- ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/
>>>>>  customer/CustomerEvents.xml (original)
>>>>> +++ ofbiz/trunk/applications/ecommerce/script/org/ofbiz/ecommerce/
>>>>>  customer/CustomerEvents.xml Sun Sep 14 12:20:12 2008
>>>>> @@ -307,6 +307,13 @@
>>>>>       <create-value value-name="partyDataSource"/>
>>>>>       <call-service service-name="createPartyRole" in-map-
>>>>>  name="partyRoleContext" include-user-login="true"/>
>>>>>
>>>>> +        <!--  Create party relationship between the new customer   and
>>>>> the product store sales representative -->
>>>>> +        <set field="partyRelationshipCtx.partyIdTo" value="admin"/>
>>>>> +        <set field="partyRelationshipCtx.roleTypeIdTo"
>>>>> value="SALES_REP"/>
>>>>> +        <set field="partyRelationshipCtx.roleTypeIdFrom"
>>>>> value="CUSTOMER"/>
>>>>> +        <set  field="partyRelationshipCtx.partyRelationshipTypeId"
>>>>> value="CUSTOMER_REL"/>
>>>>> +        <call-service service-name="createPartyRelationship" in- map-
>>>>> name="partyRelationshipCtx"/>
>>>>> +
>>>>>       <!-- shipping address -->
>>>>>       <if-compare field="parameters.USE_ADDRESS"  operator="equals"
>>>>>  value="false">
>>>>>           <!-- address not used, do nothing -->
>>>>> @@ -1368,4 +1375,4 @@
>>>>>           <store-value value-name="loggedInUser"/>
>>>>>       </if-compare>
>>>>>   </simple-method>
>>>>> -</simple-methods>
>>>>> \ No newline at end of file
>>>>> +</simple-methods>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>>
>