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

Various fixes related with sales opportunity

    Details

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

      Description

      Hans directly committed

      • r1723007 "sales opportunity creation: required-field removed, disturbed the form submission, entityone no value field, error in the log"
      • and r1723248 "various fixes to the salesopportunity list and create functions"

      I wanted to backport them and create this Jira for releases notes. But before I checked the commits and found an issue, see http://markmail.org/message/mspie3qdjcac2tci

      1. sfa.patch
        2 kB
        Hans Bakker

        Activity

        Hide
        jacques.le.roux Jacques Le Roux added a comment - - edited

        Following the discussion on dev ML. I reverted the change done with http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/secas.xml?r1=1723248&r2=1723247&pathrev=1723248 with revision: 1725574

        Show
        jacques.le.roux Jacques Le Roux added a comment - - edited Following the discussion on dev ML. I reverted the change done with http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/secas.xml?r1=1723248&r2=1723247&pathrev=1723248 with revision: 1725574
        Hide
        hansbak Hans Bakker added a comment -

        Jacques, perhaps you missed it but I answered on the mailing list with:
        because this change makes the leadparty a mandatory field. If not filled in, it will not appear in the list screen.

        Regards,
        Hans

        To re-produce the error now:
        create a new opportunity with just the opportunity name filled in.

        go back to the opportunity list, and your newly opportunity will not appear.

        Show
        hansbak Hans Bakker added a comment - Jacques, perhaps you missed it but I answered on the mailing list with: because this change makes the leadparty a mandatory field. If not filled in, it will not appear in the list screen. Regards, Hans To re-produce the error now: create a new opportunity with just the opportunity name filled in. go back to the opportunity list, and your newly opportunity will not appear.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Come on Hans, you really wants to upset me :-o ?

        No I did not miss it, but it seems you missed my anwser.
        I see your point, I though thought that, with my explanation, you would debug/fix that yourself since you committed in 1st place.

        I already answered you (see thread link in description) that to create an opportunity you need a leadPartyId. Because OOTB in UI (same with services) to create an opportunity you must

        1. use the EditSalesOpportunity request which loads the EditSalesOpportunity screen itself using the OpportunityForms.xml#EditSalesOpportunity form.
        2. This form calls the createSalesOpportunity request when it's not an update.
        3. The createSalesOpportunity request calls the createSalesOpportunity service.
        4. The createSalesOpportunity service triggers the SECAs which launch the createSalesOpportunityLeadRole and/or createSalesOpportunityAccountRole services if leadPartyId and/or accountPartyId (ONLY IF) is/are not empty.

        Since leadPartyId and/or accountPartyId are no mandatory in EditSalesOpportunity form, nor in createSalesOpportunity/updateSalesOpportunity services, that's why you have this SECA condition, else you would not be able to create/update an opportunity.

        I hope I gave you enough details to fix the issue. Hint: it's the list which should be fixed, not the SECA (and even less only one of them but the 4 - create/update for lead and account - if you would have been consistent but still wrong)

        You really have a skill to have others working for you, don't you?

        I'm an OFBiz volunteer, but sometimes I wonder... And today I better understand Adrian's harsh reaction on dev ML one day, please refer to it...

        Show
        jacques.le.roux Jacques Le Roux added a comment - Come on Hans, you really wants to upset me :-o ? No I did not miss it, but it seems you missed my anwser. I see your point, I though thought that, with my explanation, you would debug/fix that yourself since you committed in 1st place. I already answered you ( see thread link in description ) that to create an opportunity you need a leadPartyId. Because OOTB in UI (same with services) to create an opportunity you must use the EditSalesOpportunity request which loads the EditSalesOpportunity screen itself using the OpportunityForms.xml#EditSalesOpportunity form. This form calls the createSalesOpportunity request when it's not an update. The createSalesOpportunity request calls the createSalesOpportunity service. The createSalesOpportunity service triggers the SECAs which launch the createSalesOpportunityLeadRole and/or createSalesOpportunityAccountRole services if leadPartyId and/or accountPartyId (ONLY IF) is/are not empty. Since leadPartyId and/or accountPartyId are no mandatory in EditSalesOpportunity form, nor in createSalesOpportunity/updateSalesOpportunity services, that's why you have this SECA condition, else you would not be able to create/update an opportunity. I hope I gave you enough details to fix the issue. Hint: it's the list which should be fixed, not the SECA (and even less only one of them but the 4 - create/update for lead and account - if you would have been consistent but still wrong) You really have a skill to have others working for you, don't you? I'm an OFBiz volunteer, but sometimes I wonder... And today I better understand Adrian's harsh reaction on dev ML one day, please refer to it...
        Hide
        hansbak Hans Bakker added a comment -

        My apologies if I upset you Jacques, it was not my intention.

        this problems can be solved either, changing the seca to make the lead mandatory, which you do not seem to like or we change the listing program so it will list all opportunities even if there is no lead or account

        Attached a patch for the second possibility.
        in this case however it is not possible to select only the lead of the role so depending on availability either the lead or the account is shown.

        anyway that is all i can do......

        Show
        hansbak Hans Bakker added a comment - My apologies if I upset you Jacques, it was not my intention. this problems can be solved either, changing the seca to make the lead mandatory, which you do not seem to like or we change the listing program so it will list all opportunities even if there is no lead or account Attached a patch for the second possibility. in this case however it is not possible to select only the lead of the role so depending on availability either the lead or the account is shown. anyway that is all i can do......
        Hide
        hansbak Hans Bakker added a comment -

        patch to change the opportunitylist to show an opportunity even if no lead or account attached

        Show
        hansbak Hans Bakker added a comment - patch to change the opportunitylist to show an opportunity even if no lead or account attached
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hans,

        I agree with you that's almost all we can do w/o revisting the data model. I though improved things a bit at revision: 1726493

        I added a sort to have all same (with different possible roleTypeIds) opportunities subsequent, added the roleTypeId near the salesOpportunityId to make it obvious, and added a RoleType column. While at it, I also replaced useless " by real quote in groovy expressions (I have a plan to generalise this...one day...)

        Thanks!

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hans, I agree with you that's almost all we can do w/o revisting the data model. I though improved things a bit at revision: 1726493 I added a sort to have all same (with different possible roleTypeIds) opportunities subsequent, added the roleTypeId near the salesOpportunityId to make it obvious, and added a RoleType column. While at it, I also replaced useless " by real quote in groovy expressions (I have a plan to generalise this...one day...) Thanks!
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Forgot to say that I also replaced the link from the opportunities list to a view screen by an edit screen (which can do more hence less)

        Show
        jacques.le.roux Jacques Le Roux added a comment - Forgot to say that I also replaced the link from the opportunities list to a view screen by an edit screen (which can do more hence less)
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Backported in
        R15.12 r1726543, 1726544, 1726545, 1726546
        R14.12 r1726548, 1726551, 1726552, 1726553 (3 conflicts handled by hand in 2 of the commits)

        OK, I give up for the rest, who cares anyway :/

        Show
        jacques.le.roux Jacques Le Roux added a comment - Backported in R15.12 r1726543, 1726544, 1726545, 1726546 R14.12 r1726548, 1726551, 1726552, 1726553 (3 conflicts handled by hand in 2 of the commits) OK, I give up for the rest, who cares anyway :/
        Hide
        hansbak Hans Bakker added a comment -

        Jacques, i admire your dedication, but, sorry to say, i still think that making the lead party mandatory would have been much easier and even cleaner too......what is an opportunity without a lead?

        Show
        hansbak Hans Bakker added a comment - Jacques, i admire your dedication, but, sorry to say, i still think that making the lead party mandatory would have been much easier and even cleaner too......what is an opportunity without a lead?
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hans, yes I thought the same. The fix would then be in data model, services and forms, not in SECA. We could discuss that on dev ML. I wonder why it was done like that. There must be a reason. So this change should not be done unilaterally but discussed 1st on dev ML, like we use to do...

        BTW I just learned that it initially comes from OpenSourceStrategies http://markmail.org/message/w6dezxshcllfekuc

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hans, yes I thought the same. The fix would then be in data model, services and forms, not in SECA. We could discuss that on dev ML. I wonder why it was done like that. There must be a reason. So this change should not be done unilaterally but discussed 1st on dev ML, like we use to do... BTW I just learned that it initially comes from OpenSourceStrategies http://markmail.org/message/w6dezxshcllfekuc

          People

          • Assignee:
            jacques.le.roux Jacques Le Roux
            Reporter:
            jacques.le.roux Jacques Le Roux
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development