Service returning incorrect records

[Suraj Khurana]

Hello,

While using service *getPartyFromUserLogin *and it is not returning
desired results. While digging more into the issue, I found that the
service is using LIKE operator instead of returning exact records. Is it
intentional or a bug?

IMO, this is not correct and it should return exactly similar records for a
particular party.

Moving further, why it is a service? IMO, it should be a helper method.

Please let me know your thoughts about this.

--
Thanks and Regards,
*Suraj Khurana* | Sr. Enterprise Software Engineer
*HotWax Commerce* by  *HotWax Systems*
Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010

[Michael Brohl-3]

Hi Suraj,

this service seems not to be used anywhere in the system and the code
looks... ämm..., strange.

I'd propose to remove it from the codebase along with other resources
like the UI labels referenced there (if not used anywhere else).

I think it is worth to have a deeper look at the whole class.

Thanks and regards,

Michael


Am 14.11.17 um 12:09 schrieb Suraj Khurana:

> Hello,
>
> While using service *getPartyFromUserLogin *and it is not returning
> desired results. While digging more into the issue, I found that the
> service is using LIKE operator instead of returning exact records. Is it
> intentional or a bug?
>
> IMO, this is not correct and it should return exactly similar records for a
> particular party.
>
> Moving further, why it is a service? IMO, it should be a helper method.
>
> Please let me know your thoughts about this.
>
> --
> Thanks and Regards,
> *Suraj Khurana* | Sr. Enterprise Software Engineer
> *HotWax Commerce* by  *HotWax Systems*
> Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010
>

[Jacques Le Roux]

Hi Suraj,

There is an unequivocal relation from a userLoginId to a partyId.  A Party may have several userLoginIds but the reserve is not true.

Maybe this is intended to be called with only a part of the userLoginId String? You might then get several parties and refine...

So the question is: what was the desired result? Based on which userLoginId? What was the surprise? (I guess I answered later, please continue to read :))

This is pre apache era code, with some no functional modifications since.

The same applies to getPartyFromEmail and more services there.

Ah, I found that we had already a similar question from Ean Schuessler long ago (10 years!) http://markmail.org/message/fadkiyby4azqj3bj

I'd review the whole class as Michael suggested and also suggest to get the same way than I did 10 years ago: simply rename the service and related
methods to have them make sense.

Like getPartiesFromPartOfUserloginId, same idea for getPartyFromEmail to clarify more than in http://svn.apache.org/viewvc?view=revision&revision=443465

Also check/change all services in this class which use a LIKE: getPartyFromPerson, getPartyFromPartyGroup

I see no reasons to drop those services if some find them useful. I get they do since someone wrote it, even with a misleading name. Not having them
used OOTB is not a sufficient indication of removal.

Jacques


Le 14/11/2017 à 15:21, Michael Brohl a écrit :

> Hi Suraj,
>
> this service seems not to be used anywhere in the system and the code looks... ämm..., strange.
>
> I'd propose to remove it from the codebase along with other resources like the UI labels referenced there (if not used anywhere else).
>
> I think it is worth to have a deeper look at the whole class.
>
> Thanks and regards,
>
> Michael
>
>
> Am 14.11.17 um 12:09 schrieb Suraj Khurana:
>> Hello,
>>
>> While using service *getPartyFromUserLogin *and it is not returning
>> desired results. While digging more into the issue, I found that the
>> service is using LIKE operator instead of returning exact records. Is it
>> intentional or a bug?
>>
>> IMO, this is not correct and it should return exactly similar records for a
>> particular party.
>>
>> Moving further, why it is a service? IMO, it should be a helper method.
>>
>> Please let me know your thoughts about this.
>>
>> --
>> Thanks and Regards,
>> *Suraj Khurana* | Sr. Enterprise Software Engineer
>> *HotWax Commerce* by  *HotWax Systems*
>> Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010
>>
>
>

[Michael Brohl-3]

