Merged
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 50 minutes and 18 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: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughReworks the issues admin UX (shared control bar, list sorting, calendar/kanban/list pages), adds template keyword substitution with input prompts, threads team color hexes through UI and API, migrates DB enums/columns and migrations (including role color), and refactors the issue-reminder cron to target Discord channel IDs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant View as Issues View
participant Control as IssueViewControlBar
participant Dialog as Create/Edit Dialog
participant Template as Template Resolver
participant Prompt as Template Input Prompt
User->>View: Open create dialog / select template
View->>Control: Render control bar (create, filters)
Control->>Dialog: Open Create/Edit Dialog (onCreate)
Dialog->>Template: Check template (has {INPUT}?)
alt template has {INPUT}
Template-->>Prompt: Request input
Prompt->>User: Ask for substitution value
User->>Prompt: Provide value
Prompt->>Template: Return resolved root name
Template->>Dialog: Populate form with resolved names ({PARENT} resolved for children)
else no input needed
Template-->>Dialog: Apply template immediately
end
Dialog->>View: Submit -> API (ensureTeamVisible applied)
View->>API: Create/update issue(s) (visibility includes team)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/blade/src/app/admin/issues/list/page.tsx`:
- Around line 13-21: The page currently blocks access using
api.roles.hasPermission with an "and" requiring READ_ISSUES, EDIT_ISSUES,
EDIT_ISSUE_TEMPLATES and READ_ISSUE_TEMPLATES (then calls notFound()), which
mismatches the nav visibility; update the access check in hasAccess to match the
nav logic by either: (A) changing the permission check to require only
READ_ISSUES or EDIT_ISSUES (use an "or" array with "READ_ISSUES" and
"EDIT_ISSUES" or include the IS_OFFICER flag if your roles API supports it), or
(B) if you prefer stricter control, change the nav to require all four
permissions—make the change inside the hasAccess assignment
(api.roles.hasPermission) and ensure notFound() remains the fallback.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 1fae05ad-5ef2-4f90-a5fa-01147875c56d
📒 Files selected for processing (21)
apps/blade/src/app/_components/issue-calendar/calendar.tsxapps/blade/src/app/_components/issue-kanban/issues-kanban.tsxapps/blade/src/app/_components/issue-list/issues-list.tsxapps/blade/src/app/_components/issues/create-edit-dialog.tsxapps/blade/src/app/_components/issues/issue-node.tsxapps/blade/src/app/_components/issues/issue-template-dialog.tsxapps/blade/src/app/_components/issues/issue-template-keywords.tsapps/blade/src/app/_components/issues/issue-view-control-bar.tsxapps/blade/src/app/_components/navigation/reusable-user-dropdown.tsxapps/blade/src/app/_components/navigation/user-dropdown.tsxapps/blade/src/app/admin/issues/calendar/page.tsxapps/blade/src/app/admin/issues/kanban/page.tsxapps/blade/src/app/admin/issues/list/page.tsxapps/blade/src/app/admin/page.tsxapps/cron/src/crons/issue-reminders.tspackages/consts/src/index.tspackages/consts/src/issue.tspackages/db/drizzle/0002_dapper_forge.sqlpackages/db/drizzle/meta/0002_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schemas/auth.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/cron/src/crons/issue-reminders.ts (1)
346-347:⚠️ Potential issue | 🔴 CriticalCompleted issues will still get reminders.
The filter still excludes
"FINISHED", but this PR’s enum migration changes the stored value to"Finished". As written, completed issues stay in the result set and will keep generating reminder messages.Suggested fix
- where: and(isNotNull(Issue.date), ne(Issue.status, "FINISHED")), + where: and( + isNotNull(Issue.date), + ne(Issue.status, ISSUE.ISSUE_STATUS[3]), + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cron/src/crons/issue-reminders.ts` around lines 346 - 347, The query fetching issues uses ne(Issue.status, "FINISHED") which no longer matches the migrated enum value; update the filter in db.query.Issue.findMany to exclude the new enum casing (e.g., "Finished") or, better, reference the canonical enum/constant for Issue.status so finished issues are properly filtered out; locate the where clause that uses isNotNull(Issue.date) and ne(Issue.status, "...") and replace the literal with the correct enum value or constant.apps/blade/src/app/_components/issue-calendar/calendar-day-agenda.tsx (1)
51-56:⚠️ Potential issue | 🟡 MinorFinished agenda items can now be marked “Past due.”
This helper still treats
"FINISHED"as the closed state. With the new title-cased status values, completed issues stop short-circuiting here and will incorrectly render the overdue badge.Suggested fix
- if (issue.status === "FINISHED" || !issue.date) return false; + if (issue.status === "Finished" || !issue.date) return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/issue-calendar/calendar-day-agenda.tsx` around lines 51 - 56, The isOverdueIssue helper currently checks issue.status === "FINISHED" which no longer matches the title-cased status values; update isOverdueIssue to treat completed issues correctly by normalizing the status (e.g., compare issue.status.toUpperCase() === "FINISHED") or check for the title-cased value "Finished" (or use the canonical Issue status enum/value if available) before short-circuiting, so finished items do not render as overdue.apps/blade/src/app/_components/issue-calendar/calendar-issue-dialog.tsx (1)
42-50:⚠️ Potential issue | 🟡 MinorUse the renamed completed status in the overdue check.
isIssueOverdue()still skips only"FINISHED". After the enum rename, finished issues with past dates will show as overdue in this dialog.Suggested fix
- if (status === "FINISHED" || !date) return false; + if (status === "Finished" || !date) return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/issue-calendar/calendar-issue-dialog.tsx` around lines 42 - 50, The overdue check in isIssueOverdue currently ignores only the old "FINISHED" status; update the function to use the renamed completed status value from GetIssueResult["status"] (replace "FINISHED" with the new enum/member name, or check both old and new names if a migration is in progress) so completed issues are not marked overdue; adjust the conditional in isIssueOverdue accordingly and, if available, reference the canonical enum/member constant instead of a hard-coded string.
🧹 Nitpick comments (4)
apps/blade/src/app/_components/issues/create-edit-dialog.tsx (2)
459-462: Double invocation ofensureTeamVisible.
ensureTeamVisible(node.team, node.roles)is called twice on the same inputs—once for the conditional check and once for the value. This is a minor inefficiency.♻️ Compute once and reuse
+ const childVisibilityIds = ensureTeamVisible(node.team, node.roles); results.push({ // ...other fields... teamVisibilityIds: - ensureTeamVisible(node.team, node.roles).length > 0 - ? ensureTeamVisible(node.team, node.roles) + childVisibilityIds.length > 0 + ? childVisibilityIds : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/issues/create-edit-dialog.tsx` around lines 459 - 462, Compute ensureTeamVisible(node.team, node.roles) once and reuse the result for teamVisibilityIds: call ensureTeamVisible once (e.g., const visibleTeams = ensureTeamVisible(node.team, node.roles)) and then set teamVisibilityIds to visibleTeams.length > 0 ? visibleTeams : undefined; update the code around teamVisibilityIds to reference visibleTeams instead of calling ensureTeamVisible twice so you avoid the duplicate invocation.
140-143: Duplication:ensureTeamVisibleexists in both client and API.This helper is defined identically in
packages/api/src/routers/issues.ts(line 39-41). Consider extracting to a shared utility (e.g.,@forge/utils) to maintain a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/issues/create-edit-dialog.tsx` around lines 140 - 143, The function ensureTeamVisible is duplicated; extract it into a shared utility module (e.g., add an exported function ensureTeamVisible in a shared package like `@forge/utils` or similar), remove the local definitions in both create-edit-dialog.tsx and packages/api/src/routers/issues.ts, and update both files to import ensureTeamVisible from the new shared module; ensure the exported signature matches (teamId: string, roleIds?: string[] | undefined) and run/type-check to fix any import paths or types after refactor.apps/blade/src/app/_components/issues/issue-view-control-bar.tsx (1)
161-173: Potential duplicate key issue in filter pills.Using
key={tag}assumes all generated tags are unique. If a user could somehow have duplicate filter values (unlikely but theoretically possible with search terms), this would cause React key warnings.♻️ Optional: Use index-based key for safety
- {activeFilters.map((tag) => ( + {activeFilters.map((tag, index) => ( <span - key={tag} + key={`${tag}-${index}`} className="shrink-0 rounded-full border border-border bg-background/80 px-2.5 py-1 text-xs text-muted-foreground" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/issues/issue-view-control-bar.tsx` around lines 161 - 173, The active filter pills use key={tag} in the activeFilters.map which can produce duplicate React keys if the same tag appears twice; update the mapping in the IssueViewControlBar (the activeFilters.map call and its key prop) to use a stable composite key (for example combine the tag value with the index or another unique identifier) so each rendered span has a unique key (e.g., use `${tag}-${index}` style composition) to eliminate potential React key warnings.apps/blade/src/app/_components/issue-list/issues-list.tsx (1)
229-237: Potential inconsistency: empty state checksissuesbut renderssortedIssues.Line 229 checks
issues.length === 0for the empty state, but line 237 iterates oversortedIssues. SincesortedIssuesderives fromissues, this works correctly—but for readability and consistency, consider using the same variable in both places.♻️ Optional: Use sortedIssues for empty check
- {!isLoading && !error && issues.length === 0 && ( + {!isLoading && !error && sortedIssues.length === 0 && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/issue-list/issues-list.tsx` around lines 229 - 237, Empty-state check uses issues.length while the list renders sortedIssues; update the empty check to use the same derived array to avoid inconsistency. Replace the condition that currently reads issues.length === 0 with sortedIssues.length === 0 (or consistently use issues everywhere) so the empty-state branch and the mapped rendering both reference sortedIssues; ensure this change is applied in the conditional that guards rendering (the block surrounding isLoading, error, and the empty check) and run tests/preview to confirm no UI regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api/src/routers/issues.ts`:
- Around line 490-493: The current transaction deletes the entire subtree
(collectIssueSubtreeIds + tx.delete(Issue)), which diverges from standard UX;
change it to orphan descendants instead: inside db.transaction use
collectIssueSubtreeIds(input.id) or collect only descendant IDs as needed, then
run tx.update(Issue) to set parentId (or parent_id) = null for those descendant
records (reference Issue and Issue.parentId), and finally run
tx.delete(Issue).where(Issue.id.equals(input.id)) to delete only the requested
parent; ensure you keep use of db.transaction, tx.update, tx.delete,
collectIssueSubtreeIds, Issue.id and Issue.parentId symbols so behavior is
intentional and reversible.
In `@packages/api/src/routers/roles.ts`:
- Around line 21-29: fetchDiscordRoleHexColor currently returns null for both
"role has no color" and "API fetch failed", causing updateRoleLink to overwrite
stored teamHexcodeColor on transient Discord errors; change
fetchDiscordRoleHexColor so that on API/fetch errors it returns undefined (only
return null when the role exists but has no color), and update the
updateRoleLink logic to preserve the existing DB value when the helper returns
undefined by using a null-coalescing fallback that prefers the fetched value,
then the existing exist.teamHexcodeColor, then null.
In `@packages/db/drizzle/0004_youthful_galactus.sql`:
- Around line 1-8: The migration drops and recreates the enum types
("public"."issue_priority" and "public"."issue_status") but then attempts to
cast existing values back to the new enums without first mapping legacy strings,
which will fail; update rows in table "knight_hacks_issue" while the columns
"priority" and "status" are temporarily of type text to translate any old enum
label variants to the new exact enum labels (for both "priority" and "status")
before running the ALTER ... SET DATA TYPE ... USING ... casts so the cast
succeeds; locate the ALTER TABLE blocks for "knight_hacks_issue" and the
DROP/CREATE of "public"."issue_priority" and "public"."issue_status" to insert
the necessary UPDATE statements that normalize existing values to the new enum
strings.
---
Outside diff comments:
In `@apps/blade/src/app/_components/issue-calendar/calendar-day-agenda.tsx`:
- Around line 51-56: The isOverdueIssue helper currently checks issue.status ===
"FINISHED" which no longer matches the title-cased status values; update
isOverdueIssue to treat completed issues correctly by normalizing the status
(e.g., compare issue.status.toUpperCase() === "FINISHED") or check for the
title-cased value "Finished" (or use the canonical Issue status enum/value if
available) before short-circuiting, so finished items do not render as overdue.
In `@apps/blade/src/app/_components/issue-calendar/calendar-issue-dialog.tsx`:
- Around line 42-50: The overdue check in isIssueOverdue currently ignores only
the old "FINISHED" status; update the function to use the renamed completed
status value from GetIssueResult["status"] (replace "FINISHED" with the new
enum/member name, or check both old and new names if a migration is in progress)
so completed issues are not marked overdue; adjust the conditional in
isIssueOverdue accordingly and, if available, reference the canonical
enum/member constant instead of a hard-coded string.
In `@apps/cron/src/crons/issue-reminders.ts`:
- Around line 346-347: The query fetching issues uses ne(Issue.status,
"FINISHED") which no longer matches the migrated enum value; update the filter
in db.query.Issue.findMany to exclude the new enum casing (e.g., "Finished") or,
better, reference the canonical enum/constant for Issue.status so finished
issues are properly filtered out; locate the where clause that uses
isNotNull(Issue.date) and ne(Issue.status, "...") and replace the literal with
the correct enum value or constant.
---
Nitpick comments:
In `@apps/blade/src/app/_components/issue-list/issues-list.tsx`:
- Around line 229-237: Empty-state check uses issues.length while the list
renders sortedIssues; update the empty check to use the same derived array to
avoid inconsistency. Replace the condition that currently reads issues.length
=== 0 with sortedIssues.length === 0 (or consistently use issues everywhere) so
the empty-state branch and the mapped rendering both reference sortedIssues;
ensure this change is applied in the conditional that guards rendering (the
block surrounding isLoading, error, and the empty check) and run tests/preview
to confirm no UI regressions.
In `@apps/blade/src/app/_components/issues/create-edit-dialog.tsx`:
- Around line 459-462: Compute ensureTeamVisible(node.team, node.roles) once and
reuse the result for teamVisibilityIds: call ensureTeamVisible once (e.g., const
visibleTeams = ensureTeamVisible(node.team, node.roles)) and then set
teamVisibilityIds to visibleTeams.length > 0 ? visibleTeams : undefined; update
the code around teamVisibilityIds to reference visibleTeams instead of calling
ensureTeamVisible twice so you avoid the duplicate invocation.
- Around line 140-143: The function ensureTeamVisible is duplicated; extract it
into a shared utility module (e.g., add an exported function ensureTeamVisible
in a shared package like `@forge/utils` or similar), remove the local definitions
in both create-edit-dialog.tsx and packages/api/src/routers/issues.ts, and
update both files to import ensureTeamVisible from the new shared module; ensure
the exported signature matches (teamId: string, roleIds?: string[] | undefined)
and run/type-check to fix any import paths or types after refactor.
In `@apps/blade/src/app/_components/issues/issue-view-control-bar.tsx`:
- Around line 161-173: The active filter pills use key={tag} in the
activeFilters.map which can produce duplicate React keys if the same tag appears
twice; update the mapping in the IssueViewControlBar (the activeFilters.map call
and its key prop) to use a stable composite key (for example combine the tag
value with the index or another unique identifier) so each rendered span has a
unique key (e.g., use `${tag}-${index}` style composition) to eliminate
potential React key warnings.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 1e8b9571-beaf-466e-8354-4dc204e12147
📒 Files selected for processing (25)
apps/blade/src/app/_components/issue-calendar/calendar-day-agenda.tsxapps/blade/src/app/_components/issue-calendar/calendar-issue-dialog.tsxapps/blade/src/app/_components/issue-calendar/calendar.tsxapps/blade/src/app/_components/issue-kanban/issues-kanban.tsxapps/blade/src/app/_components/issue-list/issues-list.tsxapps/blade/src/app/_components/issues/create-edit-dialog.tsxapps/blade/src/app/_components/issues/issue-fetcher-pane.tsxapps/blade/src/app/_components/issues/issue-node.tsxapps/blade/src/app/_components/issues/issue-template-dialog.tsxapps/blade/src/app/_components/issues/issue-view-control-bar.tsxapps/blade/src/app/_components/navigation/reusable-user-dropdown.tsxapps/blade/src/app/_components/navigation/user-dropdown.tsxapps/blade/src/app/admin/issues/[id]/page.tsxapps/blade/src/app/admin/issues/list/page.tsxapps/blade/src/app/admin/page.tsxapps/cron/src/crons/issue-reminders.tspackages/api/src/routers/issues.tspackages/api/src/routers/roles.tspackages/consts/src/issue.tspackages/db/drizzle/0003_hot_killraven.sqlpackages/db/drizzle/0004_youthful_galactus.sqlpackages/db/drizzle/meta/0003_snapshot.jsonpackages/db/drizzle/meta/0004_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schemas/auth.ts
✅ Files skipped from review due to trivial changes (5)
- packages/db/drizzle/0003_hot_killraven.sql
- apps/blade/src/app/_components/issues/issue-template-dialog.tsx
- apps/blade/src/app/_components/navigation/user-dropdown.tsx
- packages/db/drizzle/meta/_journal.json
- packages/db/drizzle/meta/0003_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/blade/src/app/_components/issues/issue-node.tsx
- apps/blade/src/app/_components/navigation/reusable-user-dropdown.tsx
- apps/blade/src/app/admin/page.tsx
- apps/blade/src/app/admin/issues/list/page.tsx
- packages/db/src/schemas/auth.ts
- packages/consts/src/issue.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.
Why
There were some loose features that still needed to be implemented, and the fragmented pages needed to be standardized
What
Test Plan
here are some screenshots (notice shared dock and nav states):
here is a video of the {PARENT} and {INPUT} parsing:
2026-04-13.15-12-25.mp4
Checklist
pnpm db:generateand committed the generated files inpackages/db/drizzle/Summary by CodeRabbit
New Features
Improvements