Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a client-side ActivityDetailPanel, backend traceability for activities (eventId + metadata), TRPC getActivityById, expanded actor email, logging of request metadata, event-labeling utilities, and ActivityFeed UI changes to support on-demand detail viewing. Changes
Sequence DiagramsequenceDiagram
actor User
participant Feed as Activity Feed (Client)
participant Detail as ActivityDetailPanel (Client)
participant TRPC as TRPC Server
participant DB as Database
User->>Feed: Click activity item
Feed->>Feed: setSelectedActivityId(id)
Feed->>Detail: Render with activityId, projectId
Detail->>TRPC: call getActivityById(id, projectId)
TRPC->>DB: SELECT activity WITH actor (+email) and metadata WHERE id & projectId
DB-->>TRPC: activity record (including eventId, metadata, actor.email)
TRPC-->>Detail: activity data
Detail->>Detail: render header, timestamps, actor, what/old→new
Detail-->>User: display detail panel
User->>Detail: Click close
Detail->>Feed: onClose()
Feed->>Feed: clear selectedActivityId
Feed-->>User: detail panel removed
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
components/project/activity/activity-detail-panel.tsx (1)
24-43: ExtractgetActionIconinto a shared activity UI helper.This mapping is duplicated in
components/project/activity/activity-feed.tsx; centralizing it prevents drift and keeps behavior consistent.As per coding guidelines,
**/*.{ts,tsx,js,jsx}: "Check if there are existing libraries, helpers, or utils that can be used before writing new code", and**/*.{ts,tsx}: "Write modular and reusable code; check if a component can be reused before creating a new one".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/project/activity/activity-detail-panel.tsx` around lines 24 - 43, Extract the duplicated getActionIcon mapping into a shared helper (e.g., export getActionIcon from a new module like activityIcons or activityUI helper) and replace the inline function in both activity-detail-panel.tsx and activity-feed.tsx with an import of that helper; ensure the helper exports the same function signature and return shape (icon, color, bg) so existing callers (getActionIcon in activity-detail-panel and the duplicate in activity-feed) continue to work without other changes, then remove the duplicated implementation from both files and update their imports to use the shared helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/project/activity/activity-detail-panel.tsx`:
- Around line 146-152: The close button rendering in ActivityDetailPanel (the
<button> with onClick={onClose} that contains the X icon) is missing an
accessible label; update that button to include an aria-label (e.g.,
aria-label="Close" or aria-label={t('Close')} if using i18n) so screen readers
can announce its purpose, keeping the onClick handler (onClose) and the X icon
component intact.
- Around line 106-127: The loader shows indefinitely on failures and the query
key omits projectId; update the useQuery call for projects.getActivityById to
include projectId in the queryKey (e.g. ["projects","getActivityById",
projectId, activityId]) and read the query's error state (useQuery return:
isPending/isLoading, error, data/item) instead of relying solely on !item; in
the render replace the unconditional SpinnerWithSpacing for the !item case with
branching that shows SpinnerWithSpacing when loading (isPending/isLoading), an
error message/UI when error is present (use error from useQuery), and the item
details otherwise, using the existing symbols useQuery, queryKey,
projects.getActivityById, isPending/isLoading, error, item, SpinnerWithSpacing,
and Panel to locate and fix the logic.
In `@lib/activity/index.ts`:
- Around line 44-57: The code currently assigns the raw sessionId from
auth.api.getSession into metadata.sessionId; change this to avoid persisting raw
session identifiers by replacing sessionId with a non-reversible fingerprint
before adding it to activity.metadata (e.g., compute a SHA256/HMAC of sessionId
with an application secret or salt), keep the variable sessionId for fetching
but set metadata.sessionId = sessionId ? fingerprint(sessionId) : null, and
ensure any references to activity.metadata read the fingerprint rather than the
raw id.
In `@trpc/routers/projects.ts`:
- Around line 468-505: The getActivityById protectedProcedure currently returns
activity.metadata to any viewer; change it to restrict or redact that field for
non-admins by either (a) changing the DB query in
ctx.db.query.activity.findFirst to exclude the metadata column for
non-privileged users, or (b) after fetching activityItem, check an admin/owner
helper (e.g., isProjectAdmin(ctx, input.projectId) or ctx.session.user.role ===
'admin') and if the caller is not privileged, remove or replace
activityItem.metadata with a redacted value before returning; update references
to activityItem.metadata and ensure you still throw NOT_FOUND and FORBIDDEN as
before.
---
Nitpick comments:
In `@components/project/activity/activity-detail-panel.tsx`:
- Around line 24-43: Extract the duplicated getActionIcon mapping into a shared
helper (e.g., export getActionIcon from a new module like activityIcons or
activityUI helper) and replace the inline function in both
activity-detail-panel.tsx and activity-feed.tsx with an import of that helper;
ensure the helper exports the same function signature and return shape (icon,
color, bg) so existing callers (getActionIcon in activity-detail-panel and the
duplicate in activity-feed) continue to work without other changes, then remove
the duplicated implementation from both files and update their imports to use
the shared helper.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
components/project/activity/activity-detail-panel.tsxcomponents/project/activity/activity-feed.tsxdrizzle/schema.tsdrizzle/types.tslib/activity/index.tslib/activity/message.tstrpc/routers/projects.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/project/activity/activity-detail-panel.tsx (1)
104-110:⚠️ Potential issue | 🟡 MinorAdd an accessible name to the icon-only close button.
The close button still needs an
aria-labelso screen readers can announce its purpose.♿ Suggested fix
<button type="button" onClick={onClose} + aria-label="Close activity details" className="p-1 rounded-md hover:bg-muted transition-colors" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/project/activity/activity-detail-panel.tsx` around lines 104 - 110, The icon-only close <button> in ActivityDetailPanel needs an accessible name: add an aria-label (e.g., aria-label="Close") to the button that invokes onClose and mark the decorative <X /> icon as aria-hidden="true" so screen readers announce the button purpose but ignore the visual icon; update the JSX for the button containing onClick={onClose} and the <X /> element accordingly.
🧹 Nitpick comments (2)
lib/activity/index.ts (1)
45-48: Don’t silently swallow session enrichment failures.The empty
catch {}on Lines 45-48 hides real errors and makes audit enrichment failures hard to diagnose.🛠️ Suggested improvement
- } catch {} + } catch (error) { + console.warn("Failed to enrich activity metadata from session", { error }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/activity/index.ts` around lines 45 - 48, The try/catch around auth.api.getSession in lib/activity/index.ts silently swallows errors; change the empty catch to handle failures by logging the error and context and preserving normal flow: catch the exception thrown by auth.api.getSession, call the existing logger (or create one) to record the error and the request headers/context, and then set userEmail = null (as currently intended) so enrichment failures are visible but non-fatal; reference the auth.api.getSession call and the userEmail assignment when applying the fix.lib/activity/message.ts (1)
148-188: Hoist event descriptions to module scope for reuse.
descriptionsis rebuilt on everygetEventDescriptioncall. Move it to a top-level constant to reduce allocations and keep mapping data reusable.♻️ Proposed refactor
+const eventDescriptions: Record<string, Record<string, string>> = { + task: { + created: "A new task was added to the project.", + updated: "An existing task was modified.", + deleted: "A task was removed from the project.", + }, + // ...rest unchanged +}; + export function getEventDescription(type: string, action: string): string { - const descriptions: Record<string, Record<string, string>> = { - // ... - }; - - return descriptions[type]?.[action] || `${typeLabels[type] || type} was ${action}.`; + return ( + eventDescriptions[type]?.[action] || + `${typeLabels[type] || type} was ${action}.` + ); }As per coding guidelines, "Write modular and reusable code; check if a component can be reused before creating a new one".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/activity/message.ts` around lines 148 - 188, The descriptions map is currently recreated on every call inside getEventDescription; extract it to a top-level constant (e.g., const EVENT_DESCRIPTIONS = { ... }) at module scope and have getEventDescription reference EVENT_DESCRIPTIONS instead of the local descriptions variable; ensure you keep the same keys and fallback logic with typeLabels and update any imports/exports if needed so the mapping can be reused across the module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/activity/index.ts`:
- Around line 50-54: The activity metadata currently stores raw ipAddress,
userAgent and userEmail in the metadata object (see metadata and
activity.metadata); replace those raw values with privacy-safe alternatives: do
not persist userEmail or raw ipAddress/userAgent — instead persist either an
irreversible hash/fingerprint (e.g., hash(ipAddress) and hash(userAgent)) or
high-level anonymized attributes (e.g., country/region flag, deviceType, or a
boolean isInternalUser), and ensure any code that returns activity.metadata to
clients strips or redacts sensitive fields. Update all places that build
metadata (the metadata variable and the other occurrence referenced) and add a
small helper (e.g., anonymizeMetadata) to centralize hashing/redaction so
activity writers/readers use the sanitized values only.
---
Duplicate comments:
In `@components/project/activity/activity-detail-panel.tsx`:
- Around line 104-110: The icon-only close <button> in ActivityDetailPanel needs
an accessible name: add an aria-label (e.g., aria-label="Close") to the button
that invokes onClose and mark the decorative <X /> icon as aria-hidden="true" so
screen readers announce the button purpose but ignore the visual icon; update
the JSX for the button containing onClick={onClose} and the <X /> element
accordingly.
---
Nitpick comments:
In `@lib/activity/index.ts`:
- Around line 45-48: The try/catch around auth.api.getSession in
lib/activity/index.ts silently swallows errors; change the empty catch to handle
failures by logging the error and context and preserving normal flow: catch the
exception thrown by auth.api.getSession, call the existing logger (or create
one) to record the error and the request headers/context, and then set userEmail
= null (as currently intended) so enrichment failures are visible but non-fatal;
reference the auth.api.getSession call and the userEmail assignment when
applying the fix.
In `@lib/activity/message.ts`:
- Around line 148-188: The descriptions map is currently recreated on every call
inside getEventDescription; extract it to a top-level constant (e.g., const
EVENT_DESCRIPTIONS = { ... }) at module scope and have getEventDescription
reference EVENT_DESCRIPTIONS instead of the local descriptions variable; ensure
you keep the same keys and fallback logic with typeLabels and update any
imports/exports if needed so the mapping can be reused across the module.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/project/activity/activity-detail-panel.tsxcomponents/project/activity/activity-feed.tsxlib/activity/index.tslib/activity/message.tslib/utils/date.ts
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 by CodeRabbit
New Features
Refactor