PCHR-3459: Add Tool Icon to Edit Job Contract Options #2620

ajesamson · 2018-05-24T12:28:19Z

Overview

Provision for editing options while creating record is not available for some Job Contract options. This PR add such facility for Contract Type, Normal Place of Work, Contract End Reason, Revision Change Reason, Location/Standard hours, Pay Scale/Grade, Annual Benefit, Annual Deductions and Plan Type.

Before

After

Technical Details

Tool icon is added next to the input for the specified options. Each tool icon makes a call to the respective controller method for launching and managing associated options.

<div class="col-sm-1 control-label">
  <a class="pointer">
      <i class="crm-i fa-wrench" ng-click="openOptionsEditor('/civicrm/pay_scale?reset=1', 'hrjobcontract_details_contract_type')"></i>
  </a>
</div>

Sample implementation of the controller method (such as openOptionsEditor) is shown below:

function openOptionsEditor (optionUrl, fieldName) {
    var optionTypes = {
      'hrjobcontract_details_contract_type': 'contract_type',
      'hrjobcontract_details_location': 'location',
      'hrjobcontract_details_end_reason': 'end_reason'
    };

    crmAngService.loadForm(optionUrl)
      .on('crmUnload', function () {
        contractDetailsService.getOptions(fieldName, true)
          .then(function (result) {
            $rootScope.options.details[optionTypes[fieldName]] = result.obj;
          });
      });
}

Each crmAngService loadForm method includes a callback to update the dropdown list.

Some other changes were made on Location/Standard hours and Pay Scale / Grade to allow for proper api update via ajax calls. Their templates use data-field value title instead of location and pay_scale respectively.

<tr id="HRPayScale-{$row.id}" class="crm-entity {cycle values="odd-row,even-row"} {$row.class}{if $row.is_active neq 1} disabled{/if}">
  <td class="crm-editable" data-field="pay_scale">{$row.pay_scale}</td>
  <td>{$row.currency}</td>
  <td>{$row.amount}</td>
  <td>{$row.pay_frequency}</td>
  <td id="row_{$row.id}_status">{if $row.is_active eq 1} {ts}Yes{/ts} {else} {ts}No{/ts} {/if}</td>
  <td>{$row.action|replace:'xx':$row.id}</td>
</tr>

Call to API was added for some of the callbacks to have updated changes reflecting.

