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

RateAmount is not found when the level is 'WorkEffort'

    Details

    • Sprint:
      Community Day 3 - 2016

      Description

      When you create a RateAmount, you can determine for which WorkEffort it should apply. But when this RateAmount is retrieved for a specific WorkEffort, it fails to find the good RateAmount.

      The problem comes from the check done to retrieve the RateAmount at a WorkEffort level. This is the used check :

      <entity-and entity-name="RateAmount" list="amounts" filter-by-date="true">
                      <field-map field-name="rateTypeId" from-field="parameters.rateTypeId"/>
                      <field-map field-name="partyId" from-field="parameters.partyId"/>
                      <field-map field-name="workEffortId" from-field="parameters.workEffortId"/>
                      <field-map field-name="periodTypeId" from-field="parameters.periodTypeId"/>
                      <field-map field-name="rateCurrencyUomId" from-field="parameters.rateCurrencyUomId"/>
                  </entity-and>
                  <if-empty field="amounts"> 
      

      In this 'entity-and', the partyId is set as a constraint. But it is possible to enable a special RateAmount only for a WorkEffort without regarding the partyId.

      I think we have 2 options here :

      1. Remove the partyId constraint but then the retrieving may be less accurate
      2. Make the retrieving more accurate by checking first the WorkEffort, then from the retrieved list, check if the partyId matches and finally from this second retrieval, check if the emplPositionTypeId match. At the end, we would have the most accurate RateAmount.

      What do you think of it ?

      1. OFBIZ-7957.patch
        26 kB
        Montalbano Florian

        Activity

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

        Sounds good to me

        Show
        jacques.le.roux Jacques Le Roux added a comment - Sounds good to me
        Hide
        Florian M Montalbano Florian added a comment -

        Hmm, I should have specified that we can't have the two options at the same time as there are contradictory ^^'
        So Jacques Le Roux, does the first option sounds better or it is the second one ?

        Show
        Florian M Montalbano Florian added a comment - Hmm, I should have specified that we can't have the two options at the same time as there are contradictory ^^' So Jacques Le Roux , does the first option sounds better or it is the second one ?
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Yep 2 was obvious to me, sorry for the confusion. BTW, and this is also true for Taher Alkhateeb, when you have several antagonist options better to use numeration, in Jira it starts with #

        Show
        jacques.le.roux Jacques Le Roux added a comment - Yep 2 was obvious to me, sorry for the confusion. BTW, and this is also true for Taher Alkhateeb , when you have several antagonist options better to use numeration, in Jira it starts with #
        Hide
        taher Taher Alkhateeb added a comment -

        It seems to me like the fix in this JIRA is at the wrong level of abstraction.Your query depends on context correct? It should be on party, workeffort, or something else.

        Whenever I face a problem like this, I immediately think to myself (I am at the wrong level of abstraction, I need to go up). So very simply, the service needs to be broken down to more services that depend on the different contexts. Alternatively, we can also pipe the results in SECA depending on the context. This way you can have both options

        Show
        taher Taher Alkhateeb added a comment - It seems to me like the fix in this JIRA is at the wrong level of abstraction.Your query depends on context correct? It should be on party, workeffort, or something else. Whenever I face a problem like this, I immediately think to myself (I am at the wrong level of abstraction, I need to go up). So very simply, the service needs to be broken down to more services that depend on the different contexts. Alternatively, we can also pipe the results in SECA depending on the context. This way you can have both options
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Yes indeed, this is similar (in essence) to what Scott proposed for the "tracking code logic" at http://markmail.org/message/pvi3q6l6zaayjz6s

        Show
        jacques.le.roux Jacques Le Roux added a comment - Yes indeed, this is similar (in essence) to what Scott proposed for the "tracking code logic" at http://markmail.org/message/pvi3q6l6zaayjz6s
        Hide
        Florian M Montalbano Florian added a comment -

        Thanks Taher Alkhateeb for this solution and Jacques Le Roux for the source.
        As you said, I stayed at the function level.
        So, when you talk about uping the level of abstraction, I get the feeling that it is the right thing to do here. Breaking down this function into several services will do good.

        Can you explain a little more (or give an example) for this 'piping the results in SECA' thing ? Does that mean that when I want to retrieve the RateAmount of a TimeEntry for an Invoice, it will call a first service which will trigger other services call given the context ?

        Next time I need to list some antagonist stuff, I'll use the '#' to display it

        Show
        Florian M Montalbano Florian added a comment - Thanks Taher Alkhateeb for this solution and Jacques Le Roux for the source. As you said, I stayed at the function level. So, when you talk about uping the level of abstraction, I get the feeling that it is the right thing to do here. Breaking down this function into several services will do good. Can you explain a little more (or give an example) for this 'piping the results in SECA' thing ? Does that mean that when I want to retrieve the RateAmount of a TimeEntry for an Invoice, it will call a first service which will trigger other services call given the context ? Next time I need to list some antagonist stuff, I'll use the '#' to display it
        Hide
        taher Taher Alkhateeb added a comment -

        In my opinion we should minimize the use of services in other services. Having the services being independent of one another means that you can combine them in new and novel ways. But composeability requires First Independence.

        This is where in my opinion SECAs play an important role. Because here you are combining the services exactly how you need them in that specific certain context while maintaining the purity Independence and modularity of your services. This is sort of where all the hype of microservices is coming from.

        However if the SECAs become too complex or unrealistic to implement then at that point you might call services from other services.

        The code examples that you want are plentiful and you can find them in multiple components check for example the accounting component you'll probably find some good examples there.

        Show
        taher Taher Alkhateeb added a comment - In my opinion we should minimize the use of services in other services. Having the services being independent of one another means that you can combine them in new and novel ways. But composeability requires First Independence. This is where in my opinion SECAs play an important role. Because here you are combining the services exactly how you need them in that specific certain context while maintaining the purity Independence and modularity of your services. This is sort of where all the hype of microservices is coming from. However if the SECAs become too complex or unrealistic to implement then at that point you might call services from other services. The code examples that you want are plentiful and you can find them in multiple components check for example the accounting component you'll probably find some good examples there.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        BTW Florian, another tip while at it. Not sure why you used the @name notation above, but in case: you don't need to use it to notice people who have participated in the issue, Jira knows it and will forward your comments to them

        Show
        jacques.le.roux Jacques Le Roux added a comment - BTW Florian, another tip while at it. Not sure why you used the @name notation above, but in case: you don't need to use it to notice people who have participated in the issue, Jira knows it and will forward your comments to them
        Hide
        Florian M Montalbano Florian added a comment -

        Sorry about that, I thought it was used by Jira to do some "semantic search thing".
        I'll refrain to use it when it's about participant then.

        Thanks again for the tip

        Show
        Florian M Montalbano Florian added a comment - Sorry about that, I thought it was used by Jira to do some "semantic search thing". I'll refrain to use it when it's about participant then. Thanks again for the tip
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        You don't need to apologize Florian, it's just a convenient tip for you, using the @name notation is not an annoyance (but a feature )

        Show
        jacques.le.roux Jacques Le Roux added a comment - You don't need to apologize Florian, it's just a convenient tip for you, using the @name notation is not an annoyance (but a feature )
        Hide
        Florian M Montalbano Florian added a comment -

        Here is a patch to correct this issue.
        I went by the 'micro-service' solution for now and I kept the inpt/output of the modified service 'getRateAmount'.

        I'm now wondering where I can put an option to retrieve the rateAmount of a parent workEffort is the workEffortId given in the parameters does not match any rateAmount. This way, we could put a 'global' rateAmount on a porject and punctually set rateAmount on given task of this project. It could works with phases too.

        Every review of this patch is welcomed !

        Show
        Florian M Montalbano Florian added a comment - Here is a patch to correct this issue. I went by the 'micro-service' solution for now and I kept the inpt/output of the modified service 'getRateAmount'. I'm now wondering where I can put an option to retrieve the rateAmount of a parent workEffort is the workEffortId given in the parameters does not match any rateAmount. This way, we could put a 'global' rateAmount on a porject and punctually set rateAmount on given task of this project. It could works with phases too. Every review of this patch is welcomed !
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        HI Florian, I did not test your patch, but after review I think it's a good idea to proceed this way. Taher suggested SECAs, but I think it was more in a general way and I agree with what he said. In this case it's only about workeffort-rate-amouts so the domain is "siloed" and it's OK to use services called by services.

        If nobody disagree I'll commit your patch soon.

        Show
        jacques.le.roux Jacques Le Roux added a comment - HI Florian, I did not test your patch, but after review I think it's a good idea to proceed this way. Taher suggested SECAs, but I think it was more in a general way and I agree with what he said. In this case it's only about workeffort-rate-amouts so the domain is "siloed" and it's OK to use services called by services. If nobody disagree I'll commit your patch soon.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        I never used this feature before. I notice it's limited because you can't edit (search and select) a rate amount only delete. Both the related screens in accounting and workeffort does not allow to search, you always see the plain list. Ah, and the button at accounting/control/updateRateAmount says "update Rate Amount" but you actually can update only when you have just created, once you click again on the menu you can't edit as I said above. I thought about creating issues for that, but I'll simply focus on the task at hand for now.

        Show
        jacques.le.roux Jacques Le Roux added a comment - I never used this feature before. I notice it's limited because you can't edit (search and select) a rate amount only delete. Both the related screens in accounting and workeffort does not allow to search, you always see the plain list. Ah, and the button at accounting/control/updateRateAmount says "update Rate Amount" but you actually can update only when you have just created, once you click again on the menu you can't edit as I said above. I thought about creating issues for that, but I'll simply focus on the task at hand for now.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Florian,

        Your patch is in
        trunk r1761179
        R15.12, 14.12, 13.07 r1761188

        I simply fixed some typos in the documenation of the getRateAmount service.

        I appreciated the log "A valid rate entry could be found for" but just thought that it should be a warning and not an error. I'll fix that as non functional change only in trunk.

        [You] wondered where to put an option to retrieve the rateAmount of a parent workEffort is the workEffortId given in the parameters does not match any rateAmount. This way, we could put a 'global' rateAmount on a porject and punctually set rateAmount on given task of this project. It could works with phases too.

        You should create a new Jira for that.
        Also we should create a Jira to improve the rate amount screens in both accounting and workeffort components as I said above

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Florian, Your patch is in trunk r1761179 R15.12, 14.12, 13.07 r1761188 I simply fixed some typos in the documenation of the getRateAmount service. I appreciated the log "A valid rate entry could be found for" but just thought that it should be a warning and not an error. I'll fix that as non functional change only in trunk. [You] wondered where to put an option to retrieve the rateAmount of a parent workEffort is the workEffortId given in the parameters does not match any rateAmount. This way, we could put a 'global' rateAmount on a porject and punctually set rateAmount on given task of this project. It could works with phases too. You should create a new Jira for that. Also we should create a Jira to improve the rate amount screens in both accounting and workeffort components as I said above

          People

          • Assignee:
            jacques.le.roux Jacques Le Roux
            Reporter:
            Florian M Montalbano Florian
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development

                Agile