Make user deletion FK-safe: extend deletable? coverage#1704
Open
maebeale wants to merge 2 commits into
Open
Conversation
Captures the reassignment approach to the user-destroy FK constraint error (Honeybadger: UsersController#destroy raised InvalidForeignKey on resources.created_by_id). Instead of preventing deletion, this reassigns NOT NULL created_by/updated_by columns to the orphaned user and nullifies nullable foreign keys across every table with an FK to users, so the user can be destroyed without violating constraints. This is the exploratory approach; the following commit supersedes it in favor of extending the prevent-deletion path already on main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Supersedes the reassignment approach: drop the reassign_owned_records callback and instead make User#deletable? comprehensive so the UI reflects deletability accurately rather than leaning on the controller's InvalidForeignKey rescue. deletable? previously checked only 9 creator associations, so a user who owned e.g. a banner, an authored comment, community news, a created event/person, an affiliation, a self-referential created_by, a notification they sent, or a blazer record passed the check, showed the Delete button, and only then hit the rescue. It now checks every RESTRICT foreign key to users (via model classes for mapped tables and direct SQL for legacy tables: user_permissions and blazer_*). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
What is the goal of this PR and why is this important?
UsersController#destroyraisedActiveRecord::InvalidForeignKey(e.g.resources.created_by_id) when deleting a user who still owns records.User#deletable?(Prevent deletion of users with associated records #1295), butdeletable?only checks 9 creator associations. Many other RESTRICT foreign keys tousers(banners, authored comments, community news, created events, people, affiliations, user_permissions, self-referential users, blazer) are missed — so the Delete button shows, the user clicks, and only then hits the controllerrescueand an alert. Misleading UX.How did you approach the change?
deletable?to cover the remaining RESTRICT foreign keys so the Delete button is hidden whenever a user owns any blocking record, instead of relying on the rescue safety net.Anything else to add?