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

SalaryStep entity missing From and Thru dates

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Trunk
    • Fix Version/s: 16.11.01
    • Component/s: humanres
    • Labels:
      None
    • Sprint:
      Bug Crush Event - 21/2/2015

      Description

      The SalaryStep entity is missing From and Thru dates which means that the record for a SalaryStep can only hold the current value of the salary

      This means, for example, that if a company wants to apply a 10% increase to salary step value that is effective from a future date they currently would have to wait until that date is reached before they could modify the value

        Issue Links

          Activity

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

          I add to fix 3 warnings in log with r1711426

          Show
          jacques.le.roux Jacques Le Roux added a comment - I add to fix 3 warnings in log with r1711426
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          It's complete with revision: 1709272 which adds the migrateSalaryStep service to allow migrating data from OldSalaryStep to SalaryStep.

          Show
          jacques.le.roux Jacques Le Roux added a comment - It's complete with revision: 1709272 which adds the migrateSalaryStep service to allow migrating data from OldSalaryStep to SalaryStep.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Completed again at revision: 1709271

          Show
          jacques.le.roux Jacques Le Roux added a comment - Completed again at revision: 1709271
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          OK I made a last change at revision: 1709238

          Show
          jacques.le.roux Jacques Le Roux added a comment - OK I made a last change at revision: 1709238
          Show
          jacques.le.roux Jacques Le Roux added a comment - I will update the https://cwiki.apache.org/confluence/display/OFBIZ/Revisions+Requiring+Data+Migration+-+upgrade+ofbiz page soon...
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          All not trunk changes reverted at r1709235

          Show
          jacques.le.roux Jacques Le Roux added a comment - All not trunk changes reverted at r1709235
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          OK, it was a bad idea to change this from improvement to bug because of the data model modification. I will revert all changes not in trunk

          Show
          jacques.le.roux Jacques Le Roux added a comment - OK, it was a bad idea to change this from improvement to bug because of the data model modification. I will revert all changes not in trunk
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          I forgot to change the type of the issue. I decided this was a bug not an improvement because functionally it was almost useless as is.

          I also decided that the "Salary StepSeq Id" Lookup needed not only a search on the fromDate but also to display the through date and the amount. Added in
          trunk r1709231
          R14.12 r1709232
          R13.07 r1709233
          R12.04 r1709234

          Show
          jacques.le.roux Jacques Le Roux added a comment - I forgot to change the type of the issue. I decided this was a bug not an improvement because functionally it was almost useless as is. I also decided that the "Salary StepSeq Id" Lookup needed not only a search on the fromDate but also to display the through date and the amount. Added in trunk r1709231 R14.12 r1709232 R13.07 r1709233 R12.04 r1709234
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          At revision: 1709214 I completed r1709192 by adding fromDate key-map field for SalaryStep relations, in trunk.

          I decided other changes (adding a fromDate), notably in "Position Type Rate" and "Lookup Salary StepSeq Id" screens were not necessary

          Backported in
          R14.12 at r1709215+1709216
          R13.07 at r1709216+1709218+1709222
          R12.04 at r1709219+1709220+1709222

          Show
          jacques.le.roux Jacques Le Roux added a comment - At revision: 1709214 I completed r1709192 by adding fromDate key-map field for SalaryStep relations, in trunk. I decided other changes (adding a fromDate), notably in "Position Type Rate" and "Lookup Salary StepSeq Id" screens were not necessary Backported in R14.12 at r1709215+1709216 R13.07 at r1709216+1709218+1709222 R12.04 at r1709219+1709220+1709222
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Mmm, I commited at revision: 1709192 but I wonder now If I did not miss 2 or 3 things, I will check that later... I mean soon...

          Show
          jacques.le.roux Jacques Le Roux added a comment - Mmm, I commited at revision: 1709192 but I wonder now If I did not miss 2 or 3 things, I will check that later... I mean soon...
          Hide
          jacopoc Jacopo Cappellato added a comment -

          Please do some tests before committing it.

          Show
          jacopoc Jacopo Cappellato added a comment - Please do some tests before committing it.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          OK no more reviews, so I will commit as is in few days...

          Show
          jacques.le.roux Jacques Le Roux added a comment - OK no more reviews, so I will commit as is in few days...
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Exactly, so we can discuss it in an easy manner

          Show
          jacques.le.roux Jacques Le Roux added a comment - Exactly, so we can discuss it in an easy manner
          Hide
          pfm.smits Pierre Smits added a comment -

          That is not a patch. That is a transcript of how a patch could look like.

          Show
          pfm.smits Pierre Smits added a comment - That is not a patch. That is a transcript of how a patch could look like.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Here is the suggested patch

          Index: applications/humanres/entitydef/entitymodel.xml
          ===================================================================
          --- applications/humanres/entitydef/entitymodel.xml	(revision 1652873)
          +++ applications/humanres/entitydef/entitymodel.xml	(working copy)
          @@ -597,10 +597,15 @@
                       title="Salary Step Entity">
                 <field name="salaryStepSeqId" type="id-ne"></field>
                 <field name="payGradeId" type="id-ne"></field>
          +      <field name="fromDate" type="date-time"></field>
          +      <field name="thruDate" type="date-time"></field>
                 <field name="dateModified" type="date-time"></field>
                 <field name="amount" type="currency-amount"></field>
          +      <field name="createdByUserLogin" type="id-vlong"></field>
          +      <field name="lastModifiedByUserLogin" type="id-vlong"></field>
                 <prim-key field="salaryStepSeqId"/>
                 <prim-key field="payGradeId"/>
          +      <prim-key field="fromDate"/>
                 <relation type="one" fk-name="SLRY_STP_PGRD" rel-entity-name="PayGrade">
                   <key-map field-name="payGradeId"/>
                 </relation>
          Index: applications/humanres/servicedef/services.xml
          ===================================================================
          --- applications/humanres/servicedef/services.xml	(revision 1652873)
          +++ applications/humanres/servicedef/services.xml	(working copy)
          @@ -288,6 +288,7 @@
                   <permission-service service-name="humanResManagerPermission" main-action="CREATE"/>
                   <auto-attributes mode="IN" include="nonpk" optional="true"/>
                   <attribute name="payGradeId" mode="IN" type="String"/>
          +        <attribute name="fromDate" mode="IN" type="Timestamp"/>
                   <attribute name="salaryStepSeqId" mode="OUT" type="String"/>
               </service>
           
          

          I think the OOTB code is OK with that (it uses auto-attributes and auto-fields-entity)

          But this will need a comment at https://cwiki.apache.org/confluence/display/OFBIZ/Revisions+Requiring+Data+Migration+-+upgrade+ofbiz

          Show
          jacques.le.roux Jacques Le Roux added a comment - Here is the suggested patch Index: applications/humanres/entitydef/entitymodel.xml =================================================================== --- applications/humanres/entitydef/entitymodel.xml (revision 1652873) +++ applications/humanres/entitydef/entitymodel.xml (working copy) @@ -597,10 +597,15 @@ title= "Salary Step Entity" > <field name= "salaryStepSeqId" type= "id-ne" ></field> <field name= "payGradeId" type= "id-ne" ></field> + <field name= "fromDate" type= "date-time" ></field> + <field name= "thruDate" type= "date-time" ></field> <field name= "dateModified" type= "date-time" ></field> <field name= "amount" type= "currency-amount" ></field> + <field name= "createdByUserLogin" type= "id-vlong" ></field> + <field name= "lastModifiedByUserLogin" type= "id-vlong" ></field> <prim-key field= "salaryStepSeqId" /> <prim-key field= "payGradeId" /> + <prim-key field= "fromDate" /> <relation type= "one" fk-name= "SLRY_STP_PGRD" rel-entity-name= "PayGrade" > <key-map field-name= "payGradeId" /> </relation> Index: applications/humanres/servicedef/services.xml =================================================================== --- applications/humanres/servicedef/services.xml (revision 1652873) +++ applications/humanres/servicedef/services.xml (working copy) @@ -288,6 +288,7 @@ <permission-service service-name= "humanResManagerPermission" main-action= "CREATE" /> <auto-attributes mode= "IN" include= "nonpk" optional= " true " /> <attribute name= "payGradeId" mode= "IN" type= " String " /> + <attribute name= "fromDate" mode= "IN" type= "Timestamp" /> <attribute name= "salaryStepSeqId" mode= "OUT" type= " String " /> </service> I think the OOTB code is OK with that (it uses auto-attributes and auto-fields-entity) But this will need a comment at https://cwiki.apache.org/confluence/display/OFBIZ/Revisions+Requiring+Data+Migration+-+upgrade+ofbiz
          Hide
          pfm.smits Pierre Smits added a comment -

          Currently, the model for the entity looks like:

              <entity entity-name="SalaryStep"
                      package-name="org.ofbiz.humanres.employment"
                      title="Salary Step Entity">
                <field name="salaryStepSeqId" type="id-ne"></field>
                <field name="payGradeId" type="id-ne"></field>
                <field name="dateModified" type="date-time"></field>
                <field name="amount" type="currency-amount"></field>
                <prim-key field="salaryStepSeqId"/>
                <prim-key field="payGradeId"/>
                <relation type="one" fk-name="SLRY_STP_PGRD" rel-entity-name="PayGrade">
                  <key-map field-name="payGradeId"/>
                </relation>
              </entity>
          
          Show
          pfm.smits Pierre Smits added a comment - Currently, the model for the entity looks like: <entity entity-name= "SalaryStep" package -name= "org.ofbiz.humanres.employment" title= "Salary Step Entity" > <field name= "salaryStepSeqId" type= "id-ne" ></field> <field name= "payGradeId" type= "id-ne" ></field> <field name= "dateModified" type= "date-time" ></field> <field name= "amount" type= "currency-amount" ></field> <prim-key field= "salaryStepSeqId" /> <prim-key field= "payGradeId" /> <relation type= "one" fk-name= "SLRY_STP_PGRD" rel-entity-name= "PayGrade" > <key-map field-name= "payGradeId" /> </relation> </entity>
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          At 1st glance and from a logical perspective I agree with both of you. Now I did not look at the code and what these changes would entail.

          Show
          jacques.le.roux Jacques Le Roux added a comment - At 1st glance and from a logical perspective I agree with both of you. Now I did not look at the code and what these changes would entail.
          Hide
          pfm.smits Pierre Smits added a comment - - edited

          I can indeed imagine that salarysteps have a lifespan, but for audit purposes I would also include following elements into the entity and adjust the handling service accordingly.

          <field name="createdByUserLogin" type="id-vlong"></field>
          <field name="lastModifiedByUserLogin" type="id-vlong"></field>
          

          What do you think?

          Show
          pfm.smits Pierre Smits added a comment - - edited I can indeed imagine that salarysteps have a lifespan, but for audit purposes I would also include following elements into the entity and adjust the handling service accordingly. <field name= "createdByUserLogin" type= "id-vlong" ></field> <field name= "lastModifiedByUserLogin" type= "id-vlong" ></field> What do you think?

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development

                  Agile