dev/core#1588 Fix regression where empty string is passed to SettingsBag
authoreileen <emcnaughton@wikimedia.org>
Fri, 14 Feb 2020 01:50:18 +0000 (14:50 +1300)
committereileen <emcnaughton@wikimedia.org>
Fri, 14 Feb 2020 01:50:18 +0000 (14:50 +1300)
We have a scenario where the checkbox is presented but is optional. This is then in 'params' which is passed through to
PropertyBag. The value is equal to '' so it fails to validate when we use it to set the value on the PropertyBag.

I don't think we lose anything meaningful by not setting an empty string and it avoids this error so we should
merge & release as a regression fix IMHO. If we want to revist then we should do that in master

https://lab.civicrm.org/dev/core/issues/1588

CRM/Contribute/Form/Contribution/Confirm.php
Civi/Payment/PropertyBag.php

index 4d9534b8114d2e6f2888cb640370ddfd62f479d3..a71a646027ba54f82c1b62fa12c5b86553aa33f1 100644 (file)
@@ -2020,6 +2020,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
    * @param int $contactID
    *
    * @return array
+   * @throws \CRM_Core_Exception
    */
   protected function processFormSubmission($contactID) {
     if (!isset($this->_params['payment_processor_id'])) {
index afdee0abb6908da4660426ea60a30a8cbfd84ce5..cb9a6a0fe8c2918ea328fa8a3db9721a4a8eaa51 100644 (file)
@@ -278,9 +278,9 @@ class PropertyBag implements \ArrayAccess {
    * @param array $data
    */
   public function mergeLegacyInputParams($data) {
-    $this->legacyWarning("We have merged input params into the property bag for now but please rewrite code to not use this.");
+    $this->legacyWarning('We have merged input params into the property bag for now but please rewrite code to not use this.');
     foreach ($data as $key => $value) {
-      if ($value !== NULL) {
+      if ($value !== NULL && $value !== '') {
         $this->offsetSet($key, $value);
       }
     }