CRM.api3('HRJobDetails', 'getoptions', {
   'sequential': 1,
   'field': fieldName
}).done(function (data) {
   _.each(data.values, function (option) {
       contractTypes.arr.push({
         key: option.key,
         value: option.value
      });

      contractTypes.obj[option.key] = option.value;
});

The snippet below was removed due to errors when initializing inline edit because of $ being out of scope.

<script type="text/javascript">
    CRM.$('.crm-editable').crmEditable();
  </script>

The inline edit works without this implementation.

✅Manual Tests - passed
✅Jasmine Tests - passed
✅Backstopjs Test - passed
✅JS distributive files - included

igorpavlov-zz · 2018-05-24T13:29:12Z

@ajesamson please remove the : from both the PR title and the commit message

igorpavlov-zz · 2018-05-24T13:30:26Z

hrjobcontract/js/src/job-contract/controllers/modal/modal-change-reason.controller.js

@@ -29,6 +29,7 @@ define([
    $scope.cancel = cancel;
    $scope.dpOpen = dpOpen;
    $scope.save = save;
+    $scope.openRevisionChangeReasonEditor = openRevisionChangeReasonEditor;


please sort aplhabetically

igorpavlov-zz · 2018-05-24T13:34:51Z

hrjobcontract/js/src/job-contract/controllers/modal/modal-contract-new.controller.js

@@ -78,6 +78,11 @@ define([
    $scope.cancel = cancel;
    $scope.filesValidate = filesValidate;
    $scope.save = save;
+    $scope.openOptionsEditor = openOptionsEditor;


Please sort alphabetically.

igorpavlov-zz · 2018-05-24T13:35:11Z

hrjobcontract/js/src/job-contract/controllers/modal/modal-contract-new.controller.js

+    $scope.openHoursLocationOptionsEditor = openHoursLocationOptionsEditor;
+    $scope.openPayScaleGradeOptionsEditor = openPayScaleGradeOptionsEditor;
+    $scope.openAnnualBenefitOptionsEditor = openAnnualBenefitOptionsEditor;
+    $scope.openAnnualDeductionOptionsEditor = openAnnualDeductionOptionsEditor;

    // Init


Please remove that comment, we don't need it as per styleguide

igorpavlov-zz · 2018-05-24T13:35:59Z

hrjobcontract/js/src/job-contract/controllers/modal/modal-contract-new.controller.js

+    /**
+     * Opens option editor window for contract type, location, end reason or insurance
+     *
+     * @param optionUrl


The param types are forgotten:

* @param {String} optionUrl * @param {String} fieldName

igorpavlov-zz · 2018-05-24T13:45:08Z

hrjobcontract/js/src/job-contract/controllers/modal/modal-contract-new.controller.js

+          if (fieldName === 'hrjobcontract_health_health_plan_type') {
+            contractHealthService.getOptions(fieldName, true)
+              .then(function (data) {
+                var healthOptions = {};


Please use Lodash here:

_.mapValues(_.indexBy([{key: 1, value: 'one'},{key: 2, value: 'two'}], 'key'), 'value') >>> {1: "one", 2: "two"}

So it would be smth like:

var healthOptions = _.mapValues(_.indexBy(data, 'key'), 'value');

Also, between var x = ... and other types of expressions, like function calls, please leave an empty line break.

igorpavlov-zz · 2018-05-24T13:50:08Z

hrjobcontract/js/src/job-contract/controllers/modal/modal-contract.controller.js

+    function loadAnnualPayOptions (optionType, optionName) {
+      return OptionGroup.valuesOf(optionType, false)
+        .then(function (data) {
+          var options = {};


Please use lodash

igorpavlov-zz · 2018-05-24T13:51:16Z

hrjobcontract/js/src/job-contract/services/contract-pay.service.js

+            'sequential': 1,
+            'field': fieldName
+          }).done(function (data) {
+            _.each(data.values, function (option) {


can we user lodash's map to create an array and object generation as suggested above?

igorpavlov-zz · 2018-05-24T13:53:32Z

hrjobcontract/js/test/controllers/modal/modal-contract-new.controller.spec.js

@@ -86,6 +89,61 @@ define([
      });
    });

+    describe('when user clicks on the wrench icon', function () {
+      locationUrl = '/civicrm/hours_location?reset=1';


Should we split this into multiple suites? One test is just too large and it is a bit difficult to follow what's going on here.

There should be multiple describes, smth like when user clicks on the "hours location" wrench icon. etc

igorpavlov-zz · 2018-05-24T13:53:53Z

hrjobcontract/js/test/controllers/modal/modal-contract.controller.spec.js

@@ -457,6 +459,61 @@ define([
      });
    });

+    describe('when user clicks on the wrench icon', function () {
+      locationUrl = '/civicrm/hours_location?reset=1';


Same as above

igorpavlov-zz · 2018-05-24T13:56:22Z

hrjobcontract/templates/CRM/Hrjobcontract/Page/HoursLocation.tpl

@@ -62,9 +62,6 @@
        {/if}
    </div>
  </div>
-  <script type="text/javascript">


Please leave a description comment in the Tech description of the PR on why are you making this change.

Tech description has been updated.

igorpavlov-zz · 2018-05-25T11:11:35Z

hrjobcontract/js/src/job-contract/controllers/modal/modal-contract-new.controller.js

-     * @param optionName
-     * @returns {*}
+     * @param {String} optionType
+     * @param {String} optionName


Wait, why do we remove the return here? Please leave it how it was, but in the documentation it should say what it returns. If it is a {Promise}, then:

```* @return {Promise} resolved with ....`

Same for other places.

ajesamson added the ready to test label May 24, 2018

ajesamson changed the title ~~:PCHR-3459: Add Tool Icon to Edit Job Contract Options~~ PCHR-3459: Add Tool Icon to Edit Job Contract Options May 24, 2018

igorpavlov-zz suggested changes May 24, 2018

View reviewed changes

igorpavlov-zz suggested changes May 25, 2018

View reviewed changes

ajesamson force-pushed the PCHR-3459-add-tool-icon-to-edit-job-contract-options branch from cf3cf7a to 0e83888 Compare May 25, 2018 11:30

igorpavlov-zz approved these changes May 25, 2018

View reviewed changes

ajesamson force-pushed the PCHR-3459-add-tool-icon-to-edit-job-contract-options branch 3 times, most recently from c2e4a76 to 06ed650 Compare May 25, 2018 13:56

igorpavlov-zz approved these changes May 25, 2018

View reviewed changes

PCHR-3459: Add tool icon to edit job contract options

ca4980d

ajesamson force-pushed the PCHR-3459-add-tool-icon-to-edit-job-contract-options branch from 409b296 to ca4980d Compare May 25, 2018 14:40

igorpavlov-zz approved these changes May 25, 2018

View reviewed changes

ajesamson merged commit 2c4921e into PCHR-3162-configurability-changes May 25, 2018

ajesamson deleted the PCHR-3459-add-tool-icon-to-edit-job-contract-options branch May 25, 2018 15:18

ajesamson mentioned this pull request Jun 4, 2018

PCHR-3162: Configurability Changes - Tool Icon Integration #2654

Merged

davialexandre mentioned this pull request Jun 4, 2018

Sync master with staging #2662

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PCHR-3459: Add Tool Icon to Edit Job Contract Options #2620

PCHR-3459: Add Tool Icon to Edit Job Contract Options #2620

ajesamson commented May 24, 2018 •

edited

Loading

igorpavlov-zz commented May 24, 2018 •

edited

Loading

igorpavlov-zz May 24, 2018

igorpavlov-zz May 24, 2018

igorpavlov-zz May 24, 2018

igorpavlov-zz May 24, 2018

igorpavlov-zz May 24, 2018

igorpavlov-zz May 24, 2018

igorpavlov-zz May 24, 2018

igorpavlov-zz May 24, 2018

igorpavlov-zz May 24, 2018

igorpavlov-zz May 24, 2018

ajesamson May 25, 2018

igorpavlov-zz May 25, 2018

PCHR-3459: Add Tool Icon to Edit Job Contract Options #2620

PCHR-3459: Add Tool Icon to Edit Job Contract Options #2620

Conversation

ajesamson commented May 24, 2018 • edited Loading

Overview

Before

After

Technical Details

igorpavlov-zz commented May 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajesamson commented May 24, 2018 •

edited

Loading

igorpavlov-zz commented May 24, 2018 •

edited

Loading