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

Enable the status selection for WorkEffort duplication

    Details

    • Sprint:
      Community Day 3 - 2016

      Description

      It is possible to duplicate workeffort in the WorkEffort component (https://localhost:8443/workeffort/control/EditWorkEffort?workEffortId=9000) but the duplicated WorkEffort keeps the same status than the original one. So when you duplicate a cancelled task, it is locked in the same state.

      We could adapt this service to allow the selection of the status for the new workeffort. Of course, you should be able to select only status that are relevant to the workeffort type.
      A first solution is to get the currentStatusId of the workeffort to duplicate. Then we can access to the linked statusTypeId. So we can retrieve all status relevant and order them by sequenceId (that way the default value is the first statuts of the sequence : created for the task).

      1. OFBIZ-7959.patch
        10 kB
        Montalbano Florian
      2. OFBIZ-7959.patch
        9 kB
        Montalbano Florian
      3. OFBIZ-7959.patch
        6 kB
        Montalbano Florian

        Activity

        Hide
        Florian M Montalbano Florian added a comment -

        Here is a first proposition. What do you think of it ?

        Show
        Florian M Montalbano Florian added a comment - Here is a first proposition. What do you think of it ?
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hi Florian,

        I like it, there was just one thing missing after my test (review OK). The status dropdown of the duplication panel is not translated, I did not chech why.

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hi Florian, I like it, there was just one thing missing after my test (review OK). The status dropdown of the duplication panel is not translated, I did not chech why.
        Hide
        Florian M Montalbano Florian added a comment -

        Hi Jacques Le Roux,
        thanks for your review.
        I forgot to put the uiLabelsMap in the ftl. I was getting the description directly from the StatusItem table. I'll try to patch this problem.

        Show
        Florian M Montalbano Florian added a comment - Hi Jacques Le Roux , thanks for your review. I forgot to put the uiLabelsMap in the ftl. I was getting the description directly from the StatusItem table. I'll try to patch this problem.
        Hide
        soledad Nicolas Malin added a comment -

        Yes I confirm

        <option value='${statusListSelect.statusId}'>${statusListSelect.getString("description")}</option>
        

        need to be

        <option value='${statusListSelect.statusId}'>${statusListSelect.getString("description", locale)}</option>
        

        but the better way would be move this ftl en xml form ^^

        An other point :

        +        <!-- Set the statusId of the new WorkEffort to this status, otherwise, set the status to the first of the sequenceId
        +          of the statusTypeId -->
        +        <attribute name="statusId" type="String" mode="IN" optional="true"/>
        

        Use the element description instead of xml comment to explain how use this attribute

        Show
        soledad Nicolas Malin added a comment - Yes I confirm <option value='${statusListSelect.statusId}'>${statusListSelect.getString( "description" )}</option> need to be <option value='${statusListSelect.statusId}'>${statusListSelect.getString( "description" , locale)}</option> but the better way would be move this ftl en xml form ^^ An other point : + <!-- Set the statusId of the new WorkEffort to this status, otherwise, set the status to the first of the sequenceId + of the statusTypeId --> + <attribute name= "statusId" type= " String " mode= "IN" optional= " true " /> Use the element description instead of xml comment to explain how use this attribute
        Hide
        Florian M Montalbano Florian added a comment -

        Here is a patch including the comments :

        • Moved the form from flt to xml
        • Enabled the description in locale langage (warning : some label does not have translation and thus can not be translated)
        • Corrected comment in the service

        The form appears a little bit different so feel free to submit idea for improvement.

        Show
        Florian M Montalbano Florian added a comment - Here is a patch including the comments : Moved the form from flt to xml Enabled the description in locale langage (warning : some label does not have translation and thus can not be translated) Corrected comment in the service The form appears a little bit different so feel free to submit idea for improvement.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Florian,

        I just tested (no review), this works well and the form looks nice to. Maybe we should not copy the 'old" workEffortId but generate automatically a new one, which could be still overridable

        I see no (French) labels missing, else it's easy to add them

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Florian, I just tested (no review), this works well and the form looks nice to. Maybe we should not copy the 'old" workEffortId but generate automatically a new one, which could be still overridable I see no (French) labels missing, else it's easy to add them
        Hide
        Florian M Montalbano Florian added a comment -

        Thanks Jacques Le Roux.
        What should I change to generate automatically a new workEffortId ?

        For the labels, the Scrum ones are not available in French yet (i'm working on it)

        Show
        Florian M Montalbano Florian added a comment - Thanks Jacques Le Roux . What should I change to generate automatically a new workEffortId ? For the labels, the Scrum ones are not available in French yet (i'm working on it)
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hi Florian,

        I suggest this in EditWorkEffortDup

        [... ]
        <set field="newWorkEffortId" value="${groovy: delegator.getNextSeqIdLong('WorkEffort')}"/>
        [... ]
        <field name="newWorkEffortId" parameter-name="workEffortId" tooltip="${uiLabelMap.ProductDuplicateRemoveSelectedWithNewId}"><text size="20" maxlength="20"></text></field>
        [... ]
        

        The user can still force the Id to the value he wants, we just suggest the next possible automated Id

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hi Florian, I suggest this in EditWorkEffortDup [... ] <set field= "newWorkEffortId" value= "${groovy: delegator.getNextSeqIdLong('WorkEffort')}" /> [... ] <field name= "newWorkEffortId" parameter-name= "workEffortId" tooltip= "${uiLabelMap.ProductDuplicateRemoveSelectedWithNewId}" ><text size= "20" maxlength= "20" ></text></field> [... ] The user can still force the Id to the value he wants, we just suggest the next possible automated Id
        Hide
        soledad Nicolas Malin added a comment -

        Jacques I'm surprised by a getNextSeqId call out of crud or functional service. Are your sure about this ?

        Show
        soledad Nicolas Malin added a comment - Jacques I'm surprised by a getNextSeqId call out of crud or functional service. Are your sure about this ?
        Hide
        Florian M Montalbano Florian added a comment -

        Thank you Jacques, this seems to be a nice improvement.
        The only downside is that each time you refresh the page, it increments the getNextSeqIdLong even if you don't use it.
        I don't think it's a problem but I would like to confirm it.

        I attached the patch including your code.

        Show
        Florian M Montalbano Florian added a comment - Thank you Jacques, this seems to be a nice improvement. The only downside is that each time you refresh the page, it increments the getNextSeqIdLong even if you don't use it. I don't think it's a problem but I would like to confirm it. I attached the patch including your code.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        I can answer to both of you at the same time. Yes, it's not a problem the only thing it does is indeed increasing the sequence id and, apart if you have constraint on serialised (not discontinued) workEffortIds, that should not be a problem. I mean in case you like to play with the F5 touch .

        BTW sequenced-id-prefix element in minilang does the same but indeed in a transaction. So If we want to be totally secure it should be better in a service. This to avoid possible already used (in the meantime) workEffortIds, hence an entity creation rejection. But let's face it, you can put what you want there anyway. So I don't see that as a big deal, only a convenient way for users, if it fails try a new one, this can happen anyway...

        Show
        jacques.le.roux Jacques Le Roux added a comment - I can answer to both of you at the same time. Yes, it's not a problem the only thing it does is indeed increasing the sequence id and, apart if you have constraint on serialised (not discontinued) workEffortIds, that should not be a problem. I mean in case you like to play with the F5 touch . BTW sequenced-id-prefix element in minilang does the same but indeed in a transaction. So If we want to be totally secure it should be better in a service. This to avoid possible already used (in the meantime) workEffortIds, hence an entity creation rejection. But let's face it, you can put what you want there anyway. So I don't see that as a big deal, only a convenient way for users, if it fails try a new one, this can happen anyway...
        Hide
        soledad Nicolas Malin added a comment -

        Ok Florient, your submission is on trunk at revision 1761233.

        I realize some correction :

        • remove modification to load status from Scrum et Project, simply because it's not datas core. If we want set this drop-down dynamical we need to improve the status resolution by a statusType tree.
        • remove tooltip on newWorkEffortId field because is not corresponding to the situation
        • add label for field newWorkEffortId

        I'm not agree to keep newWorkEffortId field because it's potential risk for the automatic system to have an advertise user that when hurt it. But it's already present on ftl and if we really want remove it, we can open a new issue

        Show
        soledad Nicolas Malin added a comment - Ok Florient, your submission is on trunk at revision 1761233. I realize some correction : remove modification to load status from Scrum et Project, simply because it's not datas core. If we want set this drop-down dynamical we need to improve the status resolution by a statusType tree. remove tooltip on newWorkEffortId field because is not corresponding to the situation add label for field newWorkEffortId I'm not agree to keep newWorkEffortId field because it's potential risk for the automatic system to have an advertise user that when hurt it. But it's already present on ftl and if we really want remove it, we can open a new issue

          People

          • Assignee:
            soledad Nicolas Malin
            Reporter:
            Florian M Montalbano Florian
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development

                Agile