Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-6703

Cannot create more than one lead in the SFA component by same user

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Trunk
    • Fix Version/s: 14.12.01, 16.11.01
    • Component/s: marketing
    • Labels:
      None

      Description

      Repeat steps:

      1. go to SFA component and click on leads
      2. click on create new
      3. fill mandatory information and save
      4. repeat the above steps again and observe failure

      The reason for the failure is that the implementation of the createLead service tries to create a PartyRole of type OWNER every time a lead is created. So it works only the first time

        Activity

        Hide
        taher Taher Alkhateeb added a comment -

        Fixed in r1711020 by putting a check condition to create a PartyRole of type OWNER only if it does not exist

        Show
        taher Taher Alkhateeb added a comment - Fixed in r1711020 by putting a check condition to create a PartyRole of type OWNER only if it does not exist
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        HI Taher, why not backporting?

        Show
        jacques.le.roux Jacques Le Roux added a comment - HI Taher, why not backporting?
        Hide
        taher Taher Alkhateeb added a comment -

        Reopening based on Jacques recommendation to backport.

        Jacques, to which versions should I backport?

        Show
        taher Taher Alkhateeb added a comment - Reopening based on Jacques recommendation to backport. Jacques, to which versions should I backport?
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Taher, I guess at least R14.12, then maybe R13 and 12. Of course be careful when you backport. Most of the time it's straightforward but sometimes you get bitten. I can help if you want.

        Show
        jacques.le.roux Jacques Le Roux added a comment - Taher, I guess at least R14.12, then maybe R13 and 12. Of course be careful when you backport. Most of the time it's straightforward but sometimes you get bitten. I can help if you want.
        Hide
        taher Taher Alkhateeb added a comment -

        Okay, I backported to all releases
        backport to R14.12 on r1711036
        backport to R13.07 on r1711038
        backport to R12.04 on r1711040

        I actually cannot believe this! A core application component had a problem like this lurking for years! I wonder how many bugs exist everywhere else.

        Show
        taher Taher Alkhateeb added a comment - Okay, I backported to all releases backport to R14.12 on r1711036 backport to R13.07 on r1711038 backport to R12.04 on r1711040 I actually cannot believe this! A core application component had a problem like this lurking for years! I wonder how many bugs exist everywhere else.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Yes, unfortunately some parts have been not developed as well as some others :/

        If you are intererested we have 39 bug issues with patches pending https://issues.apache.org/jira/browse/OFBIZ-6697?filter=12333848

        Show
        jacques.le.roux Jacques Le Roux added a comment - Yes, unfortunately some parts have been not developed as well as some others :/ If you are intererested we have 39 bug issues with patches pending https://issues.apache.org/jira/browse/OFBIZ-6697?filter=12333848
        Hide
        hansbak Hans Bakker added a comment - - edited

        this was caused, because the internal check for existence was removed from the createPartyRole service for whatever reason, and the sfa component was not updated accordingly if i will remember well Nicolas?

        Show
        hansbak Hans Bakker added a comment - - edited this was caused, because the internal check for existence was removed from the createPartyRole service for whatever reason, and the sfa component was not updated accordingly if i will remember well Nicolas?
        Hide
        hansbak Hans Bakker added a comment -

        so, it could well be that older versions do not need patching.

        Show
        hansbak Hans Bakker added a comment - so, it could well be that older versions do not need patching.
        Hide
        taher Taher Alkhateeb added a comment - - edited

        Hi Hans,

        You are right, the revert is not necessary for releases 12.04 and 13.07, after investigating this I found that below commit. This is now irritating me more. We are switching from minilang to entity-auto without any thought to business rules embedded in the services! Why do we do that?

        I think a cleaner solution is revert the logic on all the releases including trunk and instead reintroduce the service implementation instead of entity auto. What do you think?

        r1622161 | jleroux | 2014-09-03 06:55:32 +0300 (Wed, 03 Sep 2014) | 30 lines

        A patch from Nicolas Malin for "Convert Party entites CRUD service from simple to entity-auto" OFBIZ-5750

        I converted CRUD service to entity-auto for :
        PartyRole
        PostalAddressBoundary
        PartyClassification
        PartyClassificationGroup
        PartyAttribute
        Vendor
        PartyCarrierAccount

        Show
        taher Taher Alkhateeb added a comment - - edited Hi Hans, You are right, the revert is not necessary for releases 12.04 and 13.07, after investigating this I found that below commit. This is now irritating me more. We are switching from minilang to entity-auto without any thought to business rules embedded in the services! Why do we do that? I think a cleaner solution is revert the logic on all the releases including trunk and instead reintroduce the service implementation instead of entity auto. What do you think? r1622161 | jleroux | 2014-09-03 06:55:32 +0300 (Wed, 03 Sep 2014) | 30 lines A patch from Nicolas Malin for "Convert Party entites CRUD service from simple to entity-auto" OFBIZ-5750 I converted CRUD service to entity-auto for : PartyRole PostalAddressBoundary PartyClassification PartyClassificationGroup PartyAttribute Vendor PartyCarrierAccount
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        I crossed 1 or 2 related issues. In most cases I think it was OK but the generalisation was hastily done indeed. I'd rather review the work done in OFBIZ-5750 and change only the needed parts. Notably those were check for authorisations existed before.

        Show
        jacques.le.roux Jacques Le Roux added a comment - I crossed 1 or 2 related issues. In most cases I think it was OK but the generalisation was hastily done indeed. I'd rather review the work done in OFBIZ-5750 and change only the needed parts. Notably those were check for authorisations existed before.
        Hide
        pfm.smits Pierre Smits added a comment -

        If you find any bugs, feel free to create new JIRA issues.

        Show
        pfm.smits Pierre Smits added a comment - If you find any bugs, feel free to create new JIRA issues.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Hans, at r1714408, I reverted
        r1711038 in R13.07
        r1711040 in R12.04

        Revert in R14.12 was not a good idea, it would be missing there, r1622161 was committed 03/Sep/14 for OFBIZ-5750 "Convert Party entites CRUD service from simple to entity-auto" before R14.2 was created

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Hans, at r1714408, I reverted r1711038 in R13.07 r1711040 in R12.04 Revert in R14.12 was not a good idea, it would be missing there, r1622161 was committed 03/Sep/14 for OFBIZ-5750 "Convert Party entites CRUD service from simple to entity-auto" before R14.2 was created
        Hide
        hansbak Hans Bakker added a comment -

        at the time, i also did not agree, but he really pushed it and i gave up on it....

        Show
        hansbak Hans Bakker added a comment - at the time, i also did not agree, but he really pushed it and i gave up on it....
        Hide
        gil portenseigne Gil Portenseigne added a comment -

        Hi all, Sorry for waking up a bit late on this subject, but the best way to fix this bug was replacing createPartyRole service call by ensurePartyRole service, which is done for this use.

        Indeed the entity-auto improvments made by Nicolas did not check every createPartyRole usage, leading to this kind of bug.

        Reverting entity-auto implementation is a bad solution, bringing back createPartyRole/ensurePartyRole service usage duplication, and createPartyRole naming as a CRUD service to a different one...

        Hans, Internal check is already done by ensurePartyRole, it was just the information missing here !

        Show
        gil portenseigne Gil Portenseigne added a comment - Hi all, Sorry for waking up a bit late on this subject, but the best way to fix this bug was replacing createPartyRole service call by ensurePartyRole service, which is done for this use. Indeed the entity-auto improvments made by Nicolas did not check every createPartyRole usage, leading to this kind of bug. Reverting entity-auto implementation is a bad solution, bringing back createPartyRole/ensurePartyRole service usage duplication, and createPartyRole naming as a CRUD service to a different one... Hans, Internal check is already done by ensurePartyRole, it was just the information missing here !
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Gil,

        Could you please take over and fix as yhou suggest?

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Gil, Could you please take over and fix as yhou suggest?
        Hide
        gil portenseigne Gil Portenseigne added a comment -

        Yes I will !

        Show
        gil portenseigne Gil Portenseigne added a comment - Yes I will !
        Hide
        gil portenseigne Gil Portenseigne added a comment -

        Fixed in
        Trunk r1714974
        14.12 r1714975

        Thanks all !

        Show
        gil portenseigne Gil Portenseigne added a comment - Fixed in Trunk r1714974 14.12 r1714975 Thanks all !
        Hide
        taher Taher Alkhateeb added a comment -

        Thank you Gil, Hans and Jacques. This is truly the beauty of having a community helping each other out!

        Show
        taher Taher Alkhateeb added a comment - Thank you Gil, Hans and Jacques. This is truly the beauty of having a community helping each other out!

          People

          • Assignee:
            gil portenseigne Gil Portenseigne
            Reporter:
            taher Taher Alkhateeb
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development