Fix audit endpoints dropping rows whose related active_* row is soft-deleted#449
Merged
Merged
Conversation
…deleted The FastAPI migration in #425 added joinedload(active_user) on /api/audit/users and joinedload(active_role_group) on /api/audit/groups. Both relationships carry innerjoin=True + a primaryjoin filter on deleted_at IS NULL, so SQLAlchemy emitted INNER JOINs that silently dropped audit rows whose user or role group had since been soft-deleted (reducing one production audit page from 398 entries to 19). Removed every active_* load and matching response field the migration added to either audit endpoint — the unfiltered user/group/role_group fields plus the existing deleted_at columns are all the frontend reads off audit rows, and no consumer touches the active_* keys. This restores the pre-migration wire shape and closes both INNER-JOIN regressions. Added regression tests covering the soft-deleted user and soft-deleted role-group cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI ruff format check wanted the new test function signatures on a single line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
matthew-bass
approved these changes
May 18, 2026
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
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.
Summary
joinedload(active_user)/joinedload(active_role_group)introduced by the FastAPI migration in Migrate from Flask + Marshmallow to FastAPI + Pydantic #425 that caused audit pages to silently drop rows whose user or role group had been soft-deleted (reducing one production page from 398 entries to 19).active_*fields from the audit response schemas — theuser/group/role_groupfields (with their existingdeleted_atcolumns) are all the frontend reads off audit rows; no consumer touches theactive_*keys.Why
OktaUserGroupMember.active_userandRoleGroupMap.active_role_groupare defined withinnerjoin=Trueplus aprimaryjoinfilter ondeleted_at IS NULL. When the migration eager-loaded them viajoinedload(...), SQLAlchemy emitted INNER JOINs that excluded any row whose target had since been soft-deleted, even though the audit row itself is valid history.selectinload(active_group)/selectinload(active_role_group_mapping)don't cause the same regression but were unused additions on the response and have been dropped for consistency with the pre-migration Flask wire shape. The basejoinedload(user)/joinedload(group)/joinedload(role_group)(all unfiltered) plus the existingdeleted_aton the row helpers give the frontend everything it needs to render soft-deleted entities.The shared helper
api/routers/_eager.py:40has the samejoinedload(active_user)pattern but is used bygroups/apps/users/role_requestsroutes whose consumers do readactive_user, so the fix there isjoinedload(..., innerjoin=False)orselectinload(...)rather than deletion — left for a separate follow-up.Test plan
pytest tests/— 424 passedtest_users_audit_returns_rows_when_user_is_soft_deletedandtest_groups_audit_returns_rows_when_role_group_is_soft_deletedcover the regressionruff checkclean on the touched files/groups/Role-All-Employees/audit?active=falseshows soft-deleted users with the strikethrough style (handled by the existingdisplayUserNamehelper offuser.deleted_at)🤖 Generated with Claude Code