Skip to content
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#1093: Bckport Fix to v.5.19 to Add Custom Fields to Logging Tables #15642

Conversation

MiyaNoctem
Copy link
Contributor

Overview

This fix was recently merged to master on this PR #15599.

As a regression issue injected when changing the method used to create custom fields, we need to back-port it into v5.19 rc branch.

We recently ran into problems with direct debit extension due to this change. Basically, if someone has logging enabled, and tries to install the extension, a fatal error will be thrown, complaining about a non-existing field.

Before

The problem happens when a custom field is added via XML file on installation of an extension, logging is turned on, and part of the installation process inserts information on that new field. This happens, because the procedure used to install an extension is something like:

  1. Process .xml files (creating custom groups and fields):
    1. Creates custom group table, custom group log table, and triggers to update log table.
    2. Creates custom group fields - WITHOUT creating fields on custom group log table.
  2. Runs install steps on Upgrader.php for the extension
  3. If logging is turned on, syncs every table that was modified with their corresponding log table.

So, if we include any updates on the new custom group tables on Upgrader.php, the trigger will be fired to update the table's corresponding logging table, and fail, as the new fields will not exist on the log table.

After

Fixed by building log table after all fields are added to main table, and resetting the cache mapping tables with its columns before calling fixSchemaDifferences() at the end of extension installation.

BulkSave method was adding fields to custom group table, but not to custom
group logging table. If these tables were updated on installation, before all
the schema is synced bewtween main and logging tables, fatal errors would be
thrown, as triggers tried to insert info into log tables missing required
fields.

Fixed by building log table after all fields are added to main table, and
before rebuilding triggers.
@civibot
Copy link

civibot bot commented Oct 28, 2019

(Standard links)

@civibot civibot bot added the 5.19 label Oct 28, 2019
@MiyaNoctem
Copy link
Contributor Author

@eileenmcnaughton, here's the PR to backport the fix to v5.19.

@MiyaNoctem MiyaNoctem changed the title dev/core#1093: Bckport Fix to v.519 to Add Custom Fields to Logging Tables dev/core#1093: Bckport Fix to v.5.19 to Add Custom Fields to Logging Tables Oct 28, 2019
@eileenmcnaughton eileenmcnaughton merged commit c2876f6 into civicrm:5.19 Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants