-
-
Notifications
You must be signed in to change notification settings - Fork 825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dev/core#2242 Ensure that when a custom field is deleted any associat… #19199
Conversation
…ed mapping field records are also deleted to avoid DB errors when doing exports
(Standard links)
|
…he civicrm_mapping_field table
@eileenmcnaughton @jusfreeman I've added an upgrade routine now as well |
Cool - @agileware-justin I think you reported this - will you review @seamuslee001 work on it? |
Yep will do, thanks again @seamuslee001 and for the ping @eileenmcnaughton |
I really like having an upgrade script to fix pre-existing issues for this problem. Nice one! |
Related Gitlab issue for anyone wondering, https://lab.civicrm.org/dev/core/-/issues/2242 |
@seamuslee001 please self-merge when @agileware-justin confirms - code looks good & test looks extra good |
@agileware-justin has someone from your team been able to review / test this? |
@seamuslee001 I did try to test it on CiviCRM 5.32.2 but the patch didn't apply. Is this for CiviCRM 5.34? If so then I'll need to test against https://download.civicrm.org/latest/civicrm-NIGHTLY-wordpress.zip We break for Xmas hols from 18/12, so it's only me who would have time to test it during the holidays. |
@agileware-justin yes that would be correct |
PR for #civicrm-5.34.0 |
…ed mapping field records are also deleted to avoid DB errors when doing exports
Overview
Does what it says above
Before
Possible DB error if re-using a saved field mapping for export which contains a reference to a custom field that has been deleted
After
No DB error
Technical Details
I suspect we will want to use an Upgrade to clean up stuff here as well
ping @eileenmcnaughton @jusfreeman