CIVIMM-541: Fix Orphaned Rule Logs - #5
Open
shahrukh-compuco wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the schema and adds an upgrade script to change the foreign key constraint on civirule_rule_log.rule_id from ON DELETE SET NULL to ON DELETE CASCADE. The review feedback suggests improving the cleanup step in the upgrade script by using a LEFT JOIN delete query to remove both NULL and orphaned rule_id references, which prevents potential foreign key constraint violations when applying the new constraint.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
When a CiviRule is deleted, its history records in the rule log were being left behind as orphaned entries with no link back to any rule. This PR changes the behaviour so that deleting a rule also removes its associated log entries, and cleans up the orphaned entries that already exist.
Before
The
civirule_rule_log.rule_idforeign key was defined withON DELETE SET NULL. Deleting a rule setrule_idtoNULLon its log rows, leaving orphaned records incivirule_rule_logthat reference no rule and serve no purpose.After
The foreign key is now
ON DELETE CASCADE— deleting a rule automatically deletes its log rows. Existing orphaned rows (rule_id IS NULL) are removed as part of the upgrade.Technical Details
DELETE FROM civirule_rule_log WHERE rule_id IS NULLFK_civirule_rule_log_rule_idconstraint (guarded byCRM_Core_BAO_SchemaHandler::checkFKExists())ON DELETE CASCADEComments
civirule_rule_logrows with a NULLrule_id, on the assumption these only arise from previously deleted rules. Flag if there's any scenario where NULLrule_idis intentional.CRM_Civirules_BAO_CiviRulesRule::deleteWithId()already manually deletes matching log rows before deleting a rule — the cascade makes that redundant but harmless.