CRM-21227 Fix test following merge of PR #10435 #11032
Conversation
| @@ -117,7 +117,7 @@ public function setDefaultValues() { | |||
| $defaults['contact_types_a'] .= '__' . $defaults['contact_sub_type_a']; | |||
| } | |||
|
|
|||
| $defaults['contact_types_b'] = $defaults['contact_type_b']; | |||
There was a problem hiding this comment.
This is just changing to be the same as L115 but for contact_type_b
| @@ -154,7 +154,7 @@ public function buildQuickForm() { | |||
| public function setDefaultValues() { | |||
| if (isset($this->_id)) { | |||
| $defaults = array_merge($this->_values, | |||
| CRM_Badge_BAO_Layout::getDecodedData($this->_values['data'])); | |||
| CRM_Badge_BAO_Layout::getDecodedData(CRM_Utils_Array::value('data', $this->_values, '[]'))); | |||
There was a problem hiding this comment.
This checks for data in $this->_values and passes an empty json_encoded array as the default value (getDecodedData does a jsonDecode
| @@ -60,8 +60,8 @@ public function preProcess() { | |||
| $this->_rgid = CRM_Utils_Request::retrieve('id', 'Positive', $this, FALSE, 0); | |||
| $contactTypes = civicrm_api3('Contact', 'getOptions', array('field' => "contact_type")); | |||
| $contactType = CRM_Utils_Request::retrieve('contact_type', 'String', $this, FALSE, 0); | |||
| if (in_array($contactType, $contactTypes['values'])) { | |||
| $this->_contactType = $contactTypes['values'][$contactType]; | |||
| if (CRM_Utils_Array::value($contactType, $contactTypes['values'])) { | |||
There was a problem hiding this comment.
Why the change here? It changes the logic from asking:
"is $contactType in the array $values"
to
"is $values[$contactType] truthy?"
There was a problem hiding this comment.
@mickadoo Michael if you look at how old L64 was crafted it was using it as a Key whereas in_array checks on the value so somewhere we stuffed up. I'm not sure if should be on the key or not
There was a problem hiding this comment.
so the effect here is that IF contactType is passed in via the URL we accept it if valid. If not we calculate from the rule.
Perhaps keep this & change lower down
Remove
elseif (!empty($contactType)) {
throw new CRM_Core_Exception('Contact Type is Not valid');
}
& replace with
if (!$this->_rgid && !$this->_contactType) {
CRM_Core_Session::setStatus('Contact Type could not be determined, assuming Individual');
$this->_contactType = 'Individual';
}
Or we could throw an exception - but I can't see why we don't just use a default
| @@ -83,8 +83,7 @@ public function preProcess() { | |||
| public function buildQuickForm() { | |||
| parent::buildQuickForm(); | |||
| $this->setPageTitle(ts('Financial Batch')); | |||
|
|
|||
| if (isset($this->_id)) { | |||
| if (!empty($this->_id)) { | |||
There was a problem hiding this comment.
this is fine - although I think it would be better to define the property on the class
|
Jenkins, please test this |
|
Jenkins, test this please |
|
I'm going to merge this because it does fix some tests & I don't think the question about contactTypes is a blocker |
Overview
Test has started to fail i believe due to PR #10435 see https://test.civicrm.org/job/CiviCRM-Core-PR/17314/testReport/(root)/CRM_Core_Page_HookTest/testFormsCallBuildFormOnce/ as an example
Before
Test fails
After
Test passes
Technical Details
The PR made some changes to how
$defaultsis populated from requests which has made some cascading effectsping @eileenmcnaughton @monishdeb