Am 14.11.17 um 19:15 schrieb Jacques Le Roux:
>
> I see no reasons to drop those services if some find them useful. I
> get they do since someone wrote it, even with a misleading name. Not
> having them used OOTB is not a sufficient indication of removal.
I do not agree. This functionality is not used since a decade and the
code is in bad shape.

I think it makes much more sense to put effort in the many other fields
of action we have in this project.

Michael

[Jacques Le Roux]

Le 14/11/2017 à 21:39, Michael Brohl a écrit :

>
> Am 14.11.17 um 19:15 schrieb Jacques Le Roux:
>>
>> I see no reasons to drop those services if some find them useful. I get they do since someone wrote it, even with a misleading name. Not having
>> them used OOTB is not a sufficient indication of removal.
> I do not agree. This functionality is not used since a decade and the code is in bad shape.
>
> I think it makes much more sense to put effort in the many other fields of action we have in this project.
>
> Michael
>
>
>

I can take care of renaming the methods, don't worry

Jacques

[Suraj Khurana]

Thanks, Jacques and Michael for your inputs,

I agree with both the points, either such services should be removed as
they are not being used and not written properly, or refactoring of the
whole class with proper names as per their functionality to justify their
name.

I am willing to participate in this effort if we are more intended towards
refactoring. Please let me know how we are proceeding in this.

I have created a Jira here
<https://issues.apache.org/jira/browse/OFBIZ-9982>.

--
Thanks and Regards,
*Suraj Khurana* | Sr. Enterprise Software Engineer
*HotWax Commerce*  by  *HotWax Systems*
Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010


On Wed, Nov 15, 2017 at 2:54 AM, Jacques Le Roux <
[hidden email]> wrote:

> Le 14/11/2017 à 21:39, Michael Brohl a écrit :
>
>>
>> Am 14.11.17 um 19:15 schrieb Jacques Le Roux:
>>
>>>
>>> I see no reasons to drop those services if some find them useful. I get
>>> they do since someone wrote it, even with a misleading name. Not having
>>> them used OOTB is not a sufficient indication of removal.
>>>
>> I do not agree. This functionality is not used since a decade and the
>> code is in bad shape.
>>
>> I think it makes much more sense to put effort in the many other fields
>> of action we have in this project.
>>
>> Michael
>>
>>
>>
>> I can take care of renaming the methods, don't worry
>
> Jacques
>
>

[Scott Gray-3]

I'm in favor of removing, it's been there since at least 2003 and is most
likely from a legacy search screen that has since been superseded.

Regards
Scott

On 15 November 2017 at 21:14, Suraj Khurana <[hidden email]
> wrote:

> Thanks, Jacques and Michael for your inputs,
>
> I agree with both the points, either such services should be removed as
> they are not being used and not written properly, or refactoring of the
> whole class with proper names as per their functionality to justify their
> name.
>
> I am willing to participate in this effort if we are more intended towards
> refactoring. Please let me know how we are proceeding in this.
>
> I have created a Jira here
> <https://issues.apache.org/jira/browse/OFBIZ-9982>.
>
> --
> Thanks and Regards,
> *Suraj Khurana* | Sr. Enterprise Software Engineer
> *HotWax Commerce*  by  *HotWax Systems*
> Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010
>
>
> On Wed, Nov 15, 2017 at 2:54 AM, Jacques Le Roux <
> [hidden email]> wrote:
>
> > Le 14/11/2017 à 21:39, Michael Brohl a écrit :
> >
> >>
> >> Am 14.11.17 um 19:15 schrieb Jacques Le Roux:
> >>
> >>>
> >>> I see no reasons to drop those services if some find them useful. I get
> >>> they do since someone wrote it, even with a misleading name. Not having
> >>> them used OOTB is not a sufficient indication of removal.
> >>>
> >> I do not agree. This functionality is not used since a decade and the
> >> code is in bad shape.
> >>
> >> I think it makes much more sense to put effort in the many other fields
> >> of action we have in this project.
> >>
> >> Michael
> >>
> >>
> >>
> >> I can take care of renaming the methods, don't worry
> >
> > Jacques
> >
> >
>

[Pawan Verma]

+1 for removing. I also see no use of this in the current system.

--
Thanks and Regards,

*Pawan Verma*

[Jacques Le Roux]

Thanks Suraj,

I'll take care of the renaming. At 1st glance I don't see a need for other refactorings, I let you decide on this...

Jacques


Le 15/11/2017 à 09:14, Suraj Khurana a écrit :

> Thanks, Jacques and Michael for your inputs,
>
> I agree with both the points, either such services should be removed as
> they are not being used and not written properly, or refactoring of the
> whole class with proper names as per their functionality to justify their
> name.
>
> I am willing to participate in this effort if we are more intended towards
> refactoring. Please let me know how we are proceeding in this.
>
> I have created a Jira here
> <https://issues.apache.org/jira/browse/OFBIZ-9982>.
>
> --
> Thanks and Regards,
> *Suraj Khurana* | Sr. Enterprise Software Engineer
> *HotWax Commerce*  by  *HotWax Systems*
> Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010
>
>
> On Wed, Nov 15, 2017 at 2:54 AM, Jacques Le Roux <
> [hidden email]> wrote:
>
>> Le 14/11/2017 à 21:39, Michael Brohl a écrit :
>>
>>> Am 14.11.17 um 19:15 schrieb Jacques Le Roux:
>>>
>>>> I see no reasons to drop those services if some find them useful. I get
>>>> they do since someone wrote it, even with a misleading name. Not having
>>>> them used OOTB is not a sufficient indication of removal.
>>>>
>>> I do not agree. This functionality is not used since a decade and the
>>> code is in bad shape.
>>>
>>> I think it makes much more sense to put effort in the many other fields
>>> of action we have in this project.
>>>
>>> Michael
>>>
>>>
>>>
>>> I can take care of renaming the methods, don't worry
>> Jacques
>>
>>

[Jacques Le Roux]

Hi Pawan, Scott,

We crossed on wire. I have already renamed the services. I think they can be useful in some situations. I see no reasons to remove, it's not buggy nor
bad code

Jacques


Le 15/11/2017 à 10:46, Pawan Verma a écrit :
> +1 for removing. I also see no use of this in the current system.
>
> --
> Thanks and Regards,
>
> *Pawan Verma*
>

[Michael Brohl-3]

We already have 3 people in favor of removing, please take this into
consideration.

Thanks,

Michael


Am 15.11.17 um 11:23 schrieb Jacques Le Roux:

> Hi Pawan, Scott,
>
> We crossed on wire. I have already renamed the services. I think they
> can be useful in some situations. I see no reasons to remove, it's not
> buggy nor bad code
>
> Jacques
>
>
> Le 15/11/2017 à 10:46, Pawan Verma a écrit :
>> +1 for removing. I also see no use of this in the current system.
>>
>> --
>> Thanks and Regards,
>>
>> *Pawan Verma*
>>
>

[Jacques Le Roux]

Feel free to remove if you want

Jacques


Le 15/11/2017 à 12:00, Michael Brohl a écrit :

> We already have 3 people in favor of removing, please take this into consideration.
>
> Thanks,
>
> Michael
>
>
> Am 15.11.17 um 11:23 schrieb Jacques Le Roux:
>> Hi Pawan, Scott,
>>
>> We crossed on wire. I have already renamed the services. I think they can be useful in some situations. I see no reasons to remove, it's not buggy
>> nor bad code
>>
>> Jacques
>>
>>
>> Le 15/11/2017 à 10:46, Pawan Verma a écrit :
>>> +1 for removing. I also see no use of this in the current system.
>>>
>>> --
>>> Thanks and Regards,
>>>
>>> *Pawan Verma*
>>>
>>
>
>