search

progressivetech / net.ourpowerbase.sumfields

The Summary Fields extension creates read-only custom data fields that extend Contact and are automatically populated with up-to-date totals such as total lifetime contributions, last contribution amount, last attended event, etc.
Other
8 stars 29 forks source link

Triggers cause fatal errors if site admin deletes custom fields/group #105

Open twomice opened 6 months ago

twomice commented 6 months ago

Summary:

CiviCRM allows permissioned users to delete custom fields, including those in the Summary Fields group, as well as the Summary Fields custom field group itself.

If this happens, triggers that reference those deleted custom fields will cause fatal errors, which will cause related actions in CiviCRM to fail as well. E.g., cannot record a contribution if "Total Lifetime Contributions" Summary Field is a) enabled, and b) has had its custom field deleted by a site admin.

(Disclaimer: Naturally, nobody should be deleting these custom fields, but they can and sometimes do.)

Repro steps:

  1. Install Summary Fields.
  2. Enable any summary field (in this example I'll use Total Lifetime Contributions)
  3. Perform an action that will trigger a change on that summary field (e.g. create a new contribution), and observe that the summary field is behaving normally (e.g. is updated to reflect that new contribution)
  4. Manually delete the custom field (e.g. Summary Fields :: Total Lifetime Contributions.)
  5. Attempt to repeat the action in step 3 and observe a fatal error along the lines of "DB Error: No such field".

Failed attemps to workaround:

I was hopeful that this situation could be resolved by doing one or more of the following:

However, these steps don't resolve the problem, and indeed some of them (re-anabling Summary Fields after uninstalling it) are not possible because of fatal errors due to the missing custom fields/group.

Actual tedious workaround:

  1. As far as possible, take note of all existing Summary Fields configurations at [drupal.example.org]/civicrm/admin/setting/sumfields
  2. Uninstall the sumfields extension (and thus, jsumfields extension if it exists)
  3. Examine the file [sumfields-extension-dir]/settings/sumfields.setting.php and note the names of all settings defined therein (e.g. active_fields)
  4. Via SQL, delete all rows in civicrm_settings where name is one of those keys. (This steps is required because sumfields does not remove settings upon uninstall; then, upon enable, it references those settings, some of which may point to SQL tables/columns which have been deleted per the "repro steps" above.)
  5. Re-enable the sumfields extension (and jusumfields extension if it's needed).
  6. Visit the Summary Fields configuration page at [drupal.example.org]/civicrm/admin/setting/sumfields and re-configure everything according to the original settings that you took note of in step 2.
  7. Everything should be working fine now.

Suggestions for improvement in sumfields extension:

Some ways this extension might deal with the problem of admin users deleting Summary Fields custom fields/group might be:

twomice commented 6 months ago

Original discussion in jsumfields issue queue: https://github.com/twomice/com.joineryhq.jsumfields/issues/25#issuecomment-2005150863

agileware-justin commented 6 months ago

Thanks @twomice legend!

jmcclelland commented 6 months ago

Thanks @twomice for the bug report! sumfields is an old extension and is definitely in need of some love.

I replicated the problem without much trouble. But, I was able to rectify it by simply disabling and then uninstalling sumfields (I received an error about failing to delete the custom field I manually deleted, but it nonetheless completed). Then I enabled and it seems to work properly. However, I have enough experience with sync'ing fields to know that I probably got lucky :).

I think the best and easiest option is to make the custom group "reserved" - that should result in it disappearing from the custom fields page - so nobody will be able to mess with it. I didn't know about that option when I created the extension.

That should be an easy fix (and could apply to existing installs via an upgrade script).

However, even when reserved, there will be times when things get out of sync. I think a second, more involved fix would be to use the civicrm_hook_managed hook to install, disable and uninstall the custom fields. That hook didn't exist when i wrote sumfields, so I have essentially invented my own bootleg version of it. I suspect it would be less buggy to use the core version instead.

I'm not sure how long it would take me to work on these, but I would welcome a PR from anyone who wants to take a crack at either fix.

twomice commented 6 months ago

@jmcclelland Right, great ideas regarding 'Reserved' and hook_civicrm_managed!

I don't have a business case for this fix myself, but of course I'm hoping someone does.

Thanks!