Skip to content

fix(core): resolve SQL error when removing user or user group#16887

Open
Ibochkarev wants to merge 2 commits intomodxcms:3.xfrom
Ibochkarev:fix/13725-sql-error-user-remove
Open

fix(core): resolve SQL error when removing user or user group#16887
Ibochkarev wants to merge 2 commits intomodxcms:3.xfrom
Ibochkarev:fix/13725-sql-error-user-remove

Conversation

@Ibochkarev
Copy link
Copy Markdown
Collaborator

What does it do?

Remove the composite Acls relation from modPrincipal in the schema and mysql map, because modAccess is abstract and has no table. xPDO cascade-delete attempted to query a non-existent table, generating invalid SQL (FROM AS modAccess). Add manual ACL deletion in modPrincipal::remove() before parent::remove(), iterating over principal_targets tables (Context, ResourceGroup, Category, MediaSource, Namespace). Refactor modUserGroup::remove() to remove its duplicate ACL deletion block and rely on modPrincipal::remove().

Why is it needed?

When deleting a user (or user group) via the Manager, xPDO tried to cascade-delete the composite Acls of class modAccess. modAccess is abstract—real tables are access_context, access_resource_groups, etc. xPDO could not resolve a table name and produced broken SQL, causing errors in the log even though the user was removed successfully.

How to test

  1. Create a test user in Manager (Security → Users).
  2. Delete the user from the grid.
  3. Check the error log—there should be no "Could not get table name for class: modAccess" or SQL syntax errors.
  4. Verify user removal and User Group removal still work correctly.

Related issue(s)/PR(s)

Fixes #13725

Remove composite Acls relation from modPrincipal schema because modAccess
is abstract and has no table; xPDO cascade-delete generated invalid SQL
(FROM AS modAccess). Add manual ACL deletion in modPrincipal::remove()
before parent::remove(), and refactor modUserGroup to rely on it.

Fixes modxcms#13725
@biz87
Copy link
Copy Markdown

biz87 commented Feb 25, 2026

Code Review

Summary

Fixes SQL errors when removing users or user groups by removing the invalid composite Acls relation from modPrincipal schema (since modAccess is abstract with no table) and adding manual ACL deletion in modPrincipal::remove() that iterates over concrete principal_targets tables. Refactors modUserGroup::remove() to rely on the parent's new deletion logic instead of duplicating it.

Suggestions

  • modPrincipal.php:50: array_walk($targets, 'trim') is called with the built-in trim function, but array_walk passes (&$value, $key) as arguments. This means trim receives the numeric index as the second parameter (character mask), which could inadvertently strip digits from the beginning/end of class names. Consider using $targets = array_map('trim', $targets) instead. Note: this is an existing pattern in getAttributes(), not introduced by this PR, but since you're writing new code with the same pattern, it would be good to fix it here.

Assessment

Correct fix for a real problem — xPDO cannot cascade-delete through an abstract class. Moving ACL cleanup to modPrincipal::remove() is architecturally sound since both modUser and modUserGroup extend modPrincipal. The raw SQL approach is appropriate here since xPDO can't resolve the abstract modAccess table. The modUserGroup::remove() refactoring properly removes duplicate logic. Schema and generated model changes are consistent.

Verdict: Approve

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.

SQL error in log when removing a user

2 participants