audit-mixin-rollout-tier3: budget + users (groups 3 & 4) — created_by/updated_by - #306
Merged
arutsh merged 2 commits intoSep 21, 2026
Merged
Conversation
…by/updated_by (#305) PrivilegedAccessLog in both services now inherits AuditMixin, adding created_by/updated_by (and the updated_at column the mixin also maps, missing from the initial migration draft and caught in code review). UserProfileModel is exempted from AuditColumnsMixin (design.md Decision 2): its RabbitMQ event-consumer writer has no actor context, and its request-scoped cache writer would misattribute rows to the viewer rather than the cached user. Dead code with the same misattribution shape (get_user_from_cache_or_fallback and its now-unused callees) is removed. Closes #305 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…xes mypy AuditMixin already maps id: Mapped[uuid.UUID]; the subclass's own copy was typed Mapped[t.GUID] (the column type, not the mapped Python type), which mypy flagged as incompatible with the base class after inheriting AuditMixin. Removing the redundant redeclaration keeps AuditMixin's correctly-typed column as the single source of truth. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
arutsh
deleted the
Shared/feat/Issue-305/audit-mixin-rollout-tier3-group3
branch
September 21, 2026 16:03
This branch was successfully deployed
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
PrivilegedAccessLoginservices/budgetandservices/usersnow inheritsAuditMixin, addingcreated_by/updated_by(append-only, soupdated_bystays permanentlyNULL) — andupdated_at, the mixin's third mapped column, which the initial migration draft omitted and/code-reviewcaught before merge (verified live against the siblingai/chatmigrations from this same rollout, and confirmed via schema inspection after runningalembic upgrade headlocally against both services).UserProfileModel(budget's user cache) is exempted fromAuditColumnsMixin— decision recorded indesign.mdDecision 2: its RabbitMQ event-consumer writer has no actor in scope, and its request-scoped cache writer would misattribute rows to the viewer rather than the cached user. Removed now-dead code with the same misattribution shape (get_user_from_cache,get_user_from_cache_or_fallback,user_client.get_user,UserServiceError).mypyerror caught by the pre-push hook: bothPrivilegedAccessLogmodels redundantly redeclaredidwith the wrong mapped type (Mapped[t.GUID]instead ofMapped[uuid.UUID]), conflicting withAuditMixin's ownidcolumn — removed the redeclaration.Closes #305 (users service, group 4, has no separate sub-issue — folded into this PR alongside budget).
Test plan
services/budgetandservices/userstest suites pass, including newtest_tier3_audit_columns.pyin eachblack/mypy/flake8clean on both services (pre-push hook passed)alembic upgrade headrun against real local Postgres for both services; confirmed via\d privileged_access_logsthatcreated_by/updated_by/updated_atall exist post-migration🤖 Generated with Claude Code