Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a capacity-planning dashboard and supporting types/hooks/store, extends assets with capacity readiness and max-hours-per-day (forms + normalization), persists per-project capacity window intent, and standardizes logo Image width styling across several UI components. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant CapacityPage as "CapacityPlanning Page"
participant Dashboard as "CapacityDashboard"
participant ProjectSel as "useResolvedProjectSelection"
participant Store as "useUIIntentStore"
participant Hook as "useCapacityDashboard"
participant API as "fetchCapacityDashboard"
participant Backend as Backend
User->>CapacityPage: Navigate to /capacity-planning
CapacityPage->>Dashboard: render
Dashboard->>ProjectSel: resolve projectId
ProjectSel-->>Dashboard: projectId
Dashboard->>Store: getCapacityIntent(scopeKey)
Store-->>Dashboard: windowSize ("2W"/"4W")
Dashboard->>Hook: request capacityData(projectId,startWeek,weeks)
Hook->>API: fetchCapacityDashboard(params)
API->>Backend: GET /lookahead/{projectId}/capacity-dashboard?start_week=X&weeks=Y
Backend-->>API: raw payload
API-->>Hook: normalized CapacityDashboardResponse
Hook-->>Dashboard: capacityData / loading / error / mutate
alt Data available
Dashboard-->>User: render grid, summaries, diagnostics
else Loading/Error
Dashboard-->>User: show skeleton or error with Retry
end
User->>Store: setCapacityWindowSize(scopeKey,newWindow)
Store->>Dashboard: (state update) -> Dashboard triggers Hook refetch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/app/(auth)/set-password/page.tsx (1)
25-37: Consider extracting the logo into a shared component.The identical logo configuration is now duplicated across 5 auth pages (login, register, forgot-password, reset-password, set-password). Extracting this into a shared
AuthLogocomponent would reduce duplication and make future styling changes easier to maintain.♻️ Example shared component
Create a new component at
src/components/auth/AuthLogo.tsx:import Link from "next/link"; import Image from "next/image"; export function AuthLogo() { return ( <Link href="/" className="inline-flex items-center"> <Image src="/full-logo-dark.svg" alt="SiteSpace" width={160} height={48} className="h-10 brightness-0 invert" style={{ width: "auto" }} priority /> </Link> ); }Then simplify each auth page:
-<Link href="/" className="inline-flex items-center"> - <Image - src="/full-logo-dark.svg" - alt="SiteSpace" - width={160} - height={48} - className="h-10 brightness-0 invert" - style={{ width: "auto" }} - priority - /> -</Link> +<AuthLogo />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(auth)/set-password/page.tsx around lines 25 - 37, Duplicate logo markup across auth pages should be extracted into a shared component: create an AuthLogo component (e.g., export function AuthLogo) that returns the Link/Image JSX shown, then replace the inline div/Link/Image block in set-password page (and the other auth pages: login, register, forgot-password, reset-password) with an import and usage of <AuthLogo />; ensure the new component preserves props/className/style/priority (the same src, alt, width, height, className, style) so styling and behavior remain identical.src/stores/uiIntentStore.ts (2)
135-155: Reduce duplication between lookahead/capacity intent parsers.
toCapacityIntentRecordandtoLookaheadIntentRecordare nearly identical. A small generic helper would lower drift risk when future window options change.♻️ Proposed refactor
+const toWindowIntentRecord = <TWindow extends string>( + value: unknown, + isValidWindow: (v: unknown) => v is TWindow, + fallback: TWindow, +): Record<string, { windowSize: TWindow }> => { + if (!isRecord(value)) return {}; + const entries = Object.entries(value) + .map(([scopeKey, rawIntent]) => { + if (!isRecord(rawIntent)) return null; + const windowSize = isValidWindow(rawIntent.windowSize) + ? rawIntent.windowSize + : fallback; + return [scopeKey, { windowSize }] as const; + }) + .filter( + (entry): entry is readonly [string, { windowSize: TWindow }] => + Boolean(entry), + ); + return Object.fromEntries(entries); +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/uiIntentStore.ts` around lines 135 - 155, Both toCapacityIntentRecord and toLookaheadIntentRecord share parsing logic; create a small generic helper (e.g., parseIntentRecord<TWindow> or buildWindowIntentRecord) that accepts the raw value, a type guard for valid window values (or a set/enum), a default window constant (DEFAULT_CAPACITY_WINDOW / DEFAULT_LOOKAHEAD_WINDOW), and a mapper to produce the intent object shape, then replace toCapacityIntentRecord and toLookaheadIntentRecord to call this helper. Locate the logic inside toCapacityIntentRecord (use symbols: toCapacityIntentRecord, isRecord, DEFAULT_CAPACITY_WINDOW, CapacityWindowSize) and its counterpart to implement the generic function and ensure the helper returns Record<string, IntentType> and preserves the current null-filtering and Object.fromEntries behavior.
24-24: BumpUI_INTENT_STORE_VERSIONwhen adding new persisted fields to ensure existing clients apply migrations.The
migratecallback is only invoked when the stored version differs from the configured version. Since version stays at1, clients with existing v1 storage won't trigger migration if new persisted fields are added to the schema. While the current migrate function validates all 8 fields with sensible defaults, bumping the version ensures the normalization path runs for all clients on schema changes.Also applies to: 378-392
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/uiIntentStore.ts` at line 24, The UI_INTENT_STORE_VERSION constant must be incremented so the migrate callback runs for clients with existing v1 storage when you add new persisted fields; locate the UI_INTENT_STORE_VERSION declaration and bump its numeric value (and any other occurrences of that constant in this file, e.g., the other UI intent store version references around the migrate logic) so stored versions differ and trigger the existing migrate/normalization path for all clients using the store.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/forms/CreateAssetForm.tsx`:
- Around line 248-252: The current logic converts asset.maxHoursPerDay to Number
and always clamps it even when the parse yields NaN, which can silently submit
0.5; update the block around asset.maxHoursPerDay so you first parse the value
(e.g., const parsed = Number(asset.maxHoursPerDay)) and check
Number.isFinite(parsed) before calling clampAssetMaxHoursPerDay and assigning
createRequest.max_hours_per_day; leave the field unset when the input is
non-finite or invalid to avoid sending unintended defaults.
In `@src/components/lookahead/CapacityDashboard.tsx`:
- Around line 705-711: The map in CapacityDashboard passes
capacityData.rows[assetType]?.[week] which can be undefined and causes
CapacityCellCard to crash when it reads cell.status; fix by providing a safe
fallback object or a guard: when mapping weeks, replace
cell={capacityData.rows[assetType]?.[week]} with a fallback cell (e.g., an
object with default status/values) or alternatively update CapacityCellCard to
check for undefined cell before accessing properties (adjust the
CapacityCellCard component to handle a possibly undefined prop). Ensure
references to CapacityCellCard, capacityData.rows, assetType, weeks, and
compactMode are used so reviewers can locate and verify the change.
- Around line 723-728: The map over weeks is casting
capacityData.summary_by_week[week] to CapacityWeekSummary even when it may be
undefined; update the mapping that renders CapacityWeekSummaryCell to guard
against missing summaries by providing a safe fallback or skipping the cell:
compute a default CapacityWeekSummary (matching the fields read later such as
worst_status) or check for undefined and pass a fallback prop (or null/undefined
if CapacityWeekSummaryCell handles it). Locate the map rendering
(weeks.map(...)) and the CapacityWeekSummaryCell usage, remove the unconditional
"as CapacityWeekSummary" cast, and ensure the component receives a validated
summary variable (or skip rendering) so accesses like summary.worst_status in
CapacityWeekSummaryCell (referenced around line 216) cannot throw.
---
Nitpick comments:
In `@src/app/`(auth)/set-password/page.tsx:
- Around line 25-37: Duplicate logo markup across auth pages should be extracted
into a shared component: create an AuthLogo component (e.g., export function
AuthLogo) that returns the Link/Image JSX shown, then replace the inline
div/Link/Image block in set-password page (and the other auth pages: login,
register, forgot-password, reset-password) with an import and usage of <AuthLogo
/>; ensure the new component preserves props/className/style/priority (the same
src, alt, width, height, className, style) so styling and behavior remain
identical.
In `@src/stores/uiIntentStore.ts`:
- Around line 135-155: Both toCapacityIntentRecord and toLookaheadIntentRecord
share parsing logic; create a small generic helper (e.g.,
parseIntentRecord<TWindow> or buildWindowIntentRecord) that accepts the raw
value, a type guard for valid window values (or a set/enum), a default window
constant (DEFAULT_CAPACITY_WINDOW / DEFAULT_LOOKAHEAD_WINDOW), and a mapper to
produce the intent object shape, then replace toCapacityIntentRecord and
toLookaheadIntentRecord to call this helper. Locate the logic inside
toCapacityIntentRecord (use symbols: toCapacityIntentRecord, isRecord,
DEFAULT_CAPACITY_WINDOW, CapacityWindowSize) and its counterpart to implement
the generic function and ensure the helper returns Record<string, IntentType>
and preserves the current null-filtering and Object.fromEntries behavior.
- Line 24: The UI_INTENT_STORE_VERSION constant must be incremented so the
migrate callback runs for clients with existing v1 storage when you add new
persisted fields; locate the UI_INTENT_STORE_VERSION declaration and bump its
numeric value (and any other occurrences of that constant in this file, e.g.,
the other UI intent store version references around the migrate logic) so stored
versions differ and trigger the existing migrate/normalization path for all
clients using the store.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea3441d7-f67a-4c0b-b958-02c738ed1370
📒 Files selected for processing (25)
src/app/(auth)/forgot-password/page.tsxsrc/app/(auth)/login/page.tsxsrc/app/(auth)/register/page.tsxsrc/app/(auth)/reset-password/page.tsxsrc/app/(auth)/set-password/page.tsxsrc/app/(dashboard)/assets/page.tsxsrc/app/(dashboard)/capacity-planning/page.tsxsrc/app/(dashboard)/subcontractors/page.tsxsrc/components/SideNav.tsxsrc/components/forms/CreateAssetForm.tsxsrc/components/forms/UpdateAssetForm.tsxsrc/components/landing/LandingPage.tsxsrc/components/landing/TopBar.tsxsrc/components/lookahead/CapacityDashboard.tsxsrc/components/lookahead/capacityUtils.tssrc/components/ui/dialog.tsxsrc/hooks/lookahead/api.test.tssrc/hooks/lookahead/api.tssrc/hooks/lookahead/keys.tssrc/hooks/lookahead/useLookaheadQueries.tssrc/lib/apiNormalization.tssrc/lib/formOptions.tssrc/lib/subcontractorNormalization.tssrc/stores/uiIntentStore.tssrc/types/index.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/components/lookahead/CapacityDashboard.tsx (2)
408-412: Consider early return to prevent content flash during redirect.The component continues rendering the dashboard UI while the redirect is pending, which may cause a brief flash of unauthorized content for subcontractor users.
♻️ Optional: Add early return during redirect
+ const isSubcontractor = user?.role === "subcontractor"; + useEffect(() => { - if (user?.role === "subcontractor") { + if (isSubcontractor) { router.replace("/home"); } - }, [router, user?.role]); + }, [router, isSubcontractor]); + + if (isSubcontractor) { + return null; // or a loading spinner + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/lookahead/CapacityDashboard.tsx` around lines 408 - 412, The component currently triggers router.replace("/home") inside useEffect but continues rendering the dashboard until navigation completes, causing a flash; update the component to perform an early return when the user is a subcontractor (check user?.role === "subcontractor") or when you set a local redirecting flag inside the useEffect that calls router.replace, so the render path returns null or a loading/placeholder immediately (before the rest of the dashboard JSX renders); reference the existing useEffect, router.replace, and user?.role to locate and implement this early-return or redirecting state.
74-81: Consider timezone consistency forcurrentMondayISO.The function uses
new Date()which depends on the local timezone. If the client renders just after midnight in one timezone while the server snapshot was taken in another, there could be a brief mismatch. Since this is a client component and the value is only used after mount viauseMemo, the practical impact is low.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/lookahead/CapacityDashboard.tsx` around lines 74 - 81, The currentMondayISO function relies on local time (new Date()), causing potential cross-timezone off-by-one-day issues; change it to compute the Monday date in UTC to ensure consistency: use the Date UTC getters (getUTCDate/getUTCDay/getUTCFullYear/getUTCMonth) or construct a Date from Date.now() in UTC and compute daysToMonday based on getUTCDay, then return the ISO-style "YYYY-MM-DD" string (e.g., via toISOString().slice(0,10)) so currentMondayISO always returns the same calendar day across timezones.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/forms/CreateAssetForm.tsx`:
- Around line 248-251: The code converts asset.maxHoursPerDay to a Number which
turns an empty string into 0 (then clamped to 0.5); change the guard to first
detect an empty/whitespace string and skip setting
createRequest.max_hours_per_day in that case. In other words, before calling
Number(asset.maxHoursPerDay) or clampAssetMaxHoursPerDay, check that
asset.maxHoursPerDay is not an empty string (e.g. non-empty after trim) and only
then parse to Number and apply Number.isFinite(parsedMaxHours) and
clampAssetMaxHoursPerDay; leave createRequest.max_hours_per_day undefined when
the input is empty.
In `@src/components/lookahead/CapacityDashboard.tsx`:
- Around line 726-735: The map over weeks returns null for missing summaries
which removes grid children and breaks alignment; update the mapping in the
component that renders CapacityWeekSummaryCell (the weeks.map block using
capacityData.summary_by_week) to render a placeholder cell when weekSummary is
falsy instead of returning null — either render the same placeholder div used by
CapacityCellCard for undefined cells or a minimal element with the same
class/structure as CapacityWeekSummaryCell so the grid keeps a consistent number
of children and alignment.
- Line 504: In CapacityDashboard update the container div currently using
className "mx-auto max-w-screen space-y-6" to use a valid Tailwind max-width
utility (e.g., replace "max-w-screen" with "max-w-screen-2xl" to match other
files or "max-w-full" if you want full width); locate the div in the
CapacityDashboard component and change the className to the chosen valid utility
so the layout matches the rest of the codebase.
---
Nitpick comments:
In `@src/components/lookahead/CapacityDashboard.tsx`:
- Around line 408-412: The component currently triggers router.replace("/home")
inside useEffect but continues rendering the dashboard until navigation
completes, causing a flash; update the component to perform an early return when
the user is a subcontractor (check user?.role === "subcontractor") or when you
set a local redirecting flag inside the useEffect that calls router.replace, so
the render path returns null or a loading/placeholder immediately (before the
rest of the dashboard JSX renders); reference the existing useEffect,
router.replace, and user?.role to locate and implement this early-return or
redirecting state.
- Around line 74-81: The currentMondayISO function relies on local time (new
Date()), causing potential cross-timezone off-by-one-day issues; change it to
compute the Monday date in UTC to ensure consistency: use the Date UTC getters
(getUTCDate/getUTCDay/getUTCFullYear/getUTCMonth) or construct a Date from
Date.now() in UTC and compute daysToMonday based on getUTCDay, then return the
ISO-style "YYYY-MM-DD" string (e.g., via toISOString().slice(0,10)) so
currentMondayISO always returns the same calendar day across timezones.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ef82c62-df87-466a-b6ec-2677ff95c714
📒 Files selected for processing (2)
src/components/forms/CreateAssetForm.tsxsrc/components/lookahead/CapacityDashboard.tsx
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/lookahead/CapacityDashboard.tsx`:
- Around line 51-53: The type guard isCapacityWindowSize currently uses `value
in WINDOW_WEEKS`, which returns true for inherited properties and can let
corrupted persisted values bypass fallbacks; update the check to verify
WINDOW_WEEKS has its own property (e.g., use
Object.prototype.hasOwnProperty.call(WINDOW_WEEKS, value as string) or
Object.hasOwn if available) while keeping the `typeof value === "string"` check
so the guard only returns true for actual keys of WINDOW_WEEKS.
- Around line 431-437: The capacity query is running with the default "4W"
before the persisted UI intent hydrates; update the component to avoid that by
either (A) initializing the local windowSize from the store synchronously using
useUIIntentStore.getState().getCapacityIntent(uiScopeKey) when you create the
state used by setWindowSizeLocal (so use the persisted windowSize if present and
valid via isCapacityWindowSize), or (B) prevent useCapacityDashboard from
running until the scoped intent is resolved by gating the query with
hasUIIntentHydrated && !!uiScopeKey; apply the same pattern to the other
instances noted around the useEffect (lines ~439 and ~460-470) to eliminate the
unnecessary initial refetch. Ensure you reference and update the state variable
consumed by useCapacityDashboard and the useEffect that currently calls
setWindowSizeLocal.
- Around line 408-412: The current redirect using router.replace("/home") in the
useEffect for subcontractors is async and doesn't stop queries from running;
modify the query hook options for useLookaheadSnapshot and useCapacityDashboard
to include a subcontractor role check in their enabled flags (e.g.,
useLookaheadSnapshot({ enabled: Boolean(projectId) && user?.role !==
"subcontractor", ... }) and similarly for useCapacityDashboard) so queries do
not execute for subcontractor users and prevent the brief dashboard flash; apply
the same enabled-role guard to the other hook usages mentioned (lines around
440-441 and 466-470).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c150d3ad-dad2-473b-9812-a2e200de8a19
📒 Files selected for processing (2)
src/components/forms/CreateAssetForm.tsxsrc/components/lookahead/CapacityDashboard.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/lookahead/CapacityDashboard.tsx (1)
726-742: Grid alignment fix is correctly implemented, but placeholder height is slightly inconsistent.The missing summary is now handled with a placeholder
divinstead of returningnull, which correctly maintains grid alignment.Minor observation: the placeholder uses
min-h-28(line 732) whileCapacityWeekSummaryCellusesmin-h-24(line 223). In CSS Grid this won't break alignment since row height is determined by the tallest cell, but for visual consistency, consider usingmin-h-24to match.♻️ Optional: align placeholder height
if (!weekSummary) { return ( <div key={`summary-${week}`} - className="relative flex min-h-28 flex-col justify-between p-3 bg-slate-50" + className="flex min-h-24 flex-col justify-between p-3 bg-slate-50" /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/lookahead/CapacityDashboard.tsx` around lines 726 - 742, The placeholder div returned inside weeks.map when a weekSummary is missing uses className "min-h-28" causing a visual mismatch with the real cell; change the placeholder's height to match CapacityWeekSummaryCell by replacing "min-h-28" with "min-h-24" in the placeholder div so both components use the same minimum height and keep grid cells visually consistent (locate this in the weeks.map block that renders the placeholder and the CapacityWeekSummaryCell).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/lookahead/CapacityDashboard.tsx`:
- Around line 726-742: The placeholder div returned inside weeks.map when a
weekSummary is missing uses className "min-h-28" causing a visual mismatch with
the real cell; change the placeholder's height to match CapacityWeekSummaryCell
by replacing "min-h-28" with "min-h-24" in the placeholder div so both
components use the same minimum height and keep grid cells visually consistent
(locate this in the weeks.map block that renders the placeholder and the
CapacityWeekSummaryCell).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31ac31d1-5b66-4dca-b46b-5fbefd62bc0b
📒 Files selected for processing (1)
src/components/lookahead/CapacityDashboard.tsx
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
Improvements
Tests