Skip to content

CIVIMM-541: Fix Orphaned Rule Logs - #4

Open
shahrukh-compuco wants to merge 1 commit into
3.32-patchesfrom
civimm-541-fix-orphaned-rule-logs
Open

CIVIMM-541: Fix Orphaned Rule Logs#4
shahrukh-compuco wants to merge 1 commit into
3.32-patchesfrom
civimm-541-fix-orphaned-rule-logs

Conversation

@shahrukh-compuco

Copy link
Copy Markdown
Contributor

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_id foreign key was defined with ON DELETE SET NULL. Deleting a rule set rule_id to NULL on its log rows, leaving orphaned records in civirule_rule_log that 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

  • New upgrade function, which:
    1. Deletes existing orphaned rows: DELETE FROM civirule_rule_log WHERE rule_id IS NULL
    2. Drops the existing FK_civirule_rule_log_rule_id constraint (guarded by CRM_Core_BAO_SchemaHandler::checkFKExists())
    3. Re-adds the constraint with ON DELETE CASCADE

Comments

  • The upgrade deletes all civirule_rule_log rows with a NULL rule_id, on the assumption these only arise from previously deleted rules. Flag if there's any scenario where NULL rule_id is intentional.
  • CRM_Civirules_BAO_CiviRulesRule::deleteWithId() already manually deletes matching log rows before deleting a rule — the cascade makes that redundant but harmless.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the foreign key constraint on civirule_rule_log.rule_id from ON DELETE SET NULL to ON DELETE CASCADE in both the schema definition and the upgrader. The reviewer suggested using a LEFT JOIN delete query to clean up all orphaned log entries (including those with invalid, non-existent rule_ids) instead of only deleting rows where rule_id IS NULL. This prevents potential referential integrity failures when applying the new foreign key constraint.

Comment thread CRM/Civirules/Upgrader.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant