feat(app): overhaul audit log UX - #754
Merged
Merged
Conversation
Replace raw ID-heavy table with human-readable timeline, structured detail drawer, and server-side name resolution for actors and resources.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Review limit reached
More reviews will be available in 17 minutes and 52 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive audit log UX overhaul, transforming the interface from a table-based view to a timeline-feed design with server-side enrichment. The backend gains an enrichment pipeline that resolves actors, resources, and client metadata; new audit logging is wired throughout all team, project, and token management endpoints; and the frontend refactors to use composables for data fetching and new components for timeline display with filtering and detail modals. ChangesAudit Log UX Overhaul
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Thank you for following the naming conventions! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/shelve/server/api/teams/[slug]/projects/[projectId]/variables/[variableId]/index.put.ts (1)
27-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t derive audit project metadata solely from the URL.
Line 31 updates by
variableIdonly, while Lines 40-49 log whateverprojectIdwas passed in the route. If those IDs disagree, the update can be written against one variable and audited under a different project.Suggested fix
+ const existing = await db.query.variables.findFirst({ + where: and( + eq(schema.variables.id, variableId), + eq(schema.variables.projectId, projectId), + ), + }) + if (!existing) throw createError({ statusCode: 404, statusMessage: 'Variable not found' }) + - const project = await new ProjectsService().getProject(projectId) + const project = await new ProjectsService().getProject(existing.projectId)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/shelve/server/api/teams/`[slug]/projects/[projectId]/variables/[variableId]/index.put.ts around lines 27 - 49, The audit metadata is using the route projectId which can diverge from the actual variable's project; fetch the variable's authoritative project (e.g., call VariablesService.getVariable(variableId) or use the result returned by VariablesService.updateVariable) and derive projectId/projectName from that object instead of the route param, and additionally validate that the variable's project matches the route projectId (or throw) before calling logAudit so the audit always reflects the actual resource updated (refer to VariablesService.updateVariable, getVariable, ProjectsService.getProject, logAudit, variableId and projectId).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/shelve/app/components/audit/LogDetail.vue`:
- Around line 15-16: showRawMetadata is a component-level ref that currently
persists across log selections; add a watcher that resets showRawMetadata.value
= false whenever the selected log/entry changes (watch the selection prop or
emitted selection ref used by LogDetail, e.g., the prop named entry/selectedLog)
so new entries start collapsed; apply the same reset logic where selection can
change (same component areas referenced around lines 120-136) to ensure raw
metadata is cleared on every switch.
In `@apps/shelve/app/components/audit/LogEntry.vue`:
- Around line 14-55: The row is a native <button> (the element with
`@click`="$emit('select')") that currently wraps a nested <NuxtLink>, causing
invalid interactive nesting; change the outer <button> to a non-button container
(e.g., <div>) that preserves keyboard accessibility by adding role="button",
tabindex="0", and keyboard handlers (e.g., `@keydown.enter` and `@keydown.space`
calling $emit('select')) while keeping the existing `@click`="$emit('select')"
behavior and leaving the inner <NuxtLink> and its `@click.stop` unchanged so the
link can be focused and activated independently.
In `@apps/shelve/app/composables/useAuditLogs.ts`:
- Around line 24-47: The fetchLogs function can apply stale or duplicated
responses when multiple concurrent calls complete out-of-order; to fix, add a
per-call sequence token (e.g., incrementing requestId or an inFlightToken stored
alongside loading/loadingMore) at the start of fetchLogs, capture it in the
local scope before awaiting $fetch, and only update logs.value, cursor.value,
hasMore.value and total.value if the captured token matches the current token
(thus ignoring stale responses); also ensure loading/loadingMore flags and
cursor resets are set/cleared consistently (use finally) tied to that token so
only the active request clears the loading state.
In `@apps/shelve/app/composables/useScopeLabels.ts`:
- Around line 7-12: The module currently calls useState at import time (const
scopeLabels = useState(...)), which can run outside a Nuxt app/request context;
move the useState initialization inside the exported composable function
useScopeLabels so it runs per-call. Inside useScopeLabels, create scopeLabels
via useState<ScopeLabels | null>('scope-labels', () => null) and then keep the
existing async fetchScopeLabels logic to read/update scopeLabels.value and
return it; ensure any references to scopeLabels in fetchScopeLabels still
resolve to the locally created variable.
In `@apps/shelve/server/api/teams/`[slug]/audit-logs/index.get.ts:
- Line 21: The current code pushes lt(schema.auditLogs.id, cursor) into
conditions and then uses that same conditions array to compute total, causing
total to reflect “remaining rows” after the cursor; fix by separating the
pagination filter from the filter used to compute total: build a baseConditions
(copy of conditions before adding the cursor) and use that baseConditions when
calculating total, then only push lt(schema.auditLogs.id, cursor) into
conditions for the paginated query (affecting nextCursor/rows), so total remains
the count of rows matching active filters (not filtered by cursor); apply the
same change where total is computed (lines referenced around total and where
cursor is added) so total, cursor, conditions, and nextCursor behave correctly.
In `@apps/shelve/server/api/teams/`[slug]/environments/[id].delete.ts:
- Around line 15-26: The delete endpoint currently performs
db.delete(schema.environments)... then calls clearCache(...) and awaits
logAudit(...), but if logAudit throws the HTTP response fails after the delete
committed; make the audit write best-effort so the delete remains successful:
after awaiting db.delete(...) and clearCache('Environments', team.id), wrap the
await logAudit(event, {...}) call in a try/catch (or dispatch it without
awaiting) and on failure only record/log the audit error (e.g.,
processLogger.warn/error) and do not rethrow; keep the response for the deletion
successful. Ensure you reference the existing symbols
db.delete(schema.environments), clearCache('Environments', team.id), and
logAudit(event, {...}) when making the change.
In
`@apps/shelve/server/api/teams/`[slug]/projects/[projectId]/variables/[variableId]/index.delete.ts:
- Around line 10-29: The variable lookup is only by variableId which allows
deleting a variable from one project while auditing another; update the
existence check in db.query.variables.findFirst to also require the route
project (e.g., include eq(schema.variables.projectId, projectId)) or otherwise
verify existing.projectId === projectId, and then pass or enforce the project
context when calling VariablesService.deleteVariable (or adjust deleteVariable
to accept projectId) so the deletion and the logAudit
(ProjectsService.getProject, logAudit) refer to the same project.
In `@apps/shelve/server/api/user/scope-labels.get.ts`:
- Around line 1-13: The file imports only inArray from drizzle-orm but calls eq
in the DB query; update the import statement that currently reads "import {
inArray } from 'drizzle-orm'" to also include eq so eq is available for the call
in the default defineEventHandler (used in the db.query.members.findMany where
eq(schema.members.userId, user.id) is invoked); ensure other usages of
inArray/eq in this module still work after the import change and run a
build/test to verify no runtime ReferenceError.
In `@apps/shelve/server/services/audit-enrich.ts`:
- Around line 60-74: The project/environment enrichment queries
(db.query.projects.findMany and db.query.environments.findMany that use
inArray(schema.projects.id, [...ids.projectIds]) and
inArray(schema.environments.id, [...ids.environmentIds])) must also restrict
results to the current team to avoid cross-team label leakage; update both where
clauses to include a teamId equality predicate (e.g., teamId === currentTeamId
or schema.projects.teamId/ schema.environments.teamId equals the team id in
scope) so only rows matching the team are returned before mapping into
projects.set and environments.set.
In `@apps/shelve/server/services/audit.ts`:
- Around line 50-60: The current logAuditForTeams function performs sequential
awaits over uniqueTeamIds which adds latency; replace the for-loop with a
parallelized approach by using Promise.all over uniqueTeamIds.map to call
logAudit(event, { ...payload, teamId }) for each teamId, and preserve the
existing per-entry error swallowing/handling by attaching .catch(...) to each
mapped promise (or mapping to a wrapper that returns a resolved promise on
error) so one failed audit doesn't reject the whole Promise.all; keep the
function signature and use the same uniqueTeamIds de-duplication logic.
In `@apps/shelve/server/utils/audit-present.ts`:
- Around line 36-39: The fallback summaries are unreachable because `\`Updated
variable ${meta.key ?? ''}\`.trim()` still returns "Updated variable" when
meta.key is missing; update the switch cases for 'variables.update' and
'variables.delete' to choose between two explicit strings based on whether
meta.key is present (e.g., use a conditional on meta.key) so that when meta.key
is falsy you return the project-context fallback via projectLabel(meta.projectId
as number | undefined, maps, meta); apply the same pattern for the delete case.
In `@apps/shelve/test/e2e/flows/audit-logs.ts`:
- Around line 12-16: The test currently asserts the presence of the
'token.create' audit entry immediately after a fire-and-forget write (using
ctx.api to fetch AuditLogResponse.feed.logs), which can flake; modify the test
to perform bounded polling: repeatedly call
ctx.api(`/api/teams/${ctx.teamSlug}/audit-logs`) with a short delay (e.g.,
100–500ms) up to a configurable timeout or max attempts, check
feed.logs.some(entry => entry.action === 'token.create') on each attempt, and
only fail the test if the timeout/max attempts elapse without finding the entry;
keep the existing assertions for feed.logs being an array and the summary/actor
checks after the polling succeeds.
In `@packages/types/src/Audit.ts`:
- Around line 21-35: AUDIT_ACTIONS is missing the 'token.use' member present in
the AuditAction type; update the exported AUDIT_ACTIONS array to include
'token.use' so the list stays aligned with the AuditAction contract (locate the
AUDIT_ACTIONS constant in packages/types/src/Audit.ts and add the 'token.use'
entry).
---
Outside diff comments:
In
`@apps/shelve/server/api/teams/`[slug]/projects/[projectId]/variables/[variableId]/index.put.ts:
- Around line 27-49: The audit metadata is using the route projectId which can
diverge from the actual variable's project; fetch the variable's authoritative
project (e.g., call VariablesService.getVariable(variableId) or use the result
returned by VariablesService.updateVariable) and derive projectId/projectName
from that object instead of the route param, and additionally validate that the
variable's project matches the route projectId (or throw) before calling
logAudit so the audit always reflects the actual resource updated (refer to
VariablesService.updateVariable, getVariable, ProjectsService.getProject,
logAudit, variableId and projectId).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c2c71d61-2457-4881-bc35-25dddcb807a2
📒 Files selected for processing (37)
.changeset/audit-log-ux-overhaul.mdapps/lp/content/docs/2.core-features/audit-logs.mdapps/shelve/app/components/audit/LogDetail.vueapps/shelve/app/components/audit/LogEntry.vueapps/shelve/app/components/audit/LogFeed.vueapps/shelve/app/components/audit/LogFilters.vueapps/shelve/app/components/token/Scopes.vueapps/shelve/app/composables/useAuditLogs.tsapps/shelve/app/composables/useScopeLabels.tsapps/shelve/app/pages/[teamSlug]/team.vueapps/shelve/app/pages/[teamSlug]/team/audit-logs.vueapps/shelve/app/utils/userAgent.tsapps/shelve/server/api/teams/[slug]/audit-logs/index.get.tsapps/shelve/server/api/teams/[slug]/environments/[id].delete.tsapps/shelve/server/api/teams/[slug]/environments/[id].put.tsapps/shelve/server/api/teams/[slug]/environments/index.post.tsapps/shelve/server/api/teams/[slug]/invitations/index.post.tsapps/shelve/server/api/teams/[slug]/members/[id].delete.tsapps/shelve/server/api/teams/[slug]/projects/[projectId]/index.delete.tsapps/shelve/server/api/teams/[slug]/projects/[projectId]/variables/[variableId]/index.delete.tsapps/shelve/server/api/teams/[slug]/projects/[projectId]/variables/[variableId]/index.put.tsapps/shelve/server/api/teams/[slug]/projects/[projectId]/variables/env/[envId].get.tsapps/shelve/server/api/teams/[slug]/projects/[projectId]/variables/index.post.tsapps/shelve/server/api/teams/[slug]/projects/index.post.tsapps/shelve/server/api/tokens/[id].delete.tsapps/shelve/server/api/tokens/index.post.tsapps/shelve/server/api/user/scope-labels.get.tsapps/shelve/server/middleware/2.auth.tsapps/shelve/server/services/audit-enrich.tsapps/shelve/server/services/audit.tsapps/shelve/server/services/user.tsapps/shelve/server/utils/audit-present.tsapps/shelve/server/utils/userAgent.tsapps/shelve/shared/types/auth.d.tsapps/shelve/test/e2e/flows/audit-logs.tsapps/shelve/test/unit/audit-enrich.test.tspackages/types/src/Audit.ts
Harden variable audit metadata, fix pagination total count, prevent stale fetches, scope enrichment to team, and improve a11y and E2E reliability.
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
1 participant
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
summary, actor, resource, client) via batch lookups instead of raw IDsTest plan
/{teamSlug}/team/audit-logsand verify entries show readable summaries (e.g.web-platform / production)token.createappears in the team audit feed/user/tokens, hover scoped token chips and verify tooltip shows names not IDspnpm exec vitest run test/unit/audit-enrich.test.tsinapps/shelveSummary by CodeRabbit