Merge pull request #2770 from compucorp/staging

Sync master with staging

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,5 +1,5 @@
 ## Overview
-_A brief description of the pull request. Try to keep it non-technical. Please mark your pull request with the appropriate [label(s)](https://github.com/civicrm/civihr/blob/staging/CONTRIBUTING.md#label-types)_
+_A brief description of the pull request. Try to keep it non-technical. Please mark your pull request with the appropriate [label(s)](https://github.com/compucorp/civihr/blob/staging/CONTRIBUTING.md#label-types)_
 
 ## Before
 _The current status. Please provide screenshots or gifs ([LICEcap](http://www.cockos.com/licecap/), [SilentCast](https://github.com/colinkeenan/silentcast)) where appropriate._

Jenkinsfile

@@ -49,14 +49,7 @@ pipeline {
       steps {
         script {
           // Build site with CV Buildkit
-          sh "civibuild create ${params.CIVIHR_BUILDNAME} --type drupal-clean --civi-ver 4.7.27 --url $WEBURL --admin-pass $ADMIN_PASS"
-
-          sh """
-            cd $DRUPAL_MODULES_ROOT/civicrm
-            wget -O attachments.patch https://gist.githubusercontent.com/davialexandre/199b3ebb2c69f43c07dde0f51fb02c8b/raw/0f11edad8049c6edddd7f865c801ecba5fa4c052/attachments-4.7.27.patch
-            patch -p1 -i attachments.patch
-            rm attachments.patch
-          """
+          sh "civibuild create ${params.CIVIHR_BUILDNAME} --type drupal-clean --civi-ver 5.3.0 --url $WEBURL --admin-pass $ADMIN_PASS"
 
           // Get target and PR branches name
           def prBranch = env.CHANGE_BRANCH
@@ -73,6 +66,8 @@ pipeline {
             mergeEnvBranchInAllRepos(envBranch)
           }
 
+          applyCoreForkPatch()
+
           // The JS tests use the cv tool to find the path  of an extension.
           // For it to work, the extensions have to be installed on the site
           installCiviHRExtensions()
@@ -379,12 +374,24 @@ def testJS(hrcoreFolder, java.util.LinkedHashMap extension) {
 def listCivihrGitRepoPath() {
   return [
     [
-      'url': 'https://github.com/civicrm/civihr.git',
+      'url': 'https://github.com/compucorp/civihr.git',
       'folder': "$CIVICRM_EXT_ROOT/civihr"
     ],
     [
       'url': 'https://github.com/compucorp/civihr-tasks-assignments.git',
       'folder': "$CIVICRM_EXT_ROOT/civihr_tasks"
+    ],
+    // These are not really dependencies for the tests, but both the shoreditch
+    // and the styleguide installation is hardcoded in drush-install.sh
+    // file and if the code cannot be found in the site, the installation will
+    // fail
+    [
+      'url': 'https://github.com/compucorp/org.civicrm.shoreditch.git',
+      'folder': "$CIVICRM_EXT_ROOT/org.civicrm.shoreditch"
+    ],
+    [
+      'url': 'https://github.com/compucorp/org.civicrm.styleguide.git',
+      'folder': "$CIVICRM_EXT_ROOT/org.civicrm.styleguide"
     ]
   ]
 }
@@ -464,7 +471,7 @@ def listCivihrExtensions() {
       name: 'Sample Data',
       folder: 'uk.co.compucorp.civicrm.hrsampledata',
       hasJSTests: false,
-      hasPHPTests: true
+      hasPHPTests: false
     ],
     hremergency: [
       name: 'Emergency Contacts ',
@@ -515,3 +522,13 @@ def installCiviHRExtensions() {
     ./bin/drush-install.sh
   """
 }
+
+/**
+ * Applies changes to CiviCRM from the Compucorp fork
+ */
+def applyCoreForkPatch() {
+  sh """
+    cd ${CIVICRM_EXT_ROOT}/civihr
+    ./bin/apply-core-fork-patch.sh
+  """
+}

README.md

@@ -1,12 +1,9 @@
-CiviHR is a collection of extensions defining a human-resources application
-that runs on top of the CiviCRM platform.
+[CiviHR](https://civihr.org/) is an open source HR software solution, that runs on top of the CiviCRM platform.
 
 See also:
  * Install: [doc/INSTALL.md](doc/INSTALL.md)
- * Upgrade: [doc/UPGRADE.md](doc/UPGRADE.md)
- * Develop (install, test, etc): [doc/DEVELOP.md](doc/DEVELOP.md)
- * Wiki: http://wiki.civicrm.org/confluence/display/HR/CiviHR
- * Issues: http://issues.civicrm.org/jira/secure/Dashboard.jspa?selectPageId=11213
+ * User Guide: http://civihr-documentation.readthedocs.io/en/latest/
+ * Demo site: https://demo.civihrhosting.co.uk/
 
 We are using browserstack for testing:
 

com.civicrm.hrjobroles/CRM/Hrjobroles/API/Query/ContactHrJobRolesSelect.php

@@ -66,6 +66,7 @@ private function buildCustomQuery() {
     $customQuery = CRM_Utils_SQL_Select::from(HRJobRoles::getTableName() . ' as a');
 
     $this->addJoins($customQuery);
+    $this->addWhere($customQuery);
 
     $this->query = $this->buildSelectQuery('ContactHrJobRoles');
     $this->query->merge($customQuery);
@@ -113,6 +114,23 @@ private function addJoins(CRM_Utils_SQL_Select $query) {
     $query->join(null, $joins);
   }
 
+  /**
+   * Adds where conditions to the query.
+   * The contact_id parameter is not an actual Entity field of the HRJobRoles and
+   * if passed will actually cause an error. So here, if the contact_id is present, the
+   * condition is added to filter by this field on the job contract table and then the
+   * parameter is unset.
+   *
+   * @param \CRM_Utils_SQL_Select $customQuery
+   */
+  private function addWhere(CRM_Utils_SQL_Select $customQuery) {
+    if (!empty($this->params['contact_id'])) {
+      $conditions[] = 'jc.contact_id IN (' . implode(',' , $this->params['contact_id']) . ')';
+      $customQuery->where($conditions);
+      unset($this->params['contact_id']);
+    }
+  }
+
   /**
    * Executes the query
    *

com.civicrm.hrjobroles/CRM/Hrjobroles/Upgrader.php

@@ -80,6 +80,64 @@ public function upgrade_1005() {
 
     return TRUE;
   }
+
+  /**
+   * Deletes cost centre "other" option value if not in use
+   *
+   * @return bool
+   */
+  public function upgrade_1006() {
+    $jobRoles = civicrm_api3('HrJobRoles', 'get');
+    if ($jobRoles['count'] == 0) {
+      $this->deleteCostCentreOther();
+      return TRUE;
+    }
+
+    $otherId = $this->retrieveCostCentreOtherId();
+    if ($otherId == null) {
+      return TRUE;
+    }
+    $inUse = FALSE;
+    $roles = $jobRoles['values'];
+    $pattern = '/\|' . $otherId . '\|/';
+    foreach ($roles as $role) {
+      if (preg_match($pattern, $role['cost_center'])) {
+        $inUse = TRUE;
+        break;
+      }
+    }
+
+    if (!$inUse) {
+      $this->deleteCostCentreOther();
+    }
+
+    return TRUE;
+  }
+
+  /**
+   * Deletes cost centre option value with name "other"
+   */
+  private function deleteCostCentreOther() {
+    civicrm_api3('OptionValue', 'get', [
+      'option_group_id' => 'cost_centres',
+      'name' => 'Other',
+      'api.OptionValue.delete' => ['id' => '$value.id'],
+    ]);
+  }
+
+  /**
+   * Fetches the id of cost center "other" option value
+   *
+   * @return int
+   */
+  private function retrieveCostCentreOtherId() {
+    $result = civicrm_api3('OptionValue', 'get', [
+      'option_group_id' => 'cost_centres',
+      'name' => 'Other',
+    ]);
+
+    return isset($result['id']) ? $result['id'] : null;
+  }
 
   /**
    * Creates a navigation menu item using the API

com.civicrm.hrjobroles/api/v3/ContactHrJobRoles.php

@@ -10,6 +10,10 @@
  * @throws API_Exception
  */
 function civicrm_api3_contact_hr_job_roles_get($params) {
+  if (!empty($params['contact_id'])) {
+    $params['contact_id'] = _civicrm_api3_contact_hr_job_roles_get_contacts_from_params($params);
+  }
+
   $query = new CRM_Hrjobroles_API_Query_ContactHrJobRolesSelect($params);
 
   return civicrm_api3_create_success($query->run(), $params, 'ContactHrJobRoles', 'get');
@@ -27,3 +31,22 @@ function civicrm_api3_contact_hr_job_roles_get($params) {
 function _civicrm_api3_contact_hr_job_roles_DAO() {
   return CRM_Hrjobroles_BAO_ContactHrJobRoles::class;
 }
+
+/**
+ * Extracts the list of contactID's from the $params array
+ *
+ * @param array $params
+ *
+ * @return array
+ */
+function _civicrm_api3_contact_hr_job_roles_get_contacts_from_params($params) {
+  if (!is_array($params['contact_id'])) {
+    return [$params['contact_id']];
+  }
+
+  if (!array_key_exists('IN', $params['contact_id'])) {
+    throw new InvalidArgumentException('The contact_id parameter only supports the IN operator');
+  }
+
+  return $params['contact_id']['IN'];
+}

com.civicrm.hrjobroles/info.xml

@@ -8,8 +8,8 @@
     <author>Compucorp Ltd</author>
     <email>[email protected]</email>
   </maintainer>
-  <releaseDate>2018-06-05</releaseDate>
-  <version>1.7.7</version>
+  <releaseDate>2018-07-17</releaseDate>
+  <version>1.7.8</version>
   <develStage>stable</develStage>
   <compatibility>
     <ver>4.7</ver>

com.civicrm.hrjobroles/js/karma.conf.js

@@ -1,3 +1,5 @@
+var argv = require('yargs').argv;
+
 module.exports = function (config) {
   var civicrmPath = '../../../../../';
   var civihrPath = 'tools/extensions/civihr/';
@@ -67,6 +69,10 @@ module.exports = function (config) {
         ]
       }
     },
+    reporters: argv.reporters ? argv.reporters.split(',') : ['spec'],
+    specReporter: {
+      suppressSkipped: true
+    },
     junitReporter: {
       outputDir: extPath + 'test-reports',
       useBrowserName: false,

com.civicrm.hrjobroles/tests/phpunit/api/v3/ContactHRJobRoleTest.php

@@ -118,6 +118,64 @@ public function testTheGetActionReturnsMultipleJobRoles() {
     $this->assertEquals($expectedJobRole3, $contactJobRoles[$jobRole3['id']]);
   }
 
+  /**
+   * @expectedException CiviCRM_API3_Exception
+   * @expectedExceptionMessage The contact_id parameter only supports the IN operator
+   *
+   * @dataProvider invalidGetContactIdOperators
+   */
+  public function testGetThrowsExceptionForOperatorThatIsNotTheINOperatorForContactIDParameter($operator) {
+    civicrm_api3('ContactHrJobRoles', 'get', ['contact_id' => [$operator => [1]]]);
+  }
+
+  public function testGetDoesNotThrowExceptionWhenUsingTheINOperatorForTheContactIdParameter() {
+    $values = civicrm_api3('ContactHrJobRoles', 'get', ['contact_id' => ['IN' => [1]]]);
+
+    $this->assertEquals(0, $values['is_error']);
+  }
+
+  public function testGetDoesNotThrowExceptionWhenUsingTheEqualsToOperatorForTheContactIdParameter() {
+    $values = civicrm_api3('ContactHrJobRoles', 'get', ['contact_id' => 1]);
+
+    $this->assertEquals(0, $values['is_error']);
+  }
+
+  public function testGetReturnsJobRolesForPassedInContacts() {
+    $contact1 = ContactFabricator::fabricate();
+    $contact2 = ContactFabricator::fabricate();
+    $contact3 = ContactFabricator::fabricate();
+    $contract1 = HRJobContractFabricator::fabricate(['contact_id' => $contact1['id']]);
+    $contract2 = HRJobContractFabricator::fabricate(['contact_id' => $contact2['id']]);
+    $contract3 = HRJobContractFabricator::fabricate(['contact_id' => $contact3['id']]);
+    $jobRole1 = HRJobRolesFabricator::fabricate(['job_contract_id' => $contract1['id']]);
+    $jobRole2 = HRJobRolesFabricator::fabricate(['job_contract_id' => $contract2['id']]);
+    $jobRole3 = HRJobRolesFabricator::fabricate(['job_contract_id' => $contract3['id']]);
+
+    //Get Jobroles for contact1 and contact3
+    $contactJobRoles = civicrm_api3($this->entity, $this->action, [
+      'contact_id' => ['IN' => [$contact1['id'], $contact3['id']]]
+    ])['values'];
+
+    $this->assertCount(2, $contactJobRoles);
+
+    // The CiviCRM only returns non-empty fields. Since we only set the job
+    // contract id, only the ID and the contact_id fields will be returned
+    $expectedJobRole1 = ['id' => $jobRole1['id'], 'contact_id' => $contact1['id']];
+    $expectedJobRole3 = ['id' => $jobRole3['id'], 'contact_id' => $contact3['id']];
+
+    $this->assertEquals($expectedJobRole1, $contactJobRoles[$jobRole1['id']]);
+    $this->assertEquals($expectedJobRole3, $contactJobRoles[$jobRole3['id']]);
+
+    //Get Jobroles for contact2 only
+    $contactJobRoles = civicrm_api3($this->entity, $this->action, [
+      'contact_id' => $contact2['id']
+    ])['values'];
+
+    $this->assertCount(1, $contactJobRoles);
+    $expectedJobRole2 = ['id' => $jobRole2['id'], 'contact_id' => $contact2['id']];
+    $this->assertEquals($expectedJobRole2, $contactJobRoles[$jobRole2['id']]);
+  }
+
   private function setExpectedApiPermissionException($permission) {
     $message = "API permission check failed for {$this->entity}/{$this->action} call; insufficient permission: require {$permission}";
     $this->setExpectedException('CiviCRM_API3_Exception', $message);
@@ -127,4 +185,22 @@ private function registerCurrentLoggedInContactInSession($contactID) {
     $session = CRM_Core_Session::singleton();
     $session->set('userID', $contactID);
   }
+
+  public function invalidGetContactIdOperators() {
+    return [
+      ['>'],
+      ['>='],
+      ['<='],
+      ['<'],
+      ['<>'],
+      ['!='],
+      ['BETWEEN'],
+      ['NOT BETWEEN'],
+      ['LIKE'],
+      ['NOT LIKE'],
+      ['NOT IN'],
+      ['IS NULL'],
+      ['IS NOT NULL'],
+    ];
+  }
 }

contactaccessrights/info.xml

@@ -8,8 +8,8 @@
     <author>Compucorp Ltd</author>
     <email>[email protected]</email>
   </maintainer>
-  <releaseDate>2018-06-05</releaseDate>
-  <version>1.7.7</version>
+  <releaseDate>2018-07-17</releaseDate>
+  <version>1.7.8</version>
   <develStage>stable</develStage>
   <compatibility>
     <ver>4.7</ver>

contactaccessrights/js/karma.conf.js

@@ -1,3 +1,5 @@
+var argv = require('yargs').argv;
+
 module.exports = function (config) {
   var civicrmPath = '../../../../../';
   var civihrPath = 'tools/extensions/civihr/';
@@ -69,6 +71,10 @@ module.exports = function (config) {
         ]
       }
     },
+    reporters: argv.reporters ? argv.reporters.split(',') : ['spec'],
+    specReporter: {
+      suppressSkipped: true
+    },
     junitReporter: {
       outputDir: extPath + 'test-reports',
       useBrowserName: false,