Drupal 10, the latest version of the open-source digital experience platform with even more features, is here.I was recently left scratching my head after "verify" tasks started deleting files uploaded as attachments to cases. Only a couple temporary directories are supposed to be purged during the cache clears in a verify task. However, if these settings are blank, some nasty consequences follow.
The "uploadDir" is one of the directories that gets purged. If the field doesn't contain an absolute path, it is considered a relative path, rooted at example.com/files/civicrm. If this field is blank, then that entire directory gets blown away, including all subdirectories that are supposed to persist!
I'm marking this as a major "bug report", because failing to do this can cause significant data loss.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | hosting_civicrm-implement_hooks-2794645-11.patch | 1.88 KB | ergonlogic |
| #8 | hosting_civicrm-add_hooks-2794645-8.patch | 1.89 KB | ergonlogic |











Comments
Comment #2
ergonlogicUnfortunately,
provision_civicrm_regenerate_settings_file()appears to be pulling the template from CiviCRM core, where the necessary variables can't be populated. Assuming that we want to keep doing that, we should submit a PR to allow that setting to be passed in as a variable. At least we can then apply said patch to platforms we build pending merging into CiviCRM core.In addition, we could add a hook to allow altering the list of civicrm.settings.php template search paths, so that we'd have the flexibility of overriding it in Aegir.
Comment #3
ergonlogicAnother thing... It looks like the relevant variables are mis-labeled in
civicrm.settings.php:Comment #4
ergonlogicI submitted the following issues upstream in CiviCRM's Jira:
Comment #5
ergonlogicI don't see any really clean way of adding the setting. The best I've come up with so far would be to do something like:
This *should* replace the entire line that matches both "civicrm_settings" and "uploadDir", and replace it with the line, uncommented, and set to the value we want to hardcode. Of course, we'd have to do the same for the "
global $civicrm_settings;" line as well.An alternative would be to drop using the template from CiviCRM core, and generate our own, similarly to how we generate
settings.phpfor Drupal sites. While this'd give us a whole lot more flexibility, including masking database credentials, it'd also mean more maintenance, as we adapt to changes between CiviCRM versions.Comment #6
bgm CreditAttribution: bgm at Coop SymbioTIC commentedCRM-19307 seems like the best way forward.
I would avoid a custom civicrm.settings.php.template. This file changes much more often than settings.php changes in Drupal.
Comment #7
ergonlogicBased on my research so far, I don't think there'll be an easy way to resolve CRM-19307. If I've missed anything, please don't hesitate to point it out.
In the short-term, I think a
preg_replace()-based solution, as described above, should be reasonably stable. We could guard against changes tocivicrm.settings.php.template, by adding apreg_match()to search for the same pattern, and throw a warning if it isn't found.I'll work on a patch along these lines.
Comment #8
ergonlogicHere is a patch that adds some hooks that allow altering
civicrm.settings.phpin various ways.Comment #9
ergonlogicHere's a patch that implements a couple of the hooks added in the previous patch, to hardcode
uploadsDir.Comment #10
ergonlogicComment #11
ergonlogicReplaced patch from #9. Now hard-codes the full path.
Comment #12
sleewok CreditAttribution: sleewok commentedAny chance of having this set all paths hard coded? Verify still resets all to default except for the uploaddir. I understand the other settings may be specific to the site setup. Is there any way to read the current settings values and have those hard coded?
Comment #14
bgm CreditAttribution: bgm at Coop SymbioTIC commentedJust putting this here for reference, but not implying anything else. If this works, lets keep it. I see that the commits were merged, does anything else need to be done?
This was merged in CiviCRM 4.7.21: https://github.com/civicrm/civicrm-core/pull/10511
Comment #15
bgm CreditAttribution: bgm at Coop SymbioTIC commentedI pushed a commit here: https://git.drupalcode.org/project/hosting_civicrm/-/commit/34c8c09689fe...
As of CivICRM 5.44, the override code was causing the `$civicrm_root` global var to be lost. I didn't quite understand the first regexp, but I feel like it's not required anymore.