Skip to content

List what actually blocks a user being deleted - #810

Merged
blaipr merged 1 commit into
mainfrom
fix/the-usage-panel-lists-what-blocks-a-delete
Aug 17, 2026
Merged

List what actually blocks a user being deleted#810
blaipr merged 1 commit into
mainfrom
fix/the-usage-panel-lists-what-blocks-a-delete

Conversation

@blaipr

@blaipr blaipr commented Aug 17, 2026

Copy link
Copy Markdown
Member

The user view has a panel that answers "what would break if this user were removed". It has to agree with what happens when somebody presses delete, and it did not.

Six foreign keys reference User with no ON DELETE, which is RESTRICT, so any of them stops the removal:

relation blocks the delete was in the panel
Account.userId / userEditId yes yes
PublicLink.userId yes yes
AccountHistory.userId / userEditId yes no
Notification.userId yes no
AccountToUser.userId, UserToUserGroup.userId no — cascades yes

So an administrator retiring somebody opens their user, reads a panel that lists nothing preventing the removal — or only memberships, which cascade away harmlessly — presses delete, and is refused. The cause is that the person once edited an account, or has a single unread notification. Neither is visible anywhere in the application, and neither is something they can act on without being told.

getUsageForUser() now covers both. The account history is grouped by account rather than listed per revision: it keeps a row for every change ever made, so an account edited fifty times would otherwise fill the panel with fifty identical-looking entries. The name is aggregated because the account may have been renamed since — and may not exist at all any more, which is the point of history.

The two new kinds get their own icons (history, notifications). The switch that assigned them became a match, which is one assignment instead of one per kind — it also takes the PHPStan baseline entry for that dynamic property from 4 occurrences down to 1.

Tests

UserDeletionBlockersTest runs against a real database, with the container built by hand the way PasswordResetFlowTest builds one — IntegrationTestCase mocks the database away, so a test that inserts a history row and then asks whether it is reported would be interrogating a mock.

Each test asserts both halves together: that the panel reports the relation, and that the delete really is refused. Either alone can pass while the feature is broken — the panel could list something that does not block, or the delete could be blocked by something the panel never mentions, which is exactly the bug. A fourth test is the control: a user nothing refers to is reported as unused and really can be deleted, so a panel that called everybody blocked would not satisfy the suite.

The existing unit test counted bind values and nothing else, so it stayed green while two of the six blocking relations were absent from the query entirely. It now asserts every placeholder is the user asked about, and a second test asserts the query names every table that blocks a delete.

Reverting the repository change fails three of the four real-database tests, with the control still passing.

3965 unit tests pass; PHPStan level 6 on src clean with a smaller baseline; PHPCS clean.

The user view has a panel answering "what would break if this user were
removed". It has to agree with what happens when somebody presses delete, and it
did not.

Six foreign keys reference `User` with no `ON DELETE`, which is `RESTRICT`, so
any of them stops the removal: `Account.userId` / `userEditId`,
`AccountHistory.userId` / `userEditId`, `Notification.userId` and
`PublicLink.userId`. The panel covered the accounts and the public links — and,
informatively, the account and group memberships, which cascade away and block
nothing — but not the account history and not the notifications.

So an administrator retiring somebody reads a panel listing nothing that
prevents the removal, presses delete, and is refused: the person once edited an
account, or has a single unread notification. Neither is visible anywhere, and
neither is something they can act on without being told about it.

The history is grouped by account rather than listed per revision — it keeps a
row for every change ever made, so a long-lived account would otherwise fill the
panel with identical-looking entries. The name is aggregated because the account
may have been renamed since, and may not exist any more.

The `switch` assigning an icon per kind became a `match`, which is one
assignment instead of one per kind, and takes the PHPStan baseline entry for
that dynamic property from four occurrences to one.

`UserDeletionBlockersTest` runs against a real database, since
`IntegrationTestCase` mocks the database away and a test that inserts a history
row would be asking a mock whether it was reported. Each case asserts both
halves — the panel reports it *and* the delete is really refused — because
either alone can pass while this is broken. A fourth case is the control: a
user nothing refers to is reported as unused and really can be deleted.

The existing unit test counted bind values, so it stayed green while two of the
six blocking relations were missing from the query entirely.
@blaipr
blaipr force-pushed the fix/the-usage-panel-lists-what-blocks-a-delete branch from c0b1768 to b5eca35 Compare August 17, 2026 18:11
@blaipr
blaipr merged commit 8fa4c26 into main Aug 17, 2026
8 checks passed
@blaipr
blaipr deleted the fix/the-usage-panel-lists-what-blocks-a-delete branch August 17, 2026 18:18
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