CRM-18504 validateSubTypeByEntity checks value instead of key (Fatal error after 4.7.7 upgrade)

    Details

    • Type: Bug
    • Status: Done/Fixed
    • Priority: Critical
    • Resolution: Fixed/Completed
    • Affects Version/s: 4.6.16, 4.7.7
    • Fix Version/s: 4.6.17, 4.7.8
    • Component/s: None
    • Labels:
    • Documentation Required?:
      None
    • Funding Source:
      Contributed Code

      Description

      We're experiencing an issue after updating to 4.7.7

      We're hitting a fatal error in the method CRM_Core_BAO_CustomGroup::validateSubTypeByEntity
      https://github.com/civicrm/civicrm-core/blob/6b85570b47e529d1d728e972ece3b95d9330009b/CRM/Core/BAO/CustomGroup.php#L655

      This function is looking for the entity type in an array:
      if ($entityType != 'Contact' && !in_array($entityType, $contactTypes['values'])) {

      as a value when it should be looking for the key.

      In our case we have 'Organisation' not 'Organization' so the array looks like:

      Organization => Organisation

      So in_array isn't working.

      PR forthcoming to use array_key_exists instead

        Attachments

          Activity

          [CRM-18504] validateSubTypeByEntity checks value instead of key (Fatal error after 4.7.7 upgrade)
          John K. added a comment - - edited

          Steps to recreate on master

          • Make a contact a 'sub-type' of 'Individual', eg: staff
          • Rename the 'Individual' contact type to 'Multividual'
          • View the 'staff' contact record
          • Note the fatal error
          • Apply the patch and rejoice
          John K. added a comment -

          Updated title to help prevent duplicate issues.

          Dave Jenkins added a comment -

          Same issue encountered following an upgrade from 4.6.14 to 4.6.16 . As John found, the error occurred for a custom field set that extends a contact subtype, where the parent contact type (Household in this case) had been renamed, so name != label, so value != key in $contactTypes.

          Patch from PR 8301 applied cleanly to 4.6.16 and fixed the problem - thanks for this.

          Peter Davis added a comment -

          I tried the patch but still had the errors on drush cc.

          I changed out the subtype to match what was in custom_field table and still had errors on drush cc

          Then reversed the patch - same errors.

          Then hit civicrm/upgrade which seemed to do the job so not sure if patch would have avoided the renaming of the subtype or not.

          Cord Slatton-Valle added a comment -

          I was able to use Chris Burgess' github commits from above to fix the issue when upgrading from 4.6.15 to 4.6.16, they worked perfectly as far as I can tell to resolve the issues. Thanks Chris.

          Chris Burgess added a comment -

          Thanks for testing! I intend to test more thoroughly & look at this for 4.7 today. Note also CRM-18510 which is new in 4.7.7 / 4.6.16

          Dave Jenkins added a comment -

          Hi Chris Burgess,

          Re this comment in https://github.com/fuzionnz/civicrm-core/commit/a0bcc16171d2f30c4416aeec079bab56c313390a :

          // @TODO: LIKE %foo% here means contact type 'Contact'
          // incorrectly matches a subtype of 'Media_Contact'?
          //
          // Is this the intended behaviour?

          civicrm_custom_group.extends_entity_column_value is a CHAR(1) delimited set of values. So I think it wants to use something like:

          civicrm_custom_group.extends_entity_column_value LIKE CONCAT('%', CHAR(1), $subType, CHAR(1), '%')
          [untested code]

          One question is whether this field is reliably CHAR(1) delimited in all cases, including single values in installations that started life on an ancient Civi version. In two installations I've just checked, they all are: one of these has been running since at least 4.1 (probably long before) & has 40 custom field sets, the other running since 1.5 & having 127 CF sets.

          Dave Jenkins added a comment - - edited
          Yur G added a comment -

          Have commented via github at https://github.com/civicrm/civicrm-core/pull/8301, repeating it here, not sure where would be an appropriate place. So, sorry, it still throws an error if there are deleted contact sub-types, in our case it looks like:
          "Rejected "Young_Arts_Group_Member" as not a valid subtype of "Individual"." and we have no "Young Art Group Member" contact sub-type anymore.

          Chris Burgess added a comment -

          Sharing WIP on this: CRM-18504 in Fuzion repo.

          Dave Jenkins added a comment - - edited

          Confirm that 4.6.16 + PR 8301 gives errors when there is an enabled custom field set that extends a disabled contact subtype.

          Also a new regression found on some 4.6.16 sites: several custom field groups incorrectly showing on e.g. create Meeting activity form. They extend specified activity types not including Meeting. Fixed by this:

          --- CustomGroup.php.PR8301+preg	2016-05-13 14:30:19.000000000 +0100
          +++ CustomGroup.php	2016-05-13 18:01:40.000000000 +0100
          @@ -433,7 +433,9 @@
           
               if (!empty($subTypes)) {
                 foreach ($subTypes as $key => $subType) {
          -        $subTypeClauses[] = "civicrm_custom_group.extends_entity_column_value LIKE '%" . self::validateSubTypeByEntity($entityType, $subType) . "%'";
          +        // Fix regression from 4.6.16 fixes: the value returned from validateSubTypeByEntity does
          +        // not include value separators, so they need to be added for the query.
          +        $subTypeClauses[] = "civicrm_custom_group.extends_entity_column_value LIKE '%" . CRM_Core_DAO::VALUE_SEPARATOR . self::validateSubTypeByEntity($entityType, $subType) . CRM_Core_DAO::VALUE_SEPARATOR . "%'";
                 }
                 $subTypeClause = '(' .  implode(' OR ', $subTypeClauses) . ')';
                 if (!$onlySubType) {
          
          

          The issue seems to be the LIKE matching sloppily: haven't pinned it down to a reproducible test case yet but you can see how it will go wrong. The value returned by validateSubTypeByEntity has value separators stripped, so e.g. if $subType is '1' then we're then doing this match:

          civicrm_custom_group.extends_entity_column_value LIKE '%1%'

          which will match e.g. 1, 10, 11, 12, ..., 21, ..., 31, ...

          [Edit] Created CRM-18559 to track this new issue.

          Yur G added a comment -

          What's the current status of this one? It continues to stabs us in the back here and there at contact add / edit forms.

          Dave Jenkins added a comment -

          Yur G Re contact add / edit forms, does this just affect contacts with more than one contact subtype? I've created issue CRM-18567 for that.

          Yur G added a comment -

          @Dave Jenkins : Precisely, "contacts with more than one contact subtype" it's the case; we can't edit inline nor via normal edit form, custom fields get lost. Wasn't aware of new issue, going to follow it.

          Yashodha Chaku added a comment - - edited
          Dave Jenkins added a comment -

          I posted a summary of validateSubTypeByEntity - related issues here: https://github.com/civicrm/civicrm-core/pull/8301#issuecomment-219741154 - adding link here for reference.

          Yashodha Chaku added a comment -

          PR's for both master and 4.6 have been merged.

          Dave Jenkins added a comment - - edited

          Found another variant of the validateSubTypeByEntity issues, affecting a 4.6.18 site. It occurs when an enabled custom field set extends a non-existent contact subtype: I'm assuming the subtype was deleted. This results in fatal errors across the site, as for the variants that affected 4.6.16; I think the code path may involve Drupal Views. The site in question is multi-lingual but I'm not sure this is relevant.

          CRM_Core_Exception: Invalid Filter in CRM_Core_BAO_CustomGroup::validateSubTypeByEntity() (line 647 of .../sites/all/modules/civicrm/CRM/Core/BAO/CustomGroup.php).

          Probably some checking or housekeeping should be done when a subtype is deleted or disabled, to avoid orphan entries in civicrm_custom_group.extends_entity_column_value .
          [Edit:] I've tested on 4.6.18 and on current Civi 4.7 demo and it disallows deleting the subtype:
          "Selected contact type can not be deleted. Make sure contact type doesn't have any associated custom data or group."
          So I guess it was deleted in an earlier version before this check was added.

          In better news, I verified that an enabled custom field set that extends a disabled contact subtype, which caused fatal errors in 4.6.16, does not in 4.6.18 .

          Peter Davis added a comment -

          The first site that died with 4.6.16 just died with 4.6.18 and started spitting out

          nvalid Filter in CRM_Core_BAO_CustomGroup::validateSubTypeByEntity() (line 651 of //sites/all/modules/civicrm/CRM/Core/BAO/CustomGroup.php).

          Dave Jenkins added a comment -

          Peter Davis I tracked down the above issue by putting a debugging statement in CustomGroup.php to print $subType and $subTypes before throwing the exception. This told me the problematic subType that wasn't in $subTypes.

          Seamus Lee added a comment -

          So in my mind it seems that it is correct for it to be failing validation as we can't just let it go through. I guess the question should be how hard should it fail

          Dave Jenkins added a comment -

          My colleague Nathan has found a new variant of validateSubTypeByEntity problem, filed as CRM-18970, "validateSubTypeByEntity breaks adding additional Multi-record custom values".

          Jesper Angelo added a comment -

          I just upgrade an installation directly to 4.7.8 from 4.6.18 and I get

          The website encountered an unexpected error. Please try again later.
          1 array ( '%type' => 'CRM_Core_Exception', '!message' => 'Invalid Entity Filter', '%function' => 'CRM_Core_BAO_CustomGroup::validateSubTypeByEntity()', '%file' => '/home1/ivingisl/public_html/sites/all/modules/civicrm/CRM/Core/BAO/CustomGroup.php', '%line' => 657, 'severity_level' => 3, )

          Is this somehow related? I thought it was supposed to be fixed in 4.7.8?

          Yashodha Chaku added a comment -

          Could you please replicate this on http://dmaster.demo.civicrm.org/
          If yes, please provide steps to replicate as well

          Peter Davis added a comment -

          adding my lastest understanding of 2 scenarios where this error can bite us.

          a/ if there are custom group fields set up for an entity (eg CiviCase) that has now been disabled. Can be fixed through UI by deleting the custom fields and then the custom group (this was the issue in the case where we were getting the 'views' cache error version of this problem)

          b/ if there are custom group fields have been set up to 'extend' a contact subtype, but that subtype has been renamed, leaving 'bad' data in the civicrm_custom_group.extends_entity_column_value. Can be fixed through UI finding the Custom Groups that should be extending a subtype, and resetting them to the new name of the subtype. Note the 'extends' shows nothing on the Custom Group summary page. You probably need to go look at whether the subtype 'names' are still in alignment in the civicrm_custom_group table and the civicrm_contact_type table.

          As DJ has said above, there needs to be some cleaning up in civicrm_custom_group which I think needs to occur whenever

          • a component which had custom_group set up for it is disabled
          • a subtype which had custom_group set up for it is deleted
          • a subtype which had custom_group set up for itis renamed
            otherwise the data in the civicrm_custom_group.extends_entity_column_group data become orphaned and triggers these types of errors.
          Peter Davis added a comment -

          Perhaps i have missed something, but i thought i would verify behaviour on d46 demo site. I renamed subcontacttype from Volunteer to Helper, the custom group of fields that was associated with 'Volunteer' is now listed as 'Helper' so it seems fine on there. Maybe this is new since 4.6.16 and it is just an issue for us upgrading sites that had a mismatch in the naming? (and i see this ticket has 4.7.7 in title but discusses both 4.7.x and 4.6.x)

          Seamus Lee added a comment -

          Peter Davis I think the issue with the disabled contact Sub Type was dealt with this commit and similar one for master https://github.com/civicrm/civicrm-core/commit/c705b2f4c84351a375949ebaaf83ed2727904eef

          The renaming thing i'm not sure but maybe also gets caught by that commit as well

          Peter Davis added a comment -

          thanks Seamus Lee but am unclear what that means in terms of version it was released in since this seems to be biting us still. does the big red cross mean anything at https://github.com/civicrm/civicrm-core/pull/8412/commits

          Chris Burgess added a comment - - edited

          That commit went into 4.6 branch and was released in 4.6.17 there.

          Jesper Angelo added a comment -

          Still got the error in 4.7.10. Anyone got a solution?

          The website encountered an unexpected error. Please try again later.
          1 array ( '%type' => 'CRM_Core_Exception', '!message' => 'Invalid Entity Filter', '%function' => 'CRM_Core_BAO_CustomGroup::validateSubTypeByEntity()', '%file' => '/home1/ivingisl/public_html/sites/all/modules/civicrm/CRM/Core/BAO/CustomGroup.php', '%line' => 658, 'severity_level' => 3, )

          Chris Burgess added a comment -

          Saw this again on a 4.6 site today. A couple of notes on it

          • The code in validateSubTypeByEntity() is different in 4.6 and 4.7, but the 4.7 version seemed to work OK on 4.6 codebase
          • The exception messages "Invalid Entity" and "Invalid Entity Filter" feel quite oblique - if it said "Invalid entity type 'Volunteer'" then admins would have a better chance of identifying that it's related to disabling CiviVolunteer (for example)
          • The function feels messy, accepts either value-separated integers or a string, function docs say string but returns numeric subtypes without validation? Can entity types with a numeric ID not be extended or something?
          • Possibly some odd behaviours arising from that validation (eg subtypes named unlikely things like "42", "0xae", "8.123" will be legit subtypes in any case?)
          • Code comment indicates this was maybe not meant to stay in anyway - "go with this for RC release"?
          Peter Davis added a comment -

          Adding my 'observational' rather than 'analytical' comments here. In both sites that failed recently there was a row in civicrm_custom_group.extends_entity_column_value that had an empty (not NULL) value.

          In the first scenario, the cg concerned was CiviVolunteer which extends Activity. i changed the empty to NULL and bingo drush cc menu ran and site was back up.

          In second scenario the entity was Relationship. changing to NULL did not seem to trigger the fix but changing it to a valid value (21 in my case) got the site back up.

            People

            • Assignee:
              Yashodha Chaku
              Reporter:
              John K.

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 30 minutes
                30m