Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRemoved the legacy "Offers & Items" client page, added a new "Offers" client page with full offer-management UI, updated payments routing and sidebar to the new path, replaced a local EditableInput with a shared component, added a compact mode to CodeBlock, and added a Tailwind mono font alias. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router
participant PaymentsIndex as /payments (index)
participant OffersPage
participant Config as ProjectConfig
participant Toasts
User->>Router: Navigate to /projects/:id/payments
Router->>PaymentsIndex: resolve
PaymentsIndex-->>Router: redirect -> /payments/offers
Router->>OffersPage: mount /payments/offers
OffersPage->>Config: load offers & items
Config-->>OffersPage: return config
OffersPage-->>User: render catalog & tabs
rect rgba(200,230,255,0.18)
note right of OffersPage: Create / Edit / Save flow
User->>OffersPage: create/edit offer
OffersPage->>Config: updateConfig(offer changes)
Config-->>OffersPage: persisted
OffersPage->>Toasts: show success/failure
OffersPage-->>User: refreshed view
end
sequenceDiagram
autonumber
actor User
participant OffersPage
participant OfferDialog
participant ItemDialog
User->>OffersPage: Open "New Offer" or "Edit"
OffersPage->>OfferDialog: open
OfferDialog-->>OffersPage: submit / cancel
User->>OfferDialog: add included item
OfferDialog->>ItemDialog: open item selector / create
ItemDialog-->>OfferDialog: confirm / cancel
OfferDialog-->>OffersPage: update offer draft
OffersPage->>OfferDialog: save -> trigger updateConfig
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements a comprehensive offer catalog UI for the payments system, representing a significant refactoring of the payment management interface. The changes consolidate the previously separate 'offers-and-items' functionality into a unified 'offers' section, streamlining the navigation structure and user experience.
Key changes include:
Navigation Restructuring: The sidebar navigation has been updated to replace 'Offers & Items' with just 'Offers', changing the route from /payments/offers-and-items to /payments/offers. The main payments page redirect has also been updated accordingly.
Component Architecture: Multiple components have been moved from the offers-and-items directory to the new offers directory, including layout components, dialog components, and utility components. This reorganization creates a more focused and maintainable code structure.
Enhanced UI Components: The CodeBlock component now supports a compact mode through an optional compact prop, allowing for more flexible display options with reduced sizing for icons, padding, and font sizes when space is constrained.
Comprehensive Offer Management: A new page-client.tsx file (1597 lines) implements a sophisticated offer catalog interface with features including:
- Inline editing capabilities for offers and prices
- Visual offer cards with drag-and-drop style interactions
- Price interval selection with popover controls
- Item inclusion management with quantity and repeat settings
- Customer type filtering (user/team/custom)
- Offer grouping and catalog organization
Dialog Components: Several dialog components have been refined for creating and editing offers, items, prices, and included items. These components include comprehensive validation, error handling, and multi-step wizard interfaces for complex configurations.
State Management: The new implementation uses extensive React state management with multiple useState hooks and complex useMemo calculations for handling the interactive offer catalog interface.
This refactoring consolidates payment management into a more cohesive and user-friendly interface, moving from separate offer and item management to an integrated catalog approach that better reflects how these components work together in a payment system.
Confidence score: 3/5
- This PR introduces significant complexity with a large new component and extensive state management that could lead to performance or maintenance issues
- Score reflects concerns about the 1597-line component file, potential prototype pollution vulnerability, and the complexity of the nested state management
- Pay close attention to the new
page-client.tsxfile for potential performance issues and the object property access pattern that could be vulnerable
Context used:
Rule - Use Map<A, B> instead of plain objects when using dynamic keys to avoid prototype pollution vulnerabilities. (link)
15 files reviewed, 8 comments
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx (1)
249-251: Broaden regex to match nested Offers subroutes.Current regex highlights the nav only on the exact path. Use a prefix pattern so pages under /payments/offers/* remain selected.
- regex: /^\/projects\/[^\/]+\/payments\/offers$/, + regex: /^\/projects\/[^\/]+\/payments\/offers(?:\/.*)?$/,apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (3)
795-809: Use the shared ID schema for validation instead of a hand-rolled regex.You already import userSpecifiedIdSchema; use it here to keep rules consistent.
- const trimmed = localOfferId.trim(); - const validId = trimmed && /^[a-z0-9-]+$/.test(trimmed) ? trimmed : id; + const trimmed = localOfferId.trim(); + const parsed = userSpecifiedIdSchema.safeParse(trimmed); + const validId = parsed.success ? parsed.data : id;
545-558: Avoid nested interactive elements inside menu items (a11y).Button inside DropdownMenuItem creates nested click targets. Move the click handler to DropdownMenuItem and render text-only content.
- <DropdownMenuItem key={option.value}> - <Button - key={option.value} - variant="ghost" - size="sm" - className="flex flex-col items-start" - onClick={() => { - onSave(itemId, { ...item, expires: option.value }); - }}> - {option.label} - <span className="text-xs text-muted-foreground">{option.description}</span> - </Button> - </DropdownMenuItem> + <DropdownMenuItem + key={option.value} + onClick={() => onSave(itemId, { ...item, expires: option.value })} + className="flex flex-col items-start" + > + {option.label} + <span className="text-xs text-muted-foreground">{option.description}</span> + </DropdownMenuItem>
1055-1063: Consider Map over Record for groups to match repo guidelines.Groups are read as a key–value collection; using Map improves semantics and avoids accidental prototype keys. Non-blocking since config storage likely uses objects.
Example refactor outline:
- Change CatalogViewProps.groups to Map<string, { displayName?: string }>
- Access via groups.get(groupId)?.displayName at usage sites.
Also applies to: 1146-1147
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers-and-items/page-client.tsx(0 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/page.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/workflows/workflow-list.tsx(1 hunks)apps/dashboard/src/components/code-block.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers-and-items/page-client.tsx
🧰 Additional context used
📓 Path-based instructions (3)
{apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
For blocking alerts and errors in UI, do not use toast notifications; use alerts instead
Files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/workflows/workflow-list.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsxapps/dashboard/src/components/code-block.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/workflows/workflow-list.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsxapps/dashboard/src/components/code-block.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/page.tsx
{apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Keep hover/click animations snappy; avoid pre-transition delays on hover and apply transitions after the action (e.g., fade-out on hover end)
Files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/workflows/workflow-list.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsxapps/dashboard/src/components/code-block.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/page.tsx
🧬 Code graph analysis (2)
apps/dashboard/src/components/code-block.tsx (2)
apps/dashboard/src/lib/theme.tsx (1)
useThemeWatcher(10-45)docs/src/components/icons.tsx (1)
Code(340-343)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (12)
packages/stack-shared/src/config/schema.ts (1)
CompleteConfig(993-993)packages/stack-shared/src/utils/dates.tsx (1)
DayInterval(146-146)apps/dashboard/src/lib/utils.tsx (1)
cn(7-9)packages/stack-ui/src/components/ui/use-toast.tsx (1)
toast(195-195)packages/stack-shared/src/utils/numbers.tsx (1)
prettyPrintWithMagnitudes(9-26)apps/dashboard/src/components/code-block.tsx (1)
CodeBlock(18-75)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
useAdminApp(27-34)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/dummy-data.tsx (1)
DUMMY_PAYMENTS_CONFIG(2-278)packages/stack-shared/src/utils/objects.tsx (1)
typedEntries(263-265)packages/stack-shared/src/utils/strings.tsx (1)
stringCompare(61-65)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/offer-dialog.tsx (1)
OfferDialog(48-820)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/item-dialog.tsx (1)
ItemDialog(19-178)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: all-good
- GitHub Check: setup-tests
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (2)
apps/dashboard/src/components/code-block.tsx (1)
25-26: Compact mode addition looks solid.Clean use of cn for size/spacing; no behavior change for existing callers.
Also applies to: 32-37, 43-47, 54-67
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/workflows/workflow-list.tsx (1)
6-6: Import path verified — ListSection export present.
ListSection is exported from apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/list-section.tsx (export function ListSection); no changes required.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (2)
1055-1063: Prefer Map over Record for dynamic keyed collections.
groupsis a dynamic key–value set; usingMap<string, { displayName?: string }>avoids prototype pitfalls and clarifies iteration semantics. Not blocking given config shape, but recommended for in-memory views.
468-471: Replace blocking toasts with inline errors or Alert UI (per guidelines).These toasts gate user actions. Use Alert/ActionDialog or inline helper text; toasts are fine for non-blocking success/info only. The “Price ID already exists” branch is also unreachable here.
- if (isUsed) { - toast({ title: 'Item already included' }); - return; - } + if (isUsed) return; // Already disabled; consider showing a small "Already included" note next to the option- if (Object.prototype.hasOwnProperty.call(nextPrices, newId)) { - toast({ title: "Price ID already exists" }); - return prev; // Do not change state - } + if (Object.prototype.hasOwnProperty.call(nextPrices, newId)) { + return prev; // Keep state; surface inline error near the ID field if/when ID editing is added + }- if (Object.prototype.hasOwnProperty.call(prev.includedItems, newItemId)) { - toast({ title: "Item already included" }); - return prev; - } + if (Object.prototype.hasOwnProperty.call(prev.includedItems, newItemId)) { + return prev; // Consider surfacing inline helper text near the selector trigger + }Also applies to: 979-983, 718-721
🧹 Nitpick comments (4)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (4)
641-642: Remove dead state/props:editingPriceIdandexistingPriceIds.
editingPriceIdis never used to influence rendering, andexistingPriceIdsisn’t read insideOfferPriceRow. This adds noise and maintenance overhead.- const [editingPriceId, setEditingPriceId] = useState<string | undefined>(undefined); @@ - if (editingPriceId === priceId) setEditingPriceId(undefined); @@ - if (editingPriceId && finalId === editingPriceId) { - setEditingPriceId(undefined); - } @@ - setEditingPriceId(tempId);- existingPriceIds: string[],And remove the
existingPriceIdsargument at call sites and from theOfferPriceRowprops type.Also applies to: 670-671, 727-729, 949-950, 258-261
1527-1532: UseonCheckedChangefor Checkbox; avoid toggling viaonClick.Improves a11y and prevents drift with controlled state.
- <Checkbox - checked={shouldUseDummyData} - onClick={() => setShouldUseDummyData(s => !s)} - id="use-dummy-data" - /> + <Checkbox + checked={shouldUseDummyData} + onCheckedChange={(checked) => setShouldUseDummyData(Boolean(checked))} + id="use-dummy-data" + />
792-793: Nit: small UX polish for ID display.For non-draft editing, render the current
localOfferIdin a monospace, read-only Input to keep the UI consistent when toggling edit mode.
1484-1520: Error handling onupdateConfigcalls.Wrap with try/catch and surface failure via Alert; keep toasts for success only.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
For blocking alerts and errors in UI, do not use toast notifications; use alerts instead
Files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
{apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Keep hover/click animations snappy; avoid pre-transition delays on hover and apply transitions after the action (e.g., fade-out on hover end)
Files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-31T06:43:58.231Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: AGENTS.md:0-0
Timestamp: 2025-08-31T06:43:58.231Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Prefer ES6 Map over Record when representing key–value collections
Applied to files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
📚 Learning: 2025-08-31T06:43:58.231Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: AGENTS.md:0-0
Timestamp: 2025-08-31T06:43:58.231Z
Learning: Applies to {apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx} : For blocking alerts and errors in UI, do not use toast notifications; use alerts instead
Applied to files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
🧬 Code graph analysis (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (11)
packages/stack-shared/src/config/schema.ts (1)
CompleteConfig(993-993)packages/stack-shared/src/utils/dates.tsx (1)
DayInterval(146-146)apps/dashboard/src/lib/utils.tsx (1)
cn(7-9)packages/stack-shared/src/utils/numbers.tsx (1)
prettyPrintWithMagnitudes(9-26)apps/dashboard/src/components/code-block.tsx (1)
CodeBlock(18-75)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
useAdminApp(27-34)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/dummy-data.tsx (1)
DUMMY_PAYMENTS_CONFIG(2-278)packages/stack-shared/src/utils/objects.tsx (1)
typedEntries(263-265)packages/stack-shared/src/utils/strings.tsx (1)
stringCompare(61-65)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/offer-dialog.tsx (1)
OfferDialog(48-820)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/item-dialog.tsx (1)
ItemDialog(19-178)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: restart-dev-and-test
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: Security Check
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (7)
1130-1131: Guard against missing catalog metadata (optional chaining).Avoid a runtime crash when a groupId has no metadata.
- const groupName = !isNewGroupPlaceholder ? (groupId ? ((groups[groupId].displayName || groupId)) : 'No catalog') : ''; + const groupName = !isNewGroupPlaceholder ? (groupId ? ((groups[groupId]?.displayName || groupId)) : 'No catalog') : '';
452-474: Replace blocking toast with non-blocking UX (button is already disabled).Remove the toast on duplicate item selection; the button is disabled (
disabled={isUsed}), so showing a toast violates the “no toasts for blocking errors” rule and adds noise.- if (isUsed) { - toast({ title: 'Item already included' }); - return; - } + if (isUsed) return;
711-731: Do not use toasts for blocking errors; surface inline error for duplicate Price ID.Using a toast to block an edit breaks the UI guideline. Capture the error in local state and render a small inline message near the prices section.
Add error state:
const [localOfferId, setLocalOfferId] = useState<string>(id); + const [priceError, setPriceError] = useState<string | null>(null);Replace the toast in the duplicate check and clear on success:
- if (newId && newId !== pid) { - if (Object.prototype.hasOwnProperty.call(nextPrices, newId)) { - toast({ title: "Price ID already exists" }); - return prev; // Do not change state - } - delete nextPrices[pid]; - } - nextPrices[finalId] = newPrice; + if (newId && newId !== pid) { + if (Object.prototype.hasOwnProperty.call(nextPrices, newId)) { + setPriceError("Price ID already exists"); + return prev; // Do not change state + } + delete nextPrices[pid]; + } + nextPrices[finalId] = newPrice; return { ...prev, prices: nextPrices }; }); + setPriceError(null);Render the inline error:
<div className="px-4 py-4 border-y border-border"> {renderPrimaryPrices()} + {priceError && <div className="text-xs text-destructive mt-2">{priceError}</div>} {isEditing && draft.prices !== 'include-by-default' && (Also applies to: 637-645, 916-939
960-967: Remove blocking toast in onChangeItemId; rely on validation/disabled state.Block with inline validation instead of a toast. At minimum, just drop the toast.
- if (Object.prototype.hasOwnProperty.call(prev.includedItems, newItemId)) { - toast({ title: "Item already included" }); - return prev; - } + if (Object.prototype.hasOwnProperty.call(prev.includedItems, newItemId)) { + return prev; + }
1194-1212: Replace window.alert with inline validation (or ActionDialog) for new catalog errors.Window alerts violate the UX guideline. Store an error string and render it near the input.
Add state:
const [creatingGroupKey, setCreatingGroupKey] = useState<string | undefined>(undefined); const [newGroupId, setNewGroupId] = useState(""); + const [newGroupError, setNewGroupError] = useState<string | null>(null); const newGroupInputRef = useRef<HTMLInputElement | null>(null);Inline error + clear on change/cancel:
<Input ref={newGroupInputRef} value={newGroupId} - onChange={(e) => setNewGroupId(e.target.value)} + onChange={(e) => { setNewGroupId(e.target.value); setNewGroupError(null); }} placeholder="catalog-id" className="w-56" /> + {newGroupError && <div className="text-xs text-destructive ml-1">{newGroupError}</div>}onClick={() => { setCreatingGroupKey(undefined); setNewGroupId(""); + setNewGroupError(null); setDrafts(prev => prev.filter(d => d.groupId !== groupId)); }}Swap alerts for inline errors:
- if (!id) { - alert("Catalog ID is required"); + if (!id) { + setNewGroupError("Catalog ID is required"); return; } if (!/^[a-z0-9-]+$/.test(id)) { - alert("Catalog ID must be lowercase letters, numbers, and hyphens"); + setNewGroupError("Catalog ID must be lowercase letters, numbers, and hyphens"); return; } if (Object.prototype.hasOwnProperty.call(groups, id)) { - alert("Catalog ID already exists"); + setNewGroupError("Catalog ID already exists"); return; } const offerWithGroup: Offer = { ...offer, groupId: id }; await onSaveOfferWithGroup(id, newId, offerWithGroup); setCreatingGroupKey(undefined); setNewGroupId(""); + setNewGroupError(null); setDrafts(prev => prev.filter(x => x.key !== d.key));Also applies to: 1050-1055, 1134-1141, 1143-1149
622-635: Prevent overwriting another offer when renaming Offer ID. Add uniqueness guard.Renaming to an existing ID will overwrite that offer. Validate uniqueness and block Save with a clear reason.
Add prop:
type OfferCardProps = { id: string, activeType: 'user' | 'team' | 'custom', offer: Offer, allOffers: Array<{ id: string, offer: Offer }>, + existingOfferIds: string[], existingItems: Array<{ id: string, displayName: string, customerType: string }>,Use it in component and validation:
-function OfferCard({ id, activeType, offer, allOffers, existingItems, onSave, onDelete, onDuplicate, onCreateNewItem, onOpenDetails, isDraft, onCancelDraft }: OfferCardProps) { +function OfferCard({ id, activeType, offer, allOffers, existingOfferIds, existingItems, onSave, onDelete, onDuplicate, onCreateNewItem, onOpenDetails, isDraft, onCancelDraft }: OfferCardProps) {- const canSaveOffer = draft.prices === 'include-by-default' || (typeof draft.prices === 'object' && Object.keys(pricesObject).length > 0); - const saveDisabledReason = canSaveOffer ? undefined : "Add at least one price or set Include by default"; + const trimmedId = (localOfferId ?? "").trim(); + const idValid = /^[a-z0-9-]+$/.test(trimmedId); + const idDup = trimmedId && trimmedId !== id && existingOfferIds.includes(trimmedId); + const hasPrices = draft.prices === 'include-by-default' || (typeof draft.prices === 'object' && Object.keys(pricesObject).length > 0); + const canSaveOffer = hasPrices && idValid && !idDup; + const saveDisabledReason = + !hasPrices ? "Add at least one price or set Include by default" + : !idValid ? "ID must be lowercase letters, numbers, and hyphens" + : idDup ? "Offer ID already exists" + : undefined;Block Save when invalid and use nextId safely:
- <Button size="icon" variant="ghost" onClick={async () => { - const trimmed = localOfferId.trim(); - const validId = trimmed && /^[a-z0-9-]+$/.test(trimmed) ? trimmed : id; - if (validId !== id) { - await onSave(validId, draft); - await onDelete(id); - } else { - await onSave(id, draft); - } - setIsEditing(false); - setEditingPriceId(undefined); - }} disabled={!canSaveOffer}> + <Button + size="icon" + variant="ghost" + onClick={async () => { + if (!canSaveOffer) return; + const nextId = trimmedId || id; + if (nextId !== id) { + await onSave(nextId, draft); + await onDelete(id); + } else { + await onSave(id, draft); + } + setIsEditing(false); + setEditingPriceId(undefined); + }} + disabled={!canSaveOffer} + >Pass the set of existing IDs:
offer={offer} allOffers={offers} + existingOfferIds={Array.from(usedIds)} existingItems={existingItems}offer={d.offer} allOffers={offers} + existingOfferIds={Array.from(usedIds)} existingItems={existingItems}Also applies to: 637-645, 658-662, 809-825, 1160-1181, 1182-1237
1470-1506: Success toasts OK — replace blocking window.alert(...) with in‑UI Alert/inline validation.Toasts for offer/item success are non-blocking and fine; this file contains blocking alert() calls that must be replaced with the project Alert component or inline validation UI: apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx — lines 1196, 1200, 1204.
🧹 Nitpick comments (7)
apps/dashboard/src/components/editable-input.tsx (5)
50-64: Replace blocking confirm() on blur; keep edits open or use an in-dialog alert.Avoid native confirm() (blocks thread, poor a11y). Either keep editing state on blur and let users explicitly Save/Discard, or show an inline blocking dialog/alert inside the component.
Apply this diff to keep edits open on blur (no confirm); users decide via buttons:
- onBlur={(ev) => { - if (!forceAllowBlur.current) { - if (!hasChanged) { - setEditValue(null); - } else { - // TODO this should probably be a blocking dialog instead, and it should have a "cancel" button that focuses the input again - if (confirm("You have unapplied changes. Would you like to save them?")) { - acceptRef.current?.click(); - } else { - setEditValue(null); - setHasChanged(false); - } - } - } - }} + onBlur={() => { + if (forceAllowBlur.current) return; + if (!hasChanged) { + setEditValue(null); + } + // Keep edit controls visible when there are changes; user can Save or Discard explicitly. + }}
65-69: Avoid broad preventDefault on mousedown; scope via relatedTarget instead.Global mousedown preventDefault can interfere with focus, selection, and context menus. Prefer handling intra-component focus transitions via relatedTarget in onBlur instead of suppressing pointer events.
Apply this diff to remove onMouseDown and rely on improved onBlur logic:
- onMouseDown={(ev) => { - // prevent blur from happening - ev.preventDefault(); - return false; - }}
112-142: Icon-only buttons need accessible labels.Add aria-label/title so screen readers announce actions; keep focus styles.
- <Button + <Button + aria-label={action === "accept" ? "Save changes" : "Discard changes"} + title={action === "accept" ? "Save changes" : "Discard changes"} ref={action === "accept" ? acceptRef : undefined} key={action} disabled={isLoading} type="button" variant="plain" size="plain"
95-99: Support Escape to discard edits.Improves keyboard UX.
onKeyDown={(e) => { if (e.key === 'Enter') { acceptRef.current?.click(); + } else if (e.key === 'Escape') { + setEditValue(null); + setHasChanged(false); } }}
39-42: Surface save errors inline; don’t fail silently.Capture the error from useAsyncCallback and show an inline alert near the field. Also mark the input invalid when an error exists.
- const [handleUpdate, isLoading] = useAsyncCallback(async (value: string) => { + const [handleUpdate, isLoading, updateError] = useAsyncCallback(async (value: string) => { await onUpdate?.(value); }, [onUpdate]);- className={cn( + className={cn( "w-full px-1 py-0 h-[unset] border-transparent", /* Hover */ !readOnly && "hover:ring-1 hover:ring-slate-300 dark:hover:ring-gray-500 hover:bg-slate-50 dark:hover:bg-gray-800 hover:cursor-pointer", /* Focus */ !readOnly && "focus:cursor-[unset] focus-visible:ring-slate-500 dark:focus-visible:ring-gray-50 focus-visible:bg-slate-100 dark:focus-visible:bg-gray-800", readOnly && "focus-visible:ring-0 cursor-default", shiftTextToLeft && "ml-[-7px]", inputClassName, )} + aria-invalid={!!updateError}</div> + {updateError && ( + <div role="alert" className="mt-1 text-sm text-destructive"> + {updateError instanceof Error ? updateError.message : "Failed to save. Please try again."} + </div> + )} ;Also applies to: 86-86, 143-144
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (2)
1427-1438: Prefer Map over Record for aggregations (aligns with repo guideline).Use Map for
typeCountsto avoid prototype pitfalls and to follow the Map-over-Record rule.- const typeCounts = offers.reduce((acc, { offer }) => { - const type = offer.customerType; - acc[type] = (acc[type] || 0) + 1; - return acc; - }, {} as Record<string, number>); + const typeCounts = offers.reduce((acc, { offer }) => { + const type = offer.customerType; + acc.set(type, (acc.get(type) || 0) + 1); + return acc; + }, new Map<string, number>());- const predominantType = Object.entries(typeCounts) - .sort(([, a], [, b]) => b - a)[0]?.[0]; + const predominantType = Array.from(typeCounts.entries()) + .sort(([, a], [, b]) => b - a)[0]?.[0];Also applies to: 1434-1436
45-48: Tighten the Price type alias.Index into the already-excluded object variant to avoid the odd intersection trick.
-type Price = (Offer['prices'] & object)[string]; -type PricesObject = Exclude<Offer['prices'], 'include-by-default'>; +type PricesObject = Exclude<Offer['prices'], 'include-by-default'>; +type Price = PricesObject[string];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx(3 hunks)apps/dashboard/src/components/editable-input.tsx(1 hunks)apps/dashboard/tailwind.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
For blocking alerts and errors in UI, do not use toast notifications; use alerts instead
Files:
apps/dashboard/src/components/editable-input.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/dashboard/src/components/editable-input.tsxapps/dashboard/tailwind.config.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
{apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Keep hover/click animations snappy; avoid pre-transition delays on hover and apply transitions after the action (e.g., fade-out on hover end)
Files:
apps/dashboard/src/components/editable-input.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-31T06:43:58.231Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: AGENTS.md:0-0
Timestamp: 2025-08-31T06:43:58.231Z
Learning: Applies to {apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx} : For blocking alerts and errors in UI, do not use toast notifications; use alerts instead
Applied to files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
📚 Learning: 2025-08-31T06:43:58.231Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: AGENTS.md:0-0
Timestamp: 2025-08-31T06:43:58.231Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Prefer ES6 Map over Record when representing key–value collections
Applied to files:
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
🧬 Code graph analysis (2)
apps/dashboard/src/components/editable-input.tsx (3)
packages/stack-shared/src/hooks/use-async-callback.tsx (1)
useAsyncCallback(4-27)packages/stack-ui/src/components/ui/input.tsx (1)
Input(10-41)packages/stack-shared/src/utils/errors.tsx (1)
throwErr(10-19)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (12)
packages/stack-shared/src/config/schema.ts (1)
CompleteConfig(993-993)packages/stack-shared/src/utils/dates.tsx (1)
DayInterval(146-146)apps/dashboard/src/lib/utils.tsx (1)
cn(7-9)packages/stack-shared/src/utils/numbers.tsx (1)
prettyPrintWithMagnitudes(9-26)apps/dashboard/src/components/code-block.tsx (1)
CodeBlock(18-75)apps/dashboard/src/components/editable-input.tsx (1)
EditableInput(20-145)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
useAdminApp(27-34)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/dummy-data.tsx (1)
DUMMY_PAYMENTS_CONFIG(2-278)packages/stack-shared/src/utils/objects.tsx (1)
typedEntries(263-265)packages/stack-shared/src/utils/strings.tsx (1)
stringCompare(61-65)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/offer-dialog.tsx (1)
OfferDialog(48-820)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/item-dialog.tsx (1)
ItemDialog(19-178)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Cursor Bugbot
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: all-good
- GitHub Check: lint_and_build (latest)
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: docker
🔇 Additional comments (4)
apps/dashboard/tailwind.config.ts (1)
11-13: Move mono font to theme.extend and add fallbacksDefining fontFamily at theme scope overrides Tailwind defaults; move this to theme.extend and add a fallback chain so UI doesn't regress if the CSS variable is missing.
Location: apps/dashboard/tailwind.config.ts (lines 11–13)
Apply this diff to remove the override:
- fontFamily: - mono: ["var(--font-geist-mono)"], - },Then add the extension and fallback (outside the selected lines):
import defaultTheme from "tailwindcss/defaultTheme" // ... theme: { // ... extend: { fontFamily: { mono: ["var(--font-geist-mono)", ...defaultTheme.fontFamily.mono], }, // existing extend keys... }, },I couldn't confirm from your rg output whether --font-geist-mono is defined or whether font-(mono|sans|serif) classes are used (rg reported "unrecognized file type: ts,tsx"). Re-run these checks and paste the output:
rg -n -S --glob '!**/node_modules/**' --iglob '*.{ts,tsx,js,jsx,css,scss,pcss,html}' '\-\-font-geist-mono|var\(--font-geist-mono\)' -C2 rg -n -S --glob '!**/node_modules/**' --iglob '*.{ts,tsx,js,jsx,html}' '\bfont-(mono|sans|serif)\b' -C2apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx (2)
3-3: Good move: use shared EditableInput.Consolidating the inline editor reduces duplication and keeps UX consistent.
14-14: Import of StackAssertionError is appropriate — LGTM; verify no stale EditableInput usagesImport of StackAssertionError is appropriate and used to guard provider config lookup. Sandbox ripgrep skipped files ("No files were searched"), so I couldn't confirm removal of "@/components/editable-input" or any EditableInput uses. Run locally and paste results:
rg -n --hidden -S --glob '!/node_modules/' --glob '!/dist/' --glob '!/build/' -e '@/components/editable-input' -e 'EditableInput' || trueapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (1)
499-507: Clarify whether includedItems.quantity may be zeroCurrent code is inconsistent: page-client.tsx normalizes/sets quantity → 0 (lines ~409, 432, 499–507), components/incoming schema allow min 0 (apps/.../included-item-editor.tsx ~line 50), but included-item-dialog.tsx validates >=1 (lines ~83–86). Backend purchase endpoints require purchase quantity >=1 (different concept). Decide the domain rule and align.
- If zero is invalid: enforce >=1 everywhere — change yup.min(0) → min(1) in apps/dashboard/src/components/payments/included-item-editor.tsx, clamp/normalize input and block Save in apps/.../offers/page-client.tsx (normalize at ~line 432, input handling at ~499–507), ensure newItem defaults stay 1.
- If zero is allowed: make included-item-dialog.tsx accept 0 (update its validation at ~83–86) and keep schemas/normalization consistent.
| const applyCustom = (count: number, unit: DayInterval[1]) => { | ||
| setIntervalSelection('custom'); | ||
| setUnit(unit); | ||
| setCount(count); | ||
| if (!readOnly) onChange([count, unit]); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Disallow zero/invalid custom intervals (persisted config would be invalid).
Ensure interval count is >= 1 in IntervalPopover.
- const applyCustom = (count: number, unit: DayInterval[1]) => {
- setIntervalSelection('custom');
- setUnit(unit);
- setCount(count);
- if (!readOnly) onChange([count, unit]);
- };
+ const applyCustom = (count: number, unit: DayInterval[1]) => {
+ const safeCount = Math.max(1, (count | 0));
+ setIntervalSelection('custom');
+ setUnit(unit);
+ setCount(safeCount);
+ if (!readOnly) onChange([safeCount, unit]);
+ };- const n = v === '' ? 0 : parseInt(v, 10);
- applyCustom(n, (unit || 'month') as DayInterval[1]);
+ const n = v === '' ? 1 : Math.max(1, parseInt(v, 10) || 1);
+ applyCustom(n, (unit || 'month') as DayInterval[1]);Also applies to: 204-214
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
around lines 125-131 (and similarly at lines 204-214), the applyCustom handler
currently allows zero/invalid interval counts; update it to validate that count
>= 1 before updating state or calling onChange. Specifically, if count < 1,
either clamp to 1 or return early (do not setIntervalSelection, setUnit,
setCount, or call onChange); if you choose to clamp, set count = Math.max(1,
count) and proceed. Ensure readOnly is still respected when calling onChange.
| import { AtSign, Calendar, Check, Hash, Mail, MoreHorizontal, Shield, SquareAsterisk, X } from "lucide-react"; | ||
| import { useEffect, useMemo, useRef, useState } from "react"; | ||
| import { useEffect, useMemo, useState } from "react"; | ||
| import * as yup from "yup"; |
There was a problem hiding this comment.
🧩 Analysis chain
Avoid toasts for blocking errors; show alerts in-dialog instead.
Per guidelines for apps/dashboard/*.tsx: for blocking alerts/errors (e.g., SmartFormDialog onSubmit returning 'prevent-close'), replace error toasts with an inline alert element within the dialog or form body.
Locate toast usage tied to blocking flows and replace with inline alerts:
If needed, I can draft a minimal inline alert pattern for SmartFormDialog (stateful error + alert block above fields).
🏁 Script executed:
#!/bin/bash
rg -nP --type=tsx -C3 '(prevent-close|useToast|toast\\()' apps/dashboard/src/app/(main)/(protected)/projects/\[projectId\]/users/\[userId\]/page-client.tsxLength of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "cwd: $(pwd)"
# Literal path from the review (contains parentheses/brackets) — check existence safely
target='apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx'
if [ -f "$target" ]; then
echo "Literal file found: $target"
files=("$target")
else
echo "Literal file not found; searching for page-client.tsx under apps/dashboard..."
mapfile -t files < <(find apps/dashboard -type f -name 'page-client.tsx' 2>/dev/null || true)
if [ ${#files[@]} -eq 0 ]; then
echo "No page-client.tsx files found under apps/dashboard."
else
printf 'Found: %s\n' "${files[@]}"
fi
fi
# If any files found, print head + search patterns in them; otherwise search the whole apps/dashboard for patterns
if [ ${#files[@]} -gt 0 ]; then
for f in "${files[@]}"; do
echo "---- Showing first 200 lines of: $f ----"
sed -n '1,200p' "$f" || true
echo "---- Searching for blocking-toast patterns in: $f ----"
rg -nP --type=tsx -C3 '(prevent-close|useToast|toast\()' "$f" || true
done
else
echo "---- Searching apps/dashboard for blocking-toast patterns (prevent-close, useToast, toast()) ----"
rg -nP --type=tsx -C3 '(prevent-close|useToast|toast\()' apps/dashboard || true
fi
echo "---- Searching for imports of yup in apps/dashboard ----"
rg -n --type=tsx -C3 'import .*yup' apps/dashboard || trueLength of output: 6676
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f='apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx'
if [ ! -f "$f" ]; then
echo "file not found: $f"
exit 0
fi
echo "File exists: $f"
echo "---- Searching for patterns in the file ----"
patterns=('useToast\(' 'toast\(' 'prevent-close' 'SmartFormDialog' 'FormDialog' 'onSubmit=')
for p in "${patterns[@]}"; do
echo
echo "=== PATTERN: $p ==="
grep -n -E -- "$p" "$f" || true
echo "=== CONTEXT (±3 lines) ==="
grep -n -C3 -E -- "$p" "$f" || true
done
echo
echo "---- Searching apps/dashboard for related patterns ----"
grep -R -n -E 'useToast\(|toast\(|prevent-close|SmartFormDialog|FormDialog' apps/dashboard || trueLength of output: 26267
Replace blocking error toasts with inline dialog alerts.
In apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx the onSubmit handlers call toast(...) for errors (see toast calls around lines 919–926 and 961–968) and the flow returns 'prevent-close' (line 932) — these are blocking flows and must show an inline alert inside the dialog instead of a toast. Render a stateful alert in the SmartFormDialog (SmartFormDialog/onSubmit ~lines 940–946): e.g. const [formError, setFormError] = useState(null); on error call setFormError({title, description}); render {formError && {formError.description}} above the fields; clear the error on input change or dialog close. Keep non-blocking success toasts.
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
2 participants
Review by RecurseML
🔍 Review performed on 2afaaea..f25544e
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (5)
•
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx•
apps/dashboard/src/components/code-block.tsx•
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsx•
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/page.tsx•
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/workflows/workflow-list.tsx⏭️ Files skipped (trigger manually) (10)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers-and-items/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/create-group-dialog.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/dummy-data.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/included-item-dialog.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/item-dialog.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/layout.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/list-section.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/offer-dialog.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/price-dialog.tsxSummary by CodeRabbit
New Features
Chores
Removed