-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add visible header to manual metric dialog #278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds Google Sheets range-editing UI and wiring (GSheetsRangeEditor + drawer/card changes), extracts Google Sheets A1 utilities into a shared module, adds a server mutation to update endpoint config and regenerate metric data, and makes a small UI text change in the Manual Metric dialog. Changes
Sequence Diagram(s)mermaid Client->>FrontendAPI: call updateEndpointConfigAndRegenerate(metricId, endpointConfig) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/metric/_components/manual/ManualMetricDialog.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use the tRPC dual API pattern: for server components, directly call tRPC server API (10x faster); for client components, use React hooks with TanStack Query
Use inline type imports with @trivago/prettier-plugin-sort-imports for consistent import orderingUse TypeScript 5.9 with strict type checking for all frontend and backend code
Files:
src/app/metric/_components/manual/ManualMetricDialog.tsx
src/app/metric/_components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/app/metric/_components/**/*.tsx: Metric dialogs should follow the pattern: [Provider]MetricDialog.tsx (dialog wrapper) + [Provider]MetricContent.tsx (form content) and be registered in src/app/metric/_components/index.ts
Use MetricDialogBase from base/ for shared metric dialog functionality to reduce duplication across provider-specific dialogs
Consider factory pattern for metric dialogs to reduce the 5 nearly identical wrapper components (currently duplicated across providers)
Files:
src/app/metric/_components/manual/ManualMetricDialog.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Prefer Server Components for initial data fetching; use Client Components ('use client') only for interactivity
Files:
src/app/metric/_components/manual/ManualMetricDialog.tsx
src/**/*/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Client Components must use
import { api } from '@/trpc/react'for standard HTTP/Hooks wrapper
Files:
src/app/metric/_components/manual/ManualMetricDialog.tsx
**/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Use Tailwind CSS 4 for styling with shadcn/ui and Radix UI primitive components
Files:
src/app/metric/_components/manual/ManualMetricDialog.tsx
🧠 Learnings (4)
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Metric dialogs should follow the pattern: [Provider]MetricDialog.tsx (dialog wrapper) + [Provider]MetricContent.tsx (form content) and be registered in src/app/metric/_components/index.ts
Applied to files:
src/app/metric/_components/manual/ManualMetricDialog.tsx
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Use MetricDialogBase from base/ for shared metric dialog functionality to reduce duplication across provider-specific dialogs
Applied to files:
src/app/metric/_components/manual/ManualMetricDialog.tsx
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Consider factory pattern for metric dialogs to reduce the 5 nearly identical wrapper components (currently duplicated across providers)
Applied to files:
src/app/metric/_components/manual/ManualMetricDialog.tsx
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/dashboard/**/*.tsx : Consolidate dashboard metric card duplication: add readOnly mode to dashboard-metric-card.tsx instead of maintaining separate public-dashboard-metric-card.tsx component
Applied to files:
src/app/metric/_components/manual/ManualMetricDialog.tsx
🧬 Code graph analysis (1)
src/app/metric/_components/manual/ManualMetricDialog.tsx (1)
src/components/ui/dialog.tsx (1)
DialogTitle(143-143)
| <DialogHeader> | ||
| <DialogTitle className="sr-only">Create Manual Metric</DialogTitle> | ||
| <DialogTitle>Create Manual KPI</DialogTitle> | ||
| <p className="text-muted-foreground text-sm"> | ||
| Track metrics that you update manually on a regular cadence | ||
| </p> | ||
| </DialogHeader> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# Check the structure of ManualMetricDialog.tsx and other metric dialogs
cd src/app/metric/_components || exit 1
# List all metric dialog files
echo "=== Metric dialog files structure ==="
find . -name "*MetricDialog.tsx" -type f
echo -e "\n=== ManualMetricDialog.tsx content ==="
head -60 manual/ManualMetricDialog.tsx
echo -e "\n=== Checking for integration metric dialog ==="
find . -name "*IntegrationMetricDialog.tsx" -o -name "*Integration*" | head -10Repository: drifter089/orgOS
Length of output: 1832
🏁 Script executed:
# Search for all DialogHeader usage in metric components
rg -A 8 "<DialogHeader>" src/app/metric/_components/ --type tsxRepository: drifter089/orgOS
Length of output: 87
🏁 Script executed:
# Check the metric components index.ts for registration
cat src/app/metric/_components/index.tsRepository: drifter089/orgOS
Length of output: 558
🏁 Script executed:
# Search for DialogHeader patterns in all metric dialog files with better flags
rg "DialogHeader" -A 8 src/app/metric/_components/ -g "*.tsx"Repository: drifter089/orgOS
Length of output: 3704
🏁 Script executed:
# Also check for MetricDialogBase and its DialogHeader implementation
rg "DialogHeader" -A 8 src/app/metric/_components/base/ -g "*.tsx"Repository: drifter089/orgOS
Length of output: 1761
🏁 Script executed:
# Check if other provider dialogs use MetricDialogBase or have custom DialogHeader
for file in src/app/metric/_components/{github,google-sheets,linear,posthog,youtube}/*Dialog.tsx; do
echo "=== $(basename $file) ==="
head -20 "$file" | grep -E "import.*MetricDialogBase|from.*base"
echo ""
doneRepository: drifter089/orgOS
Length of output: 532
🏁 Script executed:
# Check the actual structure of a few provider dialogs
echo "=== GitHubMetricDialog.tsx ==="
head -80 src/app/metric/_components/github/GitHubMetricDialog.tsx
echo -e "\n=== GoogleSheetsMetricDialog.tsx ==="
head -80 src/app/metric/_components/google-sheets/GoogleSheetsMetricDialog.tsxRepository: drifter089/orgOS
Length of output: 2012
Refactor ManualMetricDialog to use MetricDialogBase for consistency and to eliminate duplication.
ManualMetricDialog bypasses MetricDialogBase and directly implements Dialog/DialogHeader, making it the only metric dialog that doesn't follow the established architectural pattern. All other provider dialogs (GitHub, YouTube, PostHog, Linear, GoogleSheets) use MetricDialogBase with title and description passed as props. This creates the exact duplication across metric dialog wrappers that the coding guidelines explicitly warn against.
Refactor ManualMetricDialog to match the pattern used by all provider dialogs:
<MetricDialogBase
integrationId="manual"
title="Create Manual KPI"
description="Track metrics that you update manually on a regular cadence"
trigger={trigger}
open={open}
onOpenChange={onOpenChange}
onSuccess={onSuccess}
teamId={teamId}
>
{(props) => <ManualMetricContent {...props} />}
</MetricDialogBase>
This ensures ManualMetricDialog uses DialogDescription from MetricDialogBase (consistent with shadcn/ui Dialog patterns) rather than a custom <p> tag, and eliminates the duplicated Dialog state management logic.
🤖 Prompt for AI Agents
In src/app/metric/_components/manual/ManualMetricDialog.tsx around lines 44-49,
ManualMetricDialog currently renders Dialog/DialogHeader/DialogTitle and a
custom <p> description directly which duplicates dialog state/structure used
elsewhere; refactor it to use MetricDialogBase by passing
integrationId="manual", title="Create Manual KPI", description="Track metrics
that you update manually on a regular cadence", and the existing
trigger/open/onOpenChange/onSuccess/teamId props, and render ManualMetricContent
via the render-prop (e.g. {(props) => <ManualMetricContent {...props} />}) so
DialogDescription and dialog state are provided by MetricDialogBase and
duplicated dialog markup is removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/server/api/routers/metric.ts (1)
288-358: Well-structured mutation with proper authorization and transactional integrity.The implementation follows established patterns in this router:
- Uses
getMetricAndVerifyAccessfor authorization- Wraps destructive operations in a transaction
- Invalidates cache before returning
- Triggers background task with
voidfor fire-and-forgetOne minor consideration: the
createmutation specifies{ timeout: 15000 }for its transaction. You may want to add a similar timeout here for consistency, especially since this transaction performs multiple delete operations.🔎 Optional: Add transaction timeout for consistency
// Delete old data points and transformers in transaction - await ctx.db.$transaction(async (tx) => { + await ctx.db.$transaction(async (tx) => { // Delete old data points await tx.metricDataPoint.deleteMany({ where: { metricId: input.metricId }, }); // ... rest of transaction - }); + }, { timeout: 15000 });src/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsx (1)
116-162: Significant code duplication with GoogleSheetsMetricContent.The
dataRangecomputation logic, selection handlers (handleCellMouseDown,handleCellMouseEnter,handleRowCheckbox,handleColumnCheckbox), and table rendering are nearly identical toGoogleSheetsMetricContent. Consider extracting a shared hook or component to reduce duplication.This is a recommended refactor that could be addressed in a follow-up PR.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsxsrc/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsxsrc/app/dashboard/[teamId]/_components/drawer/index.tssrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/lib/integrations/google-sheets-utils.tssrc/server/api/routers/metric.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use the tRPC dual API pattern: for server components, directly call tRPC server API (10x faster); for client components, use React hooks with TanStack Query
Use inline type imports with @trivago/prettier-plugin-sort-imports for consistent import orderingUse TypeScript 5.9 with strict type checking for all frontend and backend code
Files:
src/app/dashboard/[teamId]/_components/drawer/index.tssrc/server/api/routers/metric.tssrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/lib/integrations/google-sheets-utils.tssrc/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsxsrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
src/app/dashboard/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Three-stage metrics transformation pipeline: Stage 1 (API → DataPoints via DataIngestionTransformer), Stage 2 (DataPoints → ChartConfig via ChartTransformer), Stage 3 (ChartConfig → UI via DashboardMetricChart with Recharts)
Files:
src/app/dashboard/[teamId]/_components/drawer/index.tssrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
src/app/**/*.ts
📄 CodeRabbit inference engine (GEMINI.md)
src/app/**/*.ts: Server Components must useimport { api } from '@/trpc/server'for direct DB calls without HTTP overhead
For API routes with cron jobs and webhooks, ensure proper authentication and input validation
Files:
src/app/dashboard/[teamId]/_components/drawer/index.ts
src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/server/api/routers/**/*.ts: Always verify resource access using authorization helpers (getMetricAndVerifyAccess, getRoleAndVerifyAccess, getTeamAndVerifyAccess) to ensure resources belong to the user's organization in tRPC procedures
Use protectedProcedure or workspaceProcedure for new tRPC procedures instead of public procedures
Invalidate cache tags after mutations using invalidateCacheByTags(ctx.db, [tagName]) to maintain consistency
src/server/api/routers/**/*.ts: In tRPC procedures, never assume authentication; always use helpers likegetTeamAndVerifyAccessto verify access before operations
Be careful when modifying team save logic related toEditSessionmodel, which handles locking for concurrent team editing
Files:
src/server/api/routers/metric.ts
src/app/dashboard/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Consolidate dashboard metric card duplication: add readOnly mode to dashboard-metric-card.tsx instead of maintaining separate public-dashboard-metric-card.tsx component
Files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Prefer Server Components for initial data fetching; use Client Components ('use client') only for interactivity
Files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsxsrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
src/**/*/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Client Components must use
import { api } from '@/trpc/react'for standard HTTP/Hooks wrapper
Files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsxsrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
**/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Use Tailwind CSS 4 for styling with shadcn/ui and Radix UI primitive components
Files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsxsrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
src/app/metric/_components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/app/metric/_components/**/*.tsx: Metric dialogs should follow the pattern: [Provider]MetricDialog.tsx (dialog wrapper) + [Provider]MetricContent.tsx (form content) and be registered in src/app/metric/_components/index.ts
Use MetricDialogBase from base/ for shared metric dialog functionality to reduce duplication across provider-specific dialogs
Consider factory pattern for metric dialogs to reduce the 5 nearly identical wrapper components (currently duplicated across providers)
Files:
src/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsx
🧠 Learnings (11)
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/dashboard/**/*.tsx : Consolidate dashboard metric card duplication: add readOnly mode to dashboard-metric-card.tsx instead of maintaining separate public-dashboard-metric-card.tsx component
Applied to files:
src/app/dashboard/[teamId]/_components/drawer/index.tssrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsxsrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : Consolidate role node duplication: extract shared RoleNodeTemplate from role-node.tsx and public-role-node.tsx with isEditable prop instead of maintaining separate components
Applied to files:
src/app/dashboard/[teamId]/_components/drawer/index.tssrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/teams/[teamId]/**/*.{ts,tsx} : For new canvas node types: create component in teams/[teamId]/_components/, add to TeamNode union in types/canvas.ts, register in nodeTypes in team-canvas.tsx, and update serialization in canvas-serialization.ts
Applied to files:
src/app/dashboard/[teamId]/_components/drawer/index.ts
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Use MetricDialogBase from base/ for shared metric dialog functionality to reduce duplication across provider-specific dialogs
Applied to files:
src/app/dashboard/[teamId]/_components/drawer/index.tssrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/dashboard/**/*.{ts,tsx} : Three-stage metrics transformation pipeline: Stage 1 (API → DataPoints via DataIngestionTransformer), Stage 2 (DataPoints → ChartConfig via ChartTransformer), Stage 3 (ChartConfig → UI via DashboardMetricChart with Recharts)
Applied to files:
src/server/api/routers/metric.tssrc/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Metric dialogs should follow the pattern: [Provider]MetricDialog.tsx (dialog wrapper) + [Provider]MetricContent.tsx (form content) and be registered in src/app/metric/_components/index.ts
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to src/app/metric/_components/**/*.tsx : Consider factory pattern for metric dialogs to reduce the 5 nearly identical wrapper components (currently duplicated across providers)
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsxsrc/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsxsrc/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
📚 Learning: 2025-12-20T22:12:00.576Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
Learning: Applies to src/app/teams/[teamId]/**/*.ts : Canvas changes must be debounced (2s) and saved via `editSession` logic to handle concurrent team edits safely
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx
📚 Learning: 2025-12-20T22:12:00.576Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : React Flow nodes must store minimal data (e.g., just `roleId`); fetch full Role data from TanStack Query cache in the Node component to keep canvas and sidebars in sync
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx
📚 Learning: 2025-12-20T16:32:46.829Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T16:32:46.829Z
Learning: Applies to **/*canvas*.{ts,tsx} : Use React Flow primitives (BaseNode, BaseHandle, ZoomSlider) from src/components/react-flow/ for consistent canvas styling
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx
📚 Learning: 2025-12-20T22:12:00.576Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
Learning: The Metrics Pipeline processes data through 3 stages: DataIngestionTransformer (Raw API → MetricDataPoint), ChartTransformer (MetricDataPoint → Chart Config), and Visualization (Chart Config → Recharts UI)
Applied to files:
src/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx
🧬 Code graph analysis (3)
src/server/api/routers/metric.ts (4)
src/server/api/trpc.ts (1)
workspaceProcedure(151-154)src/server/api/utils/authorization.ts (1)
getMetricAndVerifyAccess(238-258)src/server/api/utils/cache-strategy.ts (1)
invalidateCacheByTags(167-183)src/server/api/routers/pipeline.ts (1)
runBackgroundTask(47-140)
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx (3)
src/trpc/react.tsx (1)
api(28-28)src/components/ui/drawer.tsx (1)
Drawer(126-126)src/components/ui/button.tsx (1)
Button(62-62)
src/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsx (1)
src/lib/integrations/google-sheets-utils.ts (4)
SelectionRange(75-80)parseA1Notation(39-62)selectionToA1Notation(22-33)columnToLetter(9-17)
🔇 Additional comments (23)
src/app/dashboard/[teamId]/_components/drawer/index.ts (1)
4-4: LGTM!Clean barrel export addition that maintains alphabetical ordering and follows the existing pattern.
src/app/dashboard/[teamId]/_components/dashboard-metric-card.tsx (7)
12-12: LGTM!Appropriate icon import for the new Edit Range button.
56-67: LGTM!State and computed values are well-defined with proper null safety using optional chaining and nullish coalescing.
125-135: LGTM!The mutation follows the established pattern in this component with optimistic updates, success/error toasts, and cache invalidation.
188-200: LGTM!The handler correctly preserves existing endpoint configuration while updating the
DATA_RANGEvalue. The type cast is appropriate for the JSON-stored config.
202-208: LGTM!Good UX practice to reset edit mode when the drawer closes, preventing stale state.
341-350: LGTM!The button visibility conditions are well-thought-out, preventing the edit action during processing or when already in edit mode.
375-378: LGTM!Edit mode props are properly wired to the drawer component, maintaining a clean separation of concerns.
src/app/metric/_components/google-sheets/GoogleSheetsMetricContent.tsx (1)
28-32: Good refactor to centralize Google Sheets utilities.Moving these utilities to a shared module enables code reuse with the new
GSheetsRangeEditorcomponent and follows the DRY principle.src/lib/integrations/google-sheets-utils.ts (5)
9-17: LGTM with minor edge case consideration.The algorithm correctly handles Excel-style column naming. Note that negative input would cause an infinite loop, but since all callers use 0-indexed array indices, this is unlikely in practice.
22-33: LGTM!Correctly converts 0-indexed coordinates to 1-indexed A1 notation format.
39-62: Functional implementation with minor edge case limitations.The regex
[A-Z]+only matches uppercase column letters. Google Sheets API typically returns uppercase, but if users manually enter lowercase ranges, parsing would fail. Also, sheet names containing!would be incorrectly split.These are unlikely edge cases for this use case, but worth noting for future reference.
67-73: LGTM!Correct inverse of
columnToLetterfunction.
75-80: LGTM!Clean interface definition for selection bounds.
src/app/dashboard/[teamId]/_components/drawer/gsheets-range-editor.tsx (5)
1-35: LGTM!Clean imports and well-defined props interface.
46-76: LGTM!State organization is clean and the query configuration appropriately matches the pattern used in
GoogleSheetsMetricContent.
98-114: LGTM!Good UX to initialize the selection from the existing range and navigate to the relevant page.
314-318: LGTM!The save handler appropriately guards against empty
dataRange.
323-522: Well-structured UI with proper loading/empty states and accessibility.The component handles loading, empty data, and saving states appropriately. Accessibility labels on checkboxes are a nice touch.
src/app/dashboard/[teamId]/_components/dashboard-metric-drawer.tsx (4)
17-21: LGTM!Clean import of
GSheetsRangeEditorvia the drawer barrel file.
37-42: LGTM!Optional props with defaults maintain backward compatibility and follow good component design by lifting edit state management to the parent.
54-57: LGTM!Appropriate defaults for the optional props.
209-227: Verify handling of missing integration connection.The
connectionIdfalls back to empty string (?? ""), which would result inGSheetsRangeEditorattempting a query with an emptyconnectionId. The query hasenabled: !!connectionId, so it won't fire, but the UI would show a loading state indefinitely or "No data available".Consider whether an early guard or different UI should be shown when
connectionIdis missing (e.g., for non-integration metrics that somehow reach this code path).
Summary
Adds a visible header with title and description to the manual metric dialog, matching the styling of integration metric dialogs.
Changes
Summary by CodeRabbit
Style
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.