Conversation
📝 WalkthroughWalkthroughAdds a "custom" quota type: backend validates and creates quotas without numeric Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend as Frontend UI
participant API as Quota API
participant DB as Database
participant Audit as Audit Log
Frontend->>API: POST /workspace/:id/activity/quotas/new (payload with type)
API->>API: determine isCustom, parse value, validate inputs
alt non-custom quota
API->>DB: create Quota with value and sessionType
else custom quota
API->>DB: create Quota without value/sessionType
end
API->>DB: associate roles/departments if provided
DB-->>API: return full Quota with relations
API->>Audit: record quota creation
API-->>Frontend: 201 Created (Quota object)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/workspace/[id]/quotas.tsx (1)
552-569:⚠️ Potential issue | 🟡 MinorAvoid “null custom per timeframe” in the manage list.
With
valuenullable andtypes.custom = "custom", the manage list still interpolates{quota.value} {types[quota.type]}. For custom quotas this renders poorly. Consider a custom label instead.📝 Suggested display tweak
- <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400"> - {quota.value} {types[quota.type]} per timeframe - </p> + {quota.type === "custom" ? ( + <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400"> + Custom quota + </p> + ) : ( + <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400"> + {quota.value} {types[quota.type]} per timeframe + </p> + )}
🤖 Fix all issues with AI agents
In `@pages/api/workspace/`[id]/activity/quotas/new.ts:
- Around line 27-45: The current validation lets non-finite numbers
(NaN/Infinity) and ambiguous roles/departments states pass through; update the
check around the request body so value is validated with Number.isFinite(value)
and (optionally) Number.isInteger(value) when type !== "custom" and
convert/assign quotaData.value using the already-validated numeric value (or
parseInt from a validated string) in the quota creation block; also simplify the
roles/departments guard to require that at least one of roles or departments is
an array with length>0 (instead of the complex mixed boolean expression) so a
request with one missing and the other empty no longer slips through, and keep
the sessionType assignment conditional on !isCustom as currently written.
In `@pages/workspace/`[id]/quotas.tsx:
- Around line 388-395: The custom-quota branch in the myQuotasWithProgress
mapping sets currentValue/value to null but doesn’t signal the UI to hide the
progress block, causing a " / " fraction and an empty bar; update the mapping in
myQuotas.map (the quota.type === "custom" branch) to include a clear flag or
display value such as showProgress: false and displayValue: "N/A" (or value:
"N/A"), and then change the progress-rendering code to check showProgress (or
whether value is a valid number) before rendering the fraction/bar so custom
quotas render "N/A" or no progress UI instead of an empty fraction.
In `@prisma/schema.prisma`:
- Around line 393-397: Quota.value is now optional so any percentage math or
string interpolation using quota.value must guard against null/undefined; update
all places (e.g., the reset handler in
pages/api/workspace/[id]/activity/reset.ts, pages/workspace/[id]/quotas.tsx,
pages/workspace/[id]/activity/index.tsx, and components/profile/quotas.tsx)
where you compute (currentValue / quota.value) * 100 or embed `${quota.value}`
to instead use a null-safe alternative (e.g., use quota.value ?? 0 for the
divisor or conditionally render a placeholder like '—' and avoid division by
zero), and clamp or short-circuit percent results to a sensible default when
quota.value is null/0 to prevent NaN/Infinity.
|
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pages/workspace/[id]/quotas.tsx (2)
552-570:⚠️ Potential issue | 🟡 MinorAvoid rendering
null custom per timeframein the Manage Quotas list.Custom quotas have
value = null, but the manage list still renders{quota.value} {types[quota.type]} per timeframe, which will display a confusing “null custom…”. Consider a custom-only label there.✅ Suggested UI guard (Manage Quotas card)
- <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400"> - {quota.value} {types[quota.type]} per timeframe - </p> + {quota.type === "custom" ? ( + <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400"> + Custom quota (manual tracking) + </p> + ) : ( + <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400"> + {quota.value} {types[quota.type]} per timeframe + </p> + )}
1053-1098:⚠️ Potential issue | 🟠 MajorFix validation logic to prevent blocking custom quota submissions when
requirementfield is unmounted.The
requirementfield is conditionally rendered only whenwatchedType !== "custom", but React Hook Form keeps it registered withrequired: trueby default. When users select "custom" type and submit, validation fails because the unmounted field is still marked as required.Apply one of these fixes:
- Set
shouldUnregister: truein theuseFormoptions so unmounted fields are automatically unregistered, OR- Make the
requiredvalidation conditional:required: watchedType !== "custom"The submission handler already correctly skips the
requirementvalue for custom quotas (if (type !== "custom")), but this never gets called due to the validation blocking.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/workspace/[id]/quotas.tsx (1)
878-879:⚠️ Potential issue | 🟡 MinorCustom quotas display incorrectly in All Quotas view.
For custom quotas,
quota.valueisnullandtypes["custom"]is"custom", resulting in a display like"null custom per timeframe". The My Quotas view (lines 733-737) handles this conditionally, but the All Quotas management view does not.Suggested fix
- <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400"> - {quota.value} {types[quota.type]} per timeframe - </p> + {quota.type !== "custom" ? ( + <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400"> + {quota.value} {types[quota.type]} per timeframe + </p> + ) : ( + <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400 italic"> + Manually tracked + </p> + )}
🧹 Nitpick comments (2)
pages/workspace/[id]/quotas.tsx (2)
560-560: Display label "custom" is not user-friendly.The
typesmap provides user-facing labels (e.g., "Minutes in game", "Sessions hosted"). Using"custom"as the label appears inconsistent. This affects line 879 in the All Quotas view where it renders{quota.value} {types[quota.type]} per timeframe, which for custom quotas would display something like"null custom per timeframe".Consider a more descriptive label or handling custom quotas separately in the display logic.
Suggested fix
- custom: "custom", + custom: "Custom",And update line 879 to handle custom quotas:
{quota.type !== "custom" ? ( <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400"> {quota.value} {types[quota.type]} per timeframe </p> ) : ( <p className="text-xs text-zinc-500 mt-1 dark:text-zinc-400 italic"> Manually tracked </p> )}
617-621: Redundant condition check.The
type !== "custom"check on line 617 is redundant since"custom"is not included in the array["sessions_hosted", "sessions_attended", "sessions_logged"]. Theincludes()check alone would suffice.Simplification
- if ( type !== "custom" && - ["sessions_hosted", "sessions_attended", "sessions_logged"].includes(type) - ) { + if (["sessions_hosted", "sessions_attended", "sessions_logged"].includes(type)) {
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/workspace/[id]/quotas.tsx (1)
388-435:⚠️ Potential issue | 🟡 MinorGuard percentage calculations against null/zero quota values.
quota.valueis now nullable, so a missing/0 value on non-custom quotas will yieldNaN/Infinityand break the progress UI. Consider a safe denominator to keeppercentagestable.🛡️ Suggested guard for percentage calculations
+ const safePercent = (current: number) => + typeof quota.value === "number" && quota.value > 0 + ? (current / quota.value) * 100 + : 0; switch (quota.type) { case "mins": currentValue = activeMinutes; - percentage = (activeMinutes / quota.value) * 100; + percentage = safePercent(activeMinutes); break; case "sessions_hosted": const hostedCount = quota.sessionType && quota.sessionType !== "all" ? hostedSessionsByType[quota.sessionType] || 0 : sessionsHosted; currentValue = hostedCount; - percentage = (hostedCount / quota.value) * 100; + percentage = safePercent(hostedCount); break; case "sessions_attended": const attendedCount = quota.sessionType && quota.sessionType !== "all" ? attendedSessionsByType[quota.sessionType] || 0 : sessionsAttended; currentValue = attendedCount; - percentage = (attendedCount / quota.value) * 100; + percentage = safePercent(attendedCount); break; case "sessions_logged": const loggedCount = quota.sessionType && quota.sessionType !== "all" ? loggedSessionsByType[quota.sessionType] || 0 : totalSessionsLogged; currentValue = loggedCount; - percentage = (loggedCount / quota.value) * 100; + percentage = safePercent(loggedCount); break; case "alliance_visits": currentValue = allianceVisits; - percentage = (allianceVisits / quota.value) * 100; + percentage = safePercent(allianceVisits); break; }
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.