Skip to content

fix(device-groups): clean up config policy assignments before deleting device group - #5855

Open
mvthul wants to merge 1 commit into
LanternOps:mainfrom
mvthul:fix/cleanup-group-policy-assignments-before-delete
Open

mvthul wants to merge 1 commit into
LanternOps:mainfrom
mvthul:fix/cleanup-group-policy-assignments-before-delete

Conversation

@mvthul

@mvthul mvthul commented Sep 14, 2026

Copy link
Copy Markdown

Why

When deleting a device group that has active configuration policy assignments targeting it (level = 'device_group'), the deletion fails because policy assignments reference the target device group.

What

In deleteDeviceGroup, explicitly delete any configPolicyAssignments rows targeting the group (level = 'device_group', targetId = groupId) within the deletion transaction prior to removing the deviceGroups record.

Part of discussion #5797.

@ToddHebebrand ToddHebebrand left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for upstreaming this — the fix is correct and I verified the mechanism. Welcome aboard.

Verified root cause: there is no FK on config_policy_assignments.target_id (it's polymorphic), but device_groups carries an AFTER DELETE ... FOR EACH STATEMENT trigger (ab_config_policy_assignment_group_owner_delete) that re-validates any surviving assignments still pointing at the deleted group, and that raises 23503 because the group row is gone. Deleting the assignments first empties that set, so the ordering here is right, and the direct-delete trigger on config_policy_assignments skips validation for DELETE, so nothing else trips. Tenancy is fine: the group was already validated against orgId earlier in the same transaction.

One thing before merge: apps/api/src/__tests__/integration/deviceGroupDelete.integration.test.ts already exercises every other blocking path for this function (contracts, quotes, membership log) against a real database. Please add one case: seed a config_policy_assignments row with level = 'device_group' and targetId = group.id, call deleteDeviceGroup, assert it succeeds and the assignment row is gone. Without it the next refactor can silently reintroduce the trigger failure.

Nice-to-have: a one-line comment on apps/api/src/services/deviceGroupDelete.ts:103 naming the trigger, matching the #3313 note on the line above it.

CI is approved and running now (first-contributor gate). Once the test is in I'll enqueue it.

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.

2 participants