feat: Sprint Management Dashboard - Bounty #14 - #31
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 58 minutes and 10 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis pull request introduces a complete Sprint Management Dashboard application. It includes a Next.js 14 frontend with TypeScript, Tailwind CSS styling, and NextAuth.js for GitHub OAuth authentication. The backend provides API routes for syncing GitHub organization repositories and issues as tasks, generating AI-powered sprint assignments via OpenAI-compatible APIs with fallback heuristics, and computing time/cost savings metrics. The UI features a landing page with sign-in, a dashboard with calendar view for team assignments, a Tinder-like swipe interface for task prioritization, metrics visualization, and GitHub task import functionality. Configuration files, type definitions, and styling are included. 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (5)
dashboard/.env.example-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorUse a non-secret-looking placeholder for
OPENAI_API_KEY.
sk-...often trips secret scanners in example files. A placeholder likeyour_openai_api_key_hereavoids noisy alerts.dashboard/README.md-77-101 (1)
77-101:⚠️ Potential issue | 🟡 MinorAdd a language tag to the fenced code block.
The architecture block should use a language identifier (for example,
text) to satisfy markdownlint MD040.Proposed fix
-``` +```text dashboard/ ├── src/ │ ├── app/ ... └── .env.example</details> </blockquote></details> <details> <summary>dashboard/src/components/PrioritySwiper.tsx-142-144 (1)</summary><blockquote> `142-144`: _⚠️ Potential issue_ | _🟡 Minor_ **Emoji mismatch in helper text.** Button uses 👎 for Low but helper text shows "👍 Low". ```diff - 👍 Low · ⭐ High · 🔥 Urgent + 👎 Low · ⭐ High · 🔥 Urgentdashboard/src/components/PrioritySwiper.tsx-30-41 (1)
30-41:⚠️ Potential issue | 🟡 MinorRapid-click vulnerability during animation.
User can click multiple priority buttons before the 300ms timeout advances the index, causing multiple
onSetPrioritycalls for the same task with different priorities.Proposed fix — guard against re-entry
const handleSwipe = useCallback( (priority: "low" | "high" | "urgent") => { - if (!task) return; + if (!task || anim) return; // block while animating setAnim(priority === "low" ? "left" : "right"); onSetPriority(task.id, priority); setTimeout(() => { setAnim(null); setIndex((i) => i + 1); }, 300); }, - [task, onSetPriority] + [task, anim, onSetPriority] );dashboard/src/components/PrioritySwiper.tsx-53-53 (1)
53-53:⚠️ Potential issue | 🟡 MinorDead code —
labelsvariable is unused.Line 89 calls
label.toLowerCase()inline instead.- const labels = task.labels.map((l) => l.toLowerCase());
🧹 Nitpick comments (3)
dashboard/src/app/page.tsx (1)
1-10: Keep the landing page server-rendered.The page is almost entirely static, but
use client+hoveredforces the whole marketing page to hydrate. Move the GitHub CTA into a small client component and handle the hover effect with CSS classes.Also applies to: 40-48
dashboard/src/app/dashboard/page.tsx (1)
35-41: Hardcoded team members — acceptable for prototype.For production, fetch members dynamically from the org. The current approach works for the demo scope.
dashboard/src/components/PrioritySwiper.tsx (1)
9-22: Type duplication withTaskinpage.tsx.
TaskItemis nearly identical to theTaskinterface in the dashboard page. Consider extracting to a shared types file to avoid drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d0350f9-7bf3-4ee9-81e2-18763a2e3efb
📒 Files selected for processing (18)
dashboard/.env.exampledashboard/README.mddashboard/next.config.jsdashboard/package.jsondashboard/src/app/api/auth/[...nextauth]/route.tsdashboard/src/app/api/metrics/route.tsdashboard/src/app/api/org/sync/route.tsdashboard/src/app/api/sprint/plan/route.tsdashboard/src/app/dashboard/page.tsxdashboard/src/app/globals.cssdashboard/src/app/layout.tsxdashboard/src/app/page.tsxdashboard/src/components/CalendarView.tsxdashboard/src/components/MetricsPanel.tsxdashboard/src/components/PrioritySwiper.tsxdashboard/src/components/TaskImport.tsxdashboard/tailwind.config.tsdashboard/tsconfig.json
| const { totalTasks, assignedByAI } = (await req.json()) as { | ||
| totalTasks: number; | ||
| assignedByAI: number; |
There was a problem hiding this comment.
assignedByAI is not actually “AI” today.
In dashboard/src/app/dashboard/page.tsx, Lines 100-101 send the count of all assigned tasks, and /api/sprint/plan can still return method: "heuristic" in Lines 50-56. That means fallback plans report full AI savings here.
| try { | ||
| const res = await fetch("/api/org/sync", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ org, accessToken: token }), | ||
| }); | ||
| const data = await res.json(); |
There was a problem hiding this comment.
Missing error handling for failed API response.
If /api/org/sync returns an error status, data.tasks will be undefined and the import silently fails with no user feedback.
Proposed fix
const res = await fetch("/api/org/sync", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ org, accessToken: token }),
});
+ if (!res.ok) {
+ const err = await res.json().catch(() => ({}));
+ throw new Error(err.error ?? `Sync failed: ${res.status}`);
+ }
const data = await res.json();Consider adding an error state and displaying it to the user.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const res = await fetch("/api/org/sync", { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ org, accessToken: token }), | |
| }); | |
| const data = await res.json(); | |
| try { | |
| const res = await fetch("/api/org/sync", { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ org, accessToken: token }), | |
| }); | |
| if (!res.ok) { | |
| const err = await res.json().catch(() => ({})); | |
| throw new Error(err.error ?? `Sync failed: ${res.status}`); | |
| } | |
| const data = await res.json(); |
| <label className="block text-xs text-gray-500 mb-1">GitHub Token</label> | ||
| <input | ||
| type="password" | ||
| value={token} | ||
| onChange={(e) => setToken(e.target.value)} | ||
| placeholder="ghp_..." | ||
| className="w-full px-3 py-2 text-sm bg-gray-800 border border-gray-700 rounded-lg focus:outline-none focus:ring-1 focus:ring-brand-500 placeholder:text-gray-600" | ||
| /> |
There was a problem hiding this comment.
Minimize PAT collection in the browser UI.
Prompting users to paste personal access tokens increases credential exposure risk. Prefer using the authenticated OAuth session server-side for org sync calls.
Also applies to: 105-107
- Add eslint and eslint-config-next devDependencies - Validate OAuth env vars at startup with clear error messages - Remove accessToken from client-side session (security) - Add input validation and bounds checking in /api/metrics - Return structured warnings for partial sync failures in /api/org/sync - Add auth requirement to /api/sprint/plan endpoint - Add 30s timeout to upstream AI call via AbortSignal - Validate AI model output (known members/task IDs only) - Add error handling for fetch responses in dashboard page - Add error state in MetricsPanel component - Require token before submit in TaskImport form
Closes #14
Sprint Management Dashboard ($1,800 Bounty)
Features
Tech Stack
Files Created (18 files, 1396 lines)