Skip to content

Commit

Permalink
Merge pull request #13 from grnhse/required_multiselect_bug
Browse files Browse the repository at this point in the history
Fixes a bug in ApplicationService where the required field validator checks for the existence of key[] as sent by Greenhouse, but submission requires you to use key, thus never being able to pass a required multiselect field to an application.
  • Loading branch information
tdphillipsjr authored Aug 31, 2017
2 parents 79b50e7 + e3129e9 commit 6f3c9b3
Show file tree
Hide file tree
Showing 3 changed files with 218 additions and 1 deletion.
25 changes: 24 additions & 1 deletion src/Services/ApplicationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,35 @@ public function validateRequiredFields($postVars)
public function hasRequiredValue($postVars, $keys)
{
foreach ($keys as $key) {
if (array_key_exists($key, $postVars) && $postVars[$key] !== '') return true;
$requiredKey = $this->findKey($key, $postVars);
if (array_key_exists($requiredKey, $postVars) && $postVars[$requiredKey] !== '') return true;
}

return false;
}

/**
* This fixes an issue where the Greenhouse API returns required multiselects as <field[]> but we require
* the user to submit it as <field>, thus making the system think the required field is not set. This
* checks that either <field> or <field[]> is set, in order to work correctly but also be backward
* compatable in case external systems have already corrected for it.
*
* @params Array $key The key to check for in $postVars
* @params Array $postVars Greenhouse post parameters.
* @return Mixed string/null
*/
public function findKey($key, $postVars)
{
$multiselectKey = str_replace('[]', '', $key);
if (array_key_exists($key, $postVars)) {
return $key;
} else if (array_key_exists($multiselectKey, $postVars)) {
return $multiselectKey;
} else {
return null;
}
}

/**
* Given a job id, make a requrest to the Greenhouse API for those questions and build an associative
* array indexed on the human-readable name containing an array of the indices that must be set. The
Expand Down
54 changes: 54 additions & 0 deletions tests/Services/ApplicationServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ public function getTestJobJson()
return file_get_contents("$root/../files/test_json/single_job_response.json");
}

public function getTestJobJsonWithMultiselect()
{
$root = realpath(dirname(__FILE__));
return file_get_contents("$root/../files/test_json/single_job_response_with_multiselect_required.json");
}

public function getTestJobJsonNothingRequired()
{
$root = realpath(dirname(__FILE__));
Expand Down Expand Up @@ -77,6 +83,54 @@ public function testValidateRequiredFieldsPass()
$this->assertTrue($this->appService->validateRequiredFields($postVars));
}

public function testValidateRequiredFieldsWithMultiselect()
{
$apiStub = $this->getMockBuilder('\Greenhouse\GreenhouseToolsPhp\Services\JobApiService')
->disableOriginalConstructor()
->getMock();
$apiStub->method('getJob')->willReturn($this->getTestJobJsonWithMultiselect());
$this->appService->setJobApiService($apiStub);

$postVars = array(
'id' => '12345',
'first_name' => 'Hiram',
'last_name' => 'Abiff',
'email' => '[email protected]',
'resume_text' => 'Builder',
'cover_letter_text' => 'I built things',
'question_1042159' => 'stuff',
'question_579460' => array(225451, 225452)
);

$this->assertTrue($this->appService->validateRequiredFields($postVars));
}

/**
* Backward compatable version. This is how we checked required fields originally (with the square
* brackets, as returned by the API, but not how we required the request to be submitted.
*/
public function testValidateRequiredFieldsWithMultiselectLegacy()
{
$apiStub = $this->getMockBuilder('\Greenhouse\GreenhouseToolsPhp\Services\JobApiService')
->disableOriginalConstructor()
->getMock();
$apiStub->method('getJob')->willReturn($this->getTestJobJsonWithMultiselect());
$this->appService->setJobApiService($apiStub);

$postVars = array(
'id' => '12345',
'first_name' => 'Hiram',
'last_name' => 'Abiff',
'email' => '[email protected]',
'resume_text' => 'Builder',
'cover_letter_text' => 'I built things',
'question_1042159' => 'stuff',
'question_579460[]' => array(225451, 225452)
);

$this->assertTrue($this->appService->validateRequiredFields($postVars));
}

public function testValidateRequiredFieldsFailSingle()
{
$apiStub = $this->getMockBuilder('\Greenhouse\GreenhouseToolsPhp\Services\JobApiService')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
{
"id": 167538,
"internal_job_id": 187771,
"title": "Controller",
"updated_at": "2016-03-21T15:53:06-04:00",
"location": {
"name": "New York"
},
"absolute_url": "http://www.greenhouse.io/careers/job?gh_jid=167538",
"metadata": null,
"content": "test",
"departments": [{
"id": 7219,
"name": "Finance"
}],
"offices": [{
"id": 13,
"name": "New York",
"location": "Manhattan, NY 10007"
}],
"questions": [{
"label": "First Name",
"fields": [{
"name": "first_name",
"type": "input_text"
}],
"required": true
}, {
"label": "Last Name",
"fields": [{
"name": "last_name",
"type": "input_text"
}],
"required": true
}, {
"label": "Email",
"fields": [{
"name": "email",
"type": "input_text"
}],
"required": true
}, {
"label": "Phone",
"fields": [{
"name": "phone",
"type": "input_text"
}],
"required": false
}, {
"label": "Resume",
"fields": [{
"name": "resume",
"type": "input_file"
}, {
"name": "resume_text",
"type": "textarea"
}],
"required": true
}, {
"label": "Cover Letter",
"fields": [{
"name": "cover_letter",
"type": "input_file"
}, {
"name": "cover_letter_text",
"type": "textarea"
}],
"required": true
}, {
"label": "LinkedIn Profile",
"fields": [{
"name": "question_1042159",
"type": "input_text"
}],
"required": true
}, {
"label": "How did you hear about this job?",
"fields": [{
"name": "question_1042161",
"type": "input_text"
}],
"required": false
}, {
"label": "Are you authorized to work in the United States?",
"fields": [{
"name": "question_1042162",
"type": "multi_value_single_select",
"values": [{
"label": "No",
"value": 0
}, {
"label": "Yes",
"value": 1
}]
}],
"required": false
}, {
"label": "Will you now or in the future require visa sponsorship?",
"fields": [{
"name": "question_1042163",
"type": "multi_value_single_select",
"values": [{
"label": "No",
"value": 0
}, {
"label": "Yes",
"value": 1
}]
}],
"required": false
}, {
"label": "Do you have the right to work in the UK?",
"required": true,
"fields": [
{
"name": "question_579460[]",
"type": "multi_value_multi_select",
"values": [
{
"label": "UK Passport",
"value": 225451
},
{
"label": "EEA Passport",
"value": 225452
},
{
"label": "Other visa",
"value": 225453
},
{
"label": "No",
"value": 225454
}
]
}
]
}],
"compliance": null
}

0 comments on commit 6f3c9b3

Please sign in to comment.