Conversation
📝 WalkthroughWalkthroughAdds a cellar inventory feature: database models, domain APIs, server actions, UI components, forms/schemas, translations, and review integration to track, move, adjust, and review users' cellar items. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant BeerPage as Beer Page
participant Server as Server
participant Domain as Domain Layer
participant DB as Database
User->>BeerPage: Open beer page
BeerPage->>Server: request beer + current user
par
Server->>DB: query beer by slug
Server->>DB: query current user
end
DB-->>Server: beer, user
alt user authenticated
BeerPage->>Server: request storage locations
Server->>Domain: getStorageLocationsByUser(userId)
Domain->>DB: select storage_locations
DB-->>Domain: locations
Domain-->>Server: locations
Server-->>BeerPage: locations
BeerPage-->>User: show AddToCellar button/modal
else
BeerPage-->>User: hide AddToCellar
end
User->>BeerPage: Submit AddToCellar form
BeerPage->>Server: addToCellarAction(formData)
Server->>Domain: addToCellar(data)
Domain->>DB: insert cellar_items (+ maybe storage_locations)
DB-->>Domain: created item
Domain-->>Server: created item
Server->>DB: revalidate cellar page path
Server-->>BeerPage: success
BeerPage-->>User: success toast
sequenceDiagram
participant User as User (Browser)
participant CellarPage as Cellar Page
participant Server as Server
participant Domain as Domain Layer
participant DB as Database
participant ReviewPage as Review Page
User->>CellarPage: View cellar
CellarPage->>Server: fetch items, stats, locations (parallel)
par
Server->>Domain: getCellarItemsByUser(filters)
Server->>Domain: getCellarStats(userId)
Server->>Domain: getStorageLocationsByUser(userId)
end
Domain->>DB: queries
DB-->>Domain: results
Domain-->>Server: transformed results
Server-->>CellarPage: render items & stats
User->>CellarPage: Click "Open & Review" on item
CellarPage->>ReviewPage: navigate with cellarItemId, servingFrom, bestBefore
ReviewPage->>Server: prefill parsing
Server-->>ReviewPage: prefilled form
User->>ReviewPage: Submit review (includes cellarItemId)
ReviewPage->>Server: reviewAction(formData)
Server->>DB: insert review
DB-->>Server: review created
alt cellarItemId present
Server->>Domain: adjustCellarItemQuantity(itemId, -1)
Domain->>DB: update/delete item
DB-->>Domain: updated result
Domain-->>Server: result
Server->>DB: revalidate cellar path
end
Server-->>User: redirect/confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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: 20
🤖 Fix all issues with AI agents
In `@src/app/_components/add-to-cellar-modal/index.tsx`:
- Around line 21-41: ESLint requires the cellar schema import to come before
component imports; move the import of addToCellarSchema so it appears above the
UI/component imports (e.g., above FormCellarServingFormatSelector,
FormDatePicker, FormInput, FormStorageLocationSelector, Button, Dialog, etc.) in
src/app/_components/add-to-cellar-modal/index.tsx, preserving the exact import
specifier (addToCellarSchema) and keeping other imports unchanged.
In `@src/app/_components/form/cellar-serving-format-selector/index.tsx`:
- Around line 7-12: Reorder the import statements so the UI
cellar-serving-selector import appears before FormError to satisfy ESLint;
specifically, move the import of CellarServingFormatSelector and its type
(CellarServingFormat) to precede the import of FormError while keeping Label and
cn imports in their current relative positions.
In `@src/app/_components/form/storage-location-selector/index.tsx`:
- Line 5: The import statement in index.tsx declares useEffect but it is unused;
remove useEffect from the named imports (leave useRef and ComponentProps) in the
top-level import line to satisfy ESLint and avoid dead imports in the
StorageLocationSelector component file.
- Around line 33-60: The hidden input's defaultValue is hardcoded to "" so
existing selections aren't submitted; change the hidden input to use the field
value (e.g. defaultValue={field.value as string ?? ""}) or initialize the
inputRef value from field.value on mount, and keep handleChange updating
inputRef as-is; also remove the unused useEffect import. Update references
around inputRef, handleChange, getSelectProps/field and StorageLocationSelector
to ensure selectedLocationId continues to read from field.value.
In `@src/app/_components/ui/storage-location-selector/index.tsx`:
- Around line 48-50: The single-line conditional in the handleCreateNew function
(checking newLocationName.trim() and onCreateNew) needs explicit braces to
satisfy the curly lint rule; update the if statement inside handleCreateNew to
use a block (wrap the early return in { ... }) so the guard reads as a proper
braced conditional while leaving the rest of the async logic and calls to
onCreateNew unchanged.
In
`@src/app/`[locale]/(business)/(with-header)/breweries/[brewerySlug]/beers/[beerSlug]/page.tsx:
- Line 19: The import order violates ESLint: move the import of getCurrentUser
from "@/lib/auth" so it appears before any imports from "@/lib/config" (e.g.,
getConfig or related symbols) in page.tsx; reorder the import statements to
place getCurrentUser first, save, and re-run the linter to ensure the import
ordering rule is satisfied.
In
`@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/_components/cellar-filters/index.tsx:
- Line 5: Replace the direct import of navigation utilities from
"next/navigation" with the i18n-aware exports from "@/lib/i18n": update the
import statement that currently imports useRouter and useSearchParams to instead
import them from "@/lib/i18n" so the component (cellar-filters/index.tsx) uses
the locale-aware router; ensure any usages of useRouter and useSearchParams in
this file remain unchanged but now come from the i18n wrapper.
- Line 3: The import order violates ESLint: move the external module import for
ServingFrom from "@db/enums" so it comes after the React import; update the top
of the file so "import React..." (or any React-related imports) appear before
"import { ServingFrom } from '@db/enums'". Ensure the new order follows existing
local/alias/project import grouping and run ESLint autofix or tests to confirm
the fix.
- Around line 85-91: The ServingFrom filter only renders BOTTLE and CAN; update
the select in the cellar filters component to include all enum variants from
ServingFrom (DRAFT, GROWLER, CASK) so users can filter those items too; add
<option> entries for ServingFrom.DRAFT, ServingFrom.GROWLER, and
ServingFrom.CASK using the corresponding translation keys (e.g.,
"cellar.form.servingFormat.values.DRAFT" etc.) in the same style as the existing
BOTTLE/CAN options so the dropdown covers every ServingFrom value.
In
`@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/_components/cellar-item-card/index.tsx:
- Around line 3-8: Reorder the imports so React-related imports come before
project-local ones: move the import of ServingFrom from "@db/enums" to after the
React/third-party imports (the ones importing MapPinIcon, MinusIcon, PlusIcon,
ShoppingBagIcon from "lucide-react" and useTranslations from "next-intl" /
useActionState from "react"); ensure the top-level import order places React and
external library imports first, then internal modules like ServingFrom to
satisfy ESLint.
- Around line 24-34: The prop `username` on CellarItemCard (declared in
CellarItemCardProps and the function signature for CellarItemCard) is unused and
triggers an ESLint error; remove `username` from the CellarItemCardProps
interface and from the destructured parameters of the CellarItemCard component,
then update all call sites that pass a username prop to stop providing it (or
use it where intended). Ensure only `item` and `storageLocations` remain in the
interface and component signature to resolve the lint error.
In
`@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/schemas.ts:
- Around line 7-12: The schema allows non-positive and non-integer pagination
values; update cellarSearchParamsSchema so page uses
z.coerce.number().int().min(1).optional().default(1) to clamp to positive
integers and expiringWithinDays uses z.coerce.number().int().min(1).optional()
to ensure it is a positive integer; keep storageLocationId and servingFormat
as-is (ServingFrom enum) and ensure you apply these validators to the page and
expiringWithinDays fields in the existing cellarSearchParamsSchema.
In
`@src/app/`[locale]/(business)/(without-header)/breweries/[brewerySlug]/beers/[beerSlug]/review/_components/form/index.tsx:
- Around line 69-75: The form initializes bestBeforeDate in useForm but doesn't
initialize the component state selectedDate, so pre-filled dates from
fields.getInputProps are discarded; add an effect that reads
fields.bestBeforeDate.value (or the field returned by useForm) on mount and when
it changes and calls setSelectedDate with that value (parsing to a Date or ISO
string as your component expects) so selectedDate reflects the form's
defaultValue; update any handlers that write to both fields.bestBeforeDate and
setSelectedDate to keep them in sync (references: useForm, fields.bestBeforeDate
/ fields.getInputProps, selectedDate, setSelectedDate).
In `@src/domain/cellar/index.ts`:
- Around line 139-150: The totalValue calculation (itemsWithPrice and
totalValue) currently sums purchasePrice across items regardless of
item.purchaseCurrency; update the logic to group and sum values by currency
instead of a single scalar or null: iterate itemsWithPrice and build a map keyed
by item.purchaseCurrency that accumulates Number(item.purchasePrice) *
item.quantity per currency, and replace totalValue with that currency-to-amount
map (or return grouped totals alongside existing result). Ensure all references
to totalValue, itemsWithPrice, purchasePrice, purchaseCurrency, and quantity are
adjusted to consume the new grouped totals.
- Line 9: Remove the unused import InvalidQuantityError from the import list in
this module; locate the import statement that currently includes
InvalidQuantityError and delete that identifier (or the entire import if it
becomes empty) so the file no longer imports an unused symbol.
- Line 1: Replace the invalid top-of-file string directive "server only" with
the proper Next.js server-only import by removing the string and adding import
"server-only"; at the module top so the file is marked as server-only (replace
the existing "server only" token).
- Around line 85-87: The code maps rawItems with the async
transformRawCellarItemToCellarItem but doesn't await the resulting promises, so
items becomes Promise<CellarItem>[]; change the assignment to await
Promise.all(rawItems.map(transformRawCellarItemToCellarItem)) so items is
CellarItem[] before passing to getPaginatedResults(itemCount, page, limit).
- Around line 277-303: The read-then-write sequence using
prisma.cellarItems.findUnique followed by prisma.cellarItems.update/delete is
racy; replace it with a single atomic transaction that uses
tx.cellarItems.update with data: { quantity: { increment: delta } } and a where
that includes both id and user.id to enforce ownership, then inside the same
transaction check the returned updated.quantity and call tx.cellarItems.delete
if quantity <= 0 (returning null), mapping prisma not-found errors to
UnknownCellarItemError/UnauthorizedCellarError as needed and still returning
transformRawCellarItemToCellarItem(updated) when quantity remains positive;
ensure all references to prisma.cellarItems.findUnique,
prisma.cellarItems.update, prisma.cellarItems.delete, UnknownCellarItemError,
UnauthorizedCellarError, and transformRawCellarItemToCellarItem are updated
accordingly.
In `@src/domain/cellar/transforms.ts`:
- Line 1: The module currently contains the invalid literal "server only";
replace that with a proper server-only directive by adding an
import-of-the-server-only-package (i.e. add an import statement that imports
"server-only") so the module is restricted to server runtime and ensure the
server-only package is installed; if instead this file is truly a Server Actions
module, use the "use server" directive at the top of the file rather than the
current "server only" string.
In `@src/domain/storage-locations/index.ts`:
- Around line 31-47: getStorageLocationById is unused; either delete it or add
the same ownership guard used by updateStorageLocation/deleteStorageLocation:
call getCurrentUser() at the start of getStorageLocationById, fetch the location
as you already do, if not found throw UnknownStorageLocationError(), then check
if location.userId !== user.id and throw a ForbiddenStorageLocationError (or the
project's equivalent) before returning the mapped StorageLocation; if you keep
it, also add any necessary imports (getCurrentUser and the forbidden error) to
match the existing security pattern.
🧹 Nitpick comments (2)
src/app/[locale]/(business)/(without-header)/breweries/[brewerySlug]/beers/[beerSlug]/review/actions.ts (1)
56-64: Improve error logging to include the actual error.The
catchblock logs a static message without the error details, making debugging difficult.🔧 Proposed fix
- } catch { - console.error("Failed to decrement cellar item quantity"); + } catch (error) { + console.error("Failed to decrement cellar item quantity", error); }src/app/[locale]/(business)/(with-header)/users/[username]/cellar/_components/cellar-stats/index.tsx (1)
83-86: Consider displaying a currency symbol for totalValue.The value is displayed as a raw number (e.g.,
123.45) without a currency indicator. If users have items in different currencies, this could be confusing. Consider either showing the dominant currency or indicating "mixed" when multiple currencies exist.
Comment on lines
+33
to
+60
| const inputRef = useRef<HTMLInputElement>(null); | ||
|
|
||
| const { key, name, ...restSelectProps } = getSelectProps(field); | ||
|
|
||
| const handleChange = (location: StorageLocation | null) => { | ||
| if (inputRef.current) { | ||
| inputRef.current.value = location?.id ?? ""; | ||
| const event = new Event("input", { bubbles: true }); | ||
| inputRef.current.dispatchEvent(event); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <div | ||
| className={cn("group/form-component", "flex w-full flex-col gap-y-1")} | ||
| > | ||
| <Label htmlFor={field.id} required={field.required}> | ||
| {label ?? t("cellar.form.storageLocation.label")} | ||
| </Label> | ||
|
|
||
| <input ref={inputRef} type="hidden" name={name} defaultValue="" /> | ||
|
|
||
| <StorageLocationSelector | ||
| key={key} | ||
| locations={locations} | ||
| selectedLocationId={field.value as string | undefined} | ||
| onChange={handleChange} | ||
| onCreateNew={onCreateNew} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the full file to understand the context
cat -n src/app/_components/form/storage-location-selector/index.tsxRepository: Zythogora/zythogora.com
Length of output: 2623
🏁 Script executed:
# Search for other uses of getSelectProps in the codebase to understand what it returns
rg "getSelectProps" -A 3 -B 1Repository: Zythogora/zythogora.com
Length of output: 8214
🏁 Script executed:
# Let's look at a similar form select component to see how it handles the hidden input
cat -n src/app/_components/form/style-select/index.tsxRepository: Zythogora/zythogora.com
Length of output: 2529
🏁 Script executed:
# Let's check country-select form component as well
cat -n src/app/_components/form/country-select/index.tsxRepository: Zythogora/zythogora.com
Length of output: 2444
🏁 Script executed:
# Let's verify what getSelectProps returns by checking conform-to documentation or implementation hints
# First check if there are any type definitions or tests that show what getSelectProps returns
rg "ReturnType.*getSelectProps" -A 5Repository: Zythogora/zythogora.com
Length of output: 2726
🏁 Script executed:
# Let's check if there's package.json to see conform-to version and understand its API better
cat package.json | jq '.dependencies | select(. != null) | to_entries[] | select(.key | contains("conform"))'Repository: Zythogora/zythogora.com
Length of output: 174
🏁 Script executed:
# Let's double-check what getSelectProps includes by searching for how it's used in a form action or submission context
rg "useFormAction\|form\s*method\|FormData" src/app/_components/form/ -B 2 -A 2 | head -40Repository: Zythogora/zythogora.com
Length of output: 49
🏁 Script executed:
# Let's search for where FormStorageLocationSelector is used to understand the context better
rg "FormStorageLocationSelector" -B 5 -A 5Repository: Zythogora/zythogora.com
Length of output: 4702
🏁 Script executed:
# Let's look at the add-to-cellar-modal to see form submission logic
cat -n src/app/_components/add-to-cellar-modal/index.tsx | head -100Repository: Zythogora/zythogora.com
Length of output: 3533
🏁 Script executed:
# Let's check the schema to understand what field.value should contain
cat -n src/app/\[locale\]/\(business\)/\(with-header\)/users/\[username\]/cellar/schemas.ts | head -50Repository: Zythogora/zythogora.com
Length of output: 1989
🏁 Script executed:
# Let's check if there's an edit mode or form that uses this with existing data
rg "storageLocationId" src/app --include="*.tsx" | grep -i "edit\|update" | head -20Repository: Zythogora/zythogora.com
Length of output: 508
🏁 Script executed:
# Let's search for edit functionality related to storage location
rg "edit|update" -t tsx src/app | grep -i "storage\|location" | head -20Repository: Zythogora/zythogora.com
Length of output: 94
🏁 Script executed:
# Let's search for edit or update patterns
rg "edit|Edit|UPDATE|update" src/app/_components/form/storage-location-selector/ -A 5Repository: Zythogora/zythogora.com
Length of output: 49
🏁 Script executed:
# Let's search for where forms are used with existing data (useForm with lastResult or defaultValue from server)
rg "useForm" src/app/_components --A 10 | grep -A 10 "defaultValue\|lastResult" | head -40Repository: Zythogora/zythogora.com
Length of output: 472
🏁 Script executed:
# Let's search for useForm with context
rg "useForm" src/app/_components -A 10 | head -60Repository: Zythogora/zythogora.com
Length of output: 3382
🏁 Script executed:
# Let's verify the behavior by checking how conform-to's getSelectProps works
# and whether field.value would be populated on form edit
web_searchRepository: Zythogora/zythogora.com
Length of output: 117
Hidden input doesn't reflect initial selection.
When editing a form with an existing storage location, the hidden input's defaultValue="" means the form will submit an empty value unless the user explicitly re-selects the location. This differs from working implementations like FormStyleSelect and FormCountrySelect, which use state-driven values instead of hardcoded defaults.
Additionally, useEffect is imported but unused.
🛠️ Proposed fix
- const { key, name, ...restSelectProps } = getSelectProps(field);
+ const { key, name, defaultValue, ...restSelectProps } = getSelectProps(field);
@@
- <input ref={inputRef} type="hidden" name={name} defaultValue="" />
+ <input
+ ref={inputRef}
+ key={key}
+ type="hidden"
+ name={name}
+ defaultValue={defaultValue ?? field.value ?? ""}
+ />Remove the unused useEffect import from line 5.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const inputRef = useRef<HTMLInputElement>(null); | |
| const { key, name, ...restSelectProps } = getSelectProps(field); | |
| const handleChange = (location: StorageLocation | null) => { | |
| if (inputRef.current) { | |
| inputRef.current.value = location?.id ?? ""; | |
| const event = new Event("input", { bubbles: true }); | |
| inputRef.current.dispatchEvent(event); | |
| } | |
| }; | |
| return ( | |
| <div | |
| className={cn("group/form-component", "flex w-full flex-col gap-y-1")} | |
| > | |
| <Label htmlFor={field.id} required={field.required}> | |
| {label ?? t("cellar.form.storageLocation.label")} | |
| </Label> | |
| <input ref={inputRef} type="hidden" name={name} defaultValue="" /> | |
| <StorageLocationSelector | |
| key={key} | |
| locations={locations} | |
| selectedLocationId={field.value as string | undefined} | |
| onChange={handleChange} | |
| onCreateNew={onCreateNew} | |
| const inputRef = useRef<HTMLInputElement>(null); | |
| const { key, name, defaultValue, ...restSelectProps } = getSelectProps(field); | |
| const handleChange = (location: StorageLocation | null) => { | |
| if (inputRef.current) { | |
| inputRef.current.value = location?.id ?? ""; | |
| const event = new Event("input", { bubbles: true }); | |
| inputRef.current.dispatchEvent(event); | |
| } | |
| }; | |
| return ( | |
| <div | |
| className={cn("group/form-component", "flex w-full flex-col gap-y-1")} | |
| > | |
| <Label htmlFor={field.id} required={field.required}> | |
| {label ?? t("cellar.form.storageLocation.label")} | |
| </Label> | |
| <input | |
| ref={inputRef} | |
| key={key} | |
| type="hidden" | |
| name={name} | |
| defaultValue={defaultValue ?? field.value ?? ""} | |
| /> | |
| <StorageLocationSelector | |
| key={key} | |
| locations={locations} | |
| selectedLocationId={field.value as string | undefined} | |
| onChange={handleChange} | |
| onCreateNew={onCreateNew} |
🤖 Prompt for AI Agents
In `@src/app/_components/form/storage-location-selector/index.tsx` around lines 33
- 60, The hidden input's defaultValue is hardcoded to "" so existing selections
aren't submitted; change the hidden input to use the field value (e.g.
defaultValue={field.value as string ?? ""}) or initialize the inputRef value
from field.value on mount, and keep handleChange updating inputRef as-is; also
remove the unused useEffect import. Update references around inputRef,
handleChange, getSelectProps/field and StorageLocationSelector to ensure
selectedLocationId continues to read from field.value.
Comment on lines
+85
to
+87
| const items = rawItems.map(transformRawCellarItemToCellarItem); | ||
|
|
||
| return getPaginatedResults(items, itemCount, page, limit); |
There was a problem hiding this comment.
Missing await Promise.all() for async transforms.
transformRawCellarItemToCellarItem is an async function returning Promise<CellarItem>. Using .map() with an async function returns Promise<CellarItem>[], not CellarItem[]. The paginated results will contain unresolved Promise objects.
🐛 Proposed fix
- const items = rawItems.map(transformRawCellarItemToCellarItem);
+ const items = await Promise.all(
+ rawItems.map(transformRawCellarItemToCellarItem),
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const items = rawItems.map(transformRawCellarItemToCellarItem); | |
| return getPaginatedResults(items, itemCount, page, limit); | |
| const items = await Promise.all( | |
| rawItems.map(transformRawCellarItemToCellarItem), | |
| ); | |
| return getPaginatedResults(items, itemCount, page, limit); |
🤖 Prompt for AI Agents
In `@src/domain/cellar/index.ts` around lines 85 - 87, The code maps rawItems with
the async transformRawCellarItemToCellarItem but doesn't await the resulting
promises, so items becomes Promise<CellarItem>[]; change the assignment to await
Promise.all(rawItems.map(transformRawCellarItemToCellarItem)) so items is
CellarItem[] before passing to getPaginatedResults(itemCount, page, limit).
Comment on lines
+139
to
+150
| // Calculate total value (only for items with price) | ||
| const itemsWithPrice = items.filter( | ||
| (item) => item.purchasePrice !== null, | ||
| ); | ||
| const totalValue = | ||
| itemsWithPrice.length > 0 | ||
| ? itemsWithPrice.reduce( | ||
| (sum, item) => | ||
| sum + Number(item.purchasePrice) * item.quantity, | ||
| 0, | ||
| ) | ||
| : null; |
There was a problem hiding this comment.
Total value calculation ignores currency differences.
Items may have different purchaseCurrency values. Summing prices across currencies produces a meaningless total. Consider either filtering to a single currency, converting to a base currency, or grouping totals by currency.
Would you like me to propose an implementation that groups the total value by currency?
🤖 Prompt for AI Agents
In `@src/domain/cellar/index.ts` around lines 139 - 150, The totalValue
calculation (itemsWithPrice and totalValue) currently sums purchasePrice across
items regardless of item.purchaseCurrency; update the logic to group and sum
values by currency instead of a single scalar or null: iterate itemsWithPrice
and build a map keyed by item.purchaseCurrency that accumulates
Number(item.purchasePrice) * item.quantity per currency, and replace totalValue
with that currency-to-amount map (or return grouped totals alongside existing
result). Ensure all references to totalValue, itemsWithPrice, purchasePrice,
purchaseCurrency, and quantity are adjusted to consume the new grouped totals.
Comment on lines
+277
to
+303
| const item = await prisma.cellarItems.findUnique({ | ||
| where: { id: itemId }, | ||
| }); | ||
|
|
||
| if (!item) { | ||
| throw new UnknownCellarItemError(); | ||
| } | ||
|
|
||
| if (item.userId !== user.id) { | ||
| throw new UnauthorizedCellarError(); | ||
| } | ||
|
|
||
| const newQuantity = item.quantity + delta; | ||
|
|
||
| if (newQuantity <= 0) { | ||
| await prisma.cellarItems.delete({ where: { id: itemId } }); | ||
| return null; | ||
| } | ||
|
|
||
| const updatedItem = await prisma.cellarItems.update({ | ||
| where: { id: itemId }, | ||
| data: { quantity: newQuantity }, | ||
| include: cellarItemInclude, | ||
| }); | ||
|
|
||
| return transformRawCellarItemToCellarItem(updatedItem); | ||
| }; |
There was a problem hiding this comment.
Potential race condition in quantity adjustment.
The read (line 277) and write (lines 292-300) operations are not atomic. Concurrent requests could read the same quantity and apply conflicting deltas, resulting in incorrect final values or duplicate deletion attempts.
Consider using a transaction with an atomic update pattern.
🔒 Proposed fix using atomic update
- const item = await prisma.cellarItems.findUnique({
- where: { id: itemId },
- });
-
- if (!item) {
- throw new UnknownCellarItemError();
- }
-
- if (item.userId !== user.id) {
- throw new UnauthorizedCellarError();
- }
-
- const newQuantity = item.quantity + delta;
-
- if (newQuantity <= 0) {
- await prisma.cellarItems.delete({ where: { id: itemId } });
- return null;
- }
-
- const updatedItem = await prisma.cellarItems.update({
- where: { id: itemId },
- data: { quantity: newQuantity },
- include: cellarItemInclude,
- });
-
- return transformRawCellarItemToCellarItem(updatedItem);
+ return getPrismaTransactionClient()(async (tx) => {
+ const item = await tx.cellarItems.findUnique({
+ where: { id: itemId },
+ });
+
+ if (!item) {
+ throw new UnknownCellarItemError();
+ }
+
+ if (item.userId !== user.id) {
+ throw new UnauthorizedCellarError();
+ }
+
+ const newQuantity = item.quantity + delta;
+
+ if (newQuantity <= 0) {
+ await tx.cellarItems.delete({ where: { id: itemId } });
+ return null;
+ }
+
+ const updatedItem = await tx.cellarItems.update({
+ where: { id: itemId },
+ data: { quantity: newQuantity },
+ include: cellarItemInclude,
+ });
+
+ return transformRawCellarItemToCellarItem(updatedItem);
+ });🤖 Prompt for AI Agents
In `@src/domain/cellar/index.ts` around lines 277 - 303, The read-then-write
sequence using prisma.cellarItems.findUnique followed by
prisma.cellarItems.update/delete is racy; replace it with a single atomic
transaction that uses tx.cellarItems.update with data: { quantity: { increment:
delta } } and a where that includes both id and user.id to enforce ownership,
then inside the same transaction check the returned updated.quantity and call
tx.cellarItems.delete if quantity <= 0 (returning null), mapping prisma
not-found errors to UnknownCellarItemError/UnauthorizedCellarError as needed and
still returning transformRawCellarItemToCellarItem(updated) when quantity
remains positive; ensure all references to prisma.cellarItems.findUnique,
prisma.cellarItems.update, prisma.cellarItems.delete, UnknownCellarItemError,
UnauthorizedCellarError, and transformRawCellarItemToCellarItem are updated
accordingly.
| @@ -0,0 +1,47 @@ | |||
| "server only"; | |||
There was a problem hiding this comment.
Invalid server-only directive syntax.
The string "server only" is not a valid directive. To restrict this module to server-side usage, you should either:
- Use
import "server-only";(requires installing theserver-onlypackage), or - Use
"use server";if this is intended to be a Server Actions module (though this is a transform, not an action)
For a transform file, the server-only import is more appropriate.
🔧 Proposed fix
-"server only";
+import "server-only";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "server only"; | |
| import "server-only"; |
🤖 Prompt for AI Agents
In `@src/domain/cellar/transforms.ts` at line 1, The module currently contains the
invalid literal "server only"; replace that with a proper server-only directive
by adding an import-of-the-server-only-package (i.e. add an import statement
that imports "server-only") so the module is restricted to server runtime and
ensure the server-only package is installed; if instead this file is truly a
Server Actions module, use the "use server" directive at the top of the file
rather than the current "server only" string.
Comment on lines
+31
to
+47
| export const getStorageLocationById = cache( | ||
| async (locationId: string): Promise<StorageLocation> => { | ||
| const location = await prisma.storageLocations.findUnique({ | ||
| where: { id: locationId }, | ||
| }); | ||
|
|
||
| if (!location) { | ||
| throw new UnknownStorageLocationError(); | ||
| } | ||
|
|
||
| return { | ||
| id: location.id, | ||
| name: location.name, | ||
| userId: location.userId, | ||
| createdAt: location.createdAt, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=ts --type=tsx 'getStorageLocationById\s*\(' -B 2 -A 2Repository: Zythogora/zythogora.com
Length of output: 94
🏁 Script executed:
rg -n 'getStorageLocationById\s*\(' -B 2 -A 2Repository: Zythogora/zythogora.com
Length of output: 49
🏁 Script executed:
rg -n 'getStorageLocationById' -B 2 -A 2Repository: Zythogora/zythogora.com
Length of output: 444
🏁 Script executed:
# Let me see the full context of the storage-locations index file
cat -n src/domain/storage-locations/index.tsRepository: Zythogora/zythogora.com
Length of output: 6209
🏁 Script executed:
# Find where the storage-locations index is imported
rg -n 'from.*storage-locations' -B 2 -A 2Repository: Zythogora/zythogora.com
Length of output: 6185
🏁 Script executed:
# Check if getStorageLocationById is truly never used
# Search in all files (not just imports)
rg 'getStorageLocationById' --type ts --type jsRepository: Zythogora/zythogora.com
Length of output: 149
🏁 Script executed:
# Double-check by looking at the export statements
head -50 src/domain/storage-locations/index.ts | grep "export"Repository: Zythogora/zythogora.com
Length of output: 209
Remove unused function or add ownership checks to match security pattern.
getStorageLocationById is currently unused (no call sites found in the codebase). Either remove it as dead code, or if it's kept for future use, add an ownership check to match the pattern used in updateStorageLocation and deleteStorageLocation—both of which call getCurrentUser() and verify location.userId === user.id.
🤖 Prompt for AI Agents
In `@src/domain/storage-locations/index.ts` around lines 31 - 47,
getStorageLocationById is unused; either delete it or add the same ownership
guard used by updateStorageLocation/deleteStorageLocation: call getCurrentUser()
at the start of getStorageLocationById, fetch the location as you already do, if
not found throw UnknownStorageLocationError(), then check if location.userId !==
user.id and throw a ForbiddenStorageLocationError (or the project's equivalent)
before returning the mapped StorageLocation; if you keep it, also add any
necessary imports (getCurrentUser and the forbidden error) to match the existing
security pattern.
Implements a comprehensive beer cellar system allowing users to: - Add beers to their cellar with quantity, serving format, best before date, purchase date, price, and storage location - View cellar items with expiration status indicators - Filter by storage location, serving format, and expiration timeframe - Adjust quantities (+/-) with automatic deletion at zero - Move items between storage locations - Navigate to "Open & Review" with pre-filled data from cellar - Automatically decrement cellar quantity after submitting a review Includes: - Prisma schema for CellarItems, StorageLocations, and PurchaseLocations - Domain modules for cellar and storage-locations - UI components for cellar serving format and storage location selection - Cellar page with stats, filters, and item cards - AddToCellarModal for beer pages - Review form pre-fill integration - i18n translations for English and French
Kittease
force-pushed
the
claude/add-beer-collections-WlF7q
branch
from
February 1, 2026 11:53
d768e31 to
dc978a9
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@src/app/_components/form/cellar-serving-format-selector/index.tsx`:
- Around line 27-52: The defaultValue returned from getInputProps is being
discarded and not passed to the RadioGroup-based component, so pass defaultValue
through when rendering CellarServingFormatSelector; locate where getInputProps
is destructured (restInputProps / defaultValue) and include defaultValue in the
props spread to CellarServingFormatSelector (it accepts RadioGroup.Root props),
e.g. forward defaultValue alongside ariaInputProps and restProps so the
RadioGroup initial selection is preserved.
In `@src/app/_components/ui/cellar-serving-format-selector/index.tsx`:
- Around line 77-103: The radio items rendered from cellarServingFormatValues
(inside RadioGroup.Item) are icon-only and need accessible labels for screen
readers; update the RadioGroup.Item elements to provide an accessible name by
adding an aria-label (or include a visually hidden span) derived from the option
value (or a mapping like a getLabelForServingFormat function) so each
RadioGroup.Item and/or the CellarServingFormatIcon has a clear label; keep the
existing RadioGroup.Indicator and visual markup but ensure the label string is
descriptive (e.g., "Bottle", "Can", etc.) and use the same unique identifiers
(RadioGroup.Item, CellarServingFormatIcon, cellarServingFormatValues) so screen
readers can distinguish each option.
In
`@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/_components/cellar-item-card/index.tsx:
- Around line 151-176: The onChange handler for the storage location select is
directly calling moveAction(formData), bypassing the form's native submission;
update the handler to trigger a programmatic form submit instead (so the form's
action={moveAction} is used). Specifically, in the select's onChange replace the
FormData + moveAction call with locating the surrounding form
(e.target.closest("form")), updating the select/native input value if needed
(the select already has name="storageLocationId"), then call
form.requestSubmit() (or form.submit() if requestSubmit is not available) to let
the browser/React invoke moveAction via the form action; keep references to
moveAction, the select onChange handler, and the existing hidden input
name="itemId" intact.
In
`@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/actions.ts:
- Around line 192-203: createStorageLocationAction currently calls
createStorageLocation without handling domain errors; wrap the
createStorageLocation call in a try/catch, explicitly catch
DuplicateStorageLocationError and surface it (e.g., throw it or convert it to
the same error shape other actions use), keep the existing
UnauthorizedStorageLocationError behavior from getCurrentUser, and rethrow any
other unexpected errors so they propagate; update createStorageLocationAction
(and its use of getCurrentUser, revalidatePath, generatePath, Routes.CELLAR) to
follow the same error-handling pattern as the other actions.
In `@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/page.tsx:
- Around line 59-65: The redirect to the canonical username is missing a return
so execution continues after calling redirect; update the conditional that
compares user.username and username to return the redirect call (i.e., return
redirect(...)) so that after calling redirect (with generatePath(Routes.CELLAR,
{ username: user.username }) and locale) the function exits and no further code
runs.
In
`@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/schemas.ts:
- Around line 45-48: moveItemSchema currently declares storageLocationId as
z.string().nullable(), but the select sends "" for "None", which fails
validation; update the storageLocationId entry in moveItemSchema to
preprocess/transform empty string into null (e.g., use z.preprocess to convert
val === "" to null before validating with z.string().nullable()) so that the
form's "" maps to null and validation succeeds.
In
`@src/app/`[locale]/(business)/(without-header)/breweries/[brewerySlug]/beers/[beerSlug]/review/actions.ts:
- Around line 57-66: The catch block that handles failures from
adjustCellarItemQuantity(cellarItemId, -1) currently logs a static message;
update the catch to accept the thrown error (e.g., catch (err)) and include that
error in the log so you call console.error with a descriptive message plus the
error object (reference: adjustCellarItemQuantity, revalidatePath, generatePath,
Routes.CELLAR, cellarItemId) to make runtime failures debuggable.
In `@src/lib/i18n/translations/en.json`:
- Around line 866-870: Add the missing translation key used by AddToCellarModal:
define "cellar.addToCellar.success" in the en.json translations with a string
containing the {beerName} placeholder (e.g., a success message like "Added
{beerName} to your cellar"). This ensures the toast.success call in
AddToCellarModal can resolve cellar.addToCellar.success with the {beerName}
variable.
- Around line 834-841: Translation set for serving formats is missing entries
for ServingFrom enum values DRAFT, GROWLER, and CASK; add keys under the JSON
object "form.servingFormat.values" for "DRAFT", "GROWLER", and "CASK" with
appropriate English strings (e.g., "Draft", "Growler", "Cask") so UI dropdowns
and forms that rely on the ServingFrom enum render correctly; ensure keys
exactly match the enum symbol names (DRAFT, GROWLER, CASK) to align with lookup
code that uses those identifiers.
🧹 Nitpick comments (3)
src/domain/cellar/errors.ts (1)
1-29: Consider setting prototype and name for robust error handling.Custom error classes in TypeScript can have issues with
instanceofchecks when transpiled. Setting the prototype explicitly and assigningthis.nameimproves debuggability and ensures correct behavior.♻️ Proposed improvement
export class CellarError extends Error { constructor(message: string) { super(message); + this.name = "CellarError"; + Object.setPrototypeOf(this, new.target.prototype); } } export class UnauthorizedCellarError extends CellarError { constructor() { super("Unauthorized cellar operation"); + this.name = "UnauthorizedCellarError"; } } export class UnknownCellarItemError extends CellarError { constructor() { super("Unknown cellar item"); + this.name = "UnknownCellarItemError"; } } export class InvalidQuantityError extends CellarError { constructor() { super("Invalid quantity"); + this.name = "InvalidQuantityError"; } } export class UnknownBeerError extends CellarError { constructor() { super("Unknown beer"); + this.name = "UnknownBeerError"; } }src/app/_components/add-to-cellar-modal/index.tsx (1)
121-127: Add error handling for storage location creation.If
createStorageLocationActionthrows, the error will propagate unhandled, potentially leaving the UI in an inconsistent state. Consider wrapping in try/catch with user feedback.♻️ Proposed fix
const handleCreateStorageLocation = async (name: string) => { - const newLocation = await createStorageLocationAction(name); - setLocations((prev) => [...prev, newLocation].sort((a, b) => - a.name.localeCompare(b.name) - )); - return newLocation; + try { + const newLocation = await createStorageLocationAction(name); + setLocations((prev) => [...prev, newLocation].sort((a, b) => + a.name.localeCompare(b.name) + )); + return newLocation; + } catch (error) { + toast.error(t("cellar.form.errors.SOMETHING_WENT_WRONG")); + throw error; + } };src/app/[locale]/(business)/(with-header)/users/[username]/cellar/_components/cellar-item-card/index.tsx (1)
37-38: Action state results are ignored.The first element of
useActionState(the state) is discarded. Consider handling error states to provide user feedback when actions fail.💡 Example improvement
- const [, adjustAction] = useActionState(adjustQuantityAction, null); - const [, moveAction] = useActionState(moveItemAction, null); + const [adjustState, adjustAction] = useActionState(adjustQuantityAction, null); + const [moveState, moveAction] = useActionState(moveItemAction, null); + + // Optionally show error toast/notification when adjustState?.error or moveState?.error
Comment on lines
+27
to
+52
| const { key, ...restInputProps } = getInputProps(field, { | ||
| type: "text", | ||
| ariaAttributes: true, | ||
| }); | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const { defaultValue, value, type, ...ariaInputProps } = restInputProps; | ||
|
|
||
| return ( | ||
| <div | ||
| className={cn("group/form-component", "flex w-full flex-col gap-y-1")} | ||
| > | ||
| <Label htmlFor={field.id} required={field.required}> | ||
| {field.value | ||
| ? t( | ||
| `cellar.form.servingFormat.values.${field.value as CellarServingFormat}`, | ||
| ) | ||
| : t("cellar.form.servingFormat.label")} | ||
| </Label> | ||
|
|
||
| <div className="w-full"> | ||
| <CellarServingFormatSelector | ||
| key={key} | ||
| {...ariaInputProps} | ||
| {...restProps} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the CellarServingFormatSelector component
fd "cellar-serving-format-selector" -type fRepository: Zythogora/zythogora.com
Length of output: 237
🏁 Script executed:
# Let's also check the specific file mentioned in the review
cat -n src/app/_components/form/cellar-serving-format-selector/index.tsxRepository: Zythogora/zythogora.com
Length of output: 2169
🏁 Script executed:
# Search for CellarServingFormatSelector component definition
rg "export.*CellarServingFormatSelector" -A 20Repository: Zythogora/zythogora.com
Length of output: 291
🏁 Script executed:
# Read the CellarServingFormatSelector UI component
cat -n src/app/_components/ui/cellar-serving-format-selector/index.tsxRepository: Zythogora/zythogora.com
Length of output: 4084
🏁 Script executed:
# Search for RadioGroup component to understand how it's being used
rg "RadioGroup" -B 2 -A 5 src/app/_components/Repository: Zythogora/zythogora.com
Length of output: 15383
Pass the initial value to RadioGroup to preserve selections.
defaultValue from getInputProps is currently discarded (line 33) and never passed to CellarServingFormatSelector. Since CellarServingFormatSelectorProps extends ComponentProps<typeof RadioGroup.Root>, the component accepts defaultValue, but without it, the RadioGroup has no initial value set, causing selected values to be lost on re-renders.
✅ Suggested fix
- const { key, ...restInputProps } = getInputProps(field, {
+ const { key, defaultValue, ...restInputProps } = getInputProps(field, {
type: "text",
ariaAttributes: true,
});
- // eslint-disable-next-line `@typescript-eslint/no-unused-vars`
- const { defaultValue, value, type, ...ariaInputProps } = restInputProps;
+ const { value: _value, type: _type, ...ariaInputProps } = restInputProps;
@@
<div className="w-full">
<CellarServingFormatSelector
key={key}
+ defaultValue={
+ (defaultValue ?? field.value) as CellarServingFormat | undefined
+ }
{...ariaInputProps}
{...restProps}
/>🤖 Prompt for AI Agents
In `@src/app/_components/form/cellar-serving-format-selector/index.tsx` around
lines 27 - 52, The defaultValue returned from getInputProps is being discarded
and not passed to the RadioGroup-based component, so pass defaultValue through
when rendering CellarServingFormatSelector; locate where getInputProps is
destructured (restInputProps / defaultValue) and include defaultValue in the
props spread to CellarServingFormatSelector (it accepts RadioGroup.Root props),
e.g. forward defaultValue alongside ariaInputProps and restProps so the
RadioGroup initial selection is preserved.
Comment on lines
+77
to
+103
| {cellarServingFormatValues.map((value) => ( | ||
| <RadioGroup.Item | ||
| key={value} | ||
| value={value} | ||
| className={cn( | ||
| "group/cellar-serving-format-item", | ||
| "border-foreground relative flex items-center justify-center border-2 px-4", | ||
| "py-4 @3xl:py-6", | ||
| "before:bg-foreground before:absolute before:-inset-x-0.5 before:top-0 before:-bottom-1 before:z-[-2]", | ||
| "first-of-type:rounded-l last-of-type:rounded-r", | ||
| "first-of-type:before:rounded-l last-of-type:before:rounded-r", | ||
| "data-[state=checked]:bg-primary data-[state=checked]:-bottom-0.5 data-[state=checked]:before:hidden", | ||
| "focus-visible:bottom-0! focus-visible:z-50", | ||
| "focus-visible:before:hidden", | ||
| )} | ||
| > | ||
| <CellarServingFormatIcon | ||
| size={32} | ||
| type={value} | ||
| className={cn( | ||
| "fill-foreground overflow-visible", | ||
| "size-8 @3xl:size-10", | ||
| "group-data-[state=checked]/cellar-serving-format-item:fill-stone-950", | ||
| )} | ||
| /> | ||
|
|
||
| <RadioGroup.Indicator className="sr-only" /> |
There was a problem hiding this comment.
Add accessible labels for the serving-format options.
The radio items are icon-only, so screen readers can’t distinguish the choices. Add aria-label (and/or sr-only text) per option.
♿ Suggested fix
{cellarServingFormatValues.map((value) => (
<RadioGroup.Item
key={value}
value={value}
+ aria-label={value}
className={cn(
"group/cellar-serving-format-item",
"border-foreground relative flex items-center justify-center border-2 px-4",
"py-4 `@3xl`:py-6",
"before:bg-foreground before:absolute before:-inset-x-0.5 before:top-0 before:-bottom-1 before:z-[-2]",
"first-of-type:rounded-l last-of-type:rounded-r",
"first-of-type:before:rounded-l last-of-type:before:rounded-r",
"data-[state=checked]:bg-primary data-[state=checked]:-bottom-0.5 data-[state=checked]:before:hidden",
"focus-visible:bottom-0! focus-visible:z-50",
"focus-visible:before:hidden",
)}
>
+ <span className="sr-only">{value}</span>
<CellarServingFormatIcon
size={32}
type={value}
className={cn(
"fill-foreground overflow-visible",
"size-8 `@3xl`:size-10",
"group-data-[state=checked]/cellar-serving-format-item:fill-stone-950",
)}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {cellarServingFormatValues.map((value) => ( | |
| <RadioGroup.Item | |
| key={value} | |
| value={value} | |
| className={cn( | |
| "group/cellar-serving-format-item", | |
| "border-foreground relative flex items-center justify-center border-2 px-4", | |
| "py-4 @3xl:py-6", | |
| "before:bg-foreground before:absolute before:-inset-x-0.5 before:top-0 before:-bottom-1 before:z-[-2]", | |
| "first-of-type:rounded-l last-of-type:rounded-r", | |
| "first-of-type:before:rounded-l last-of-type:before:rounded-r", | |
| "data-[state=checked]:bg-primary data-[state=checked]:-bottom-0.5 data-[state=checked]:before:hidden", | |
| "focus-visible:bottom-0! focus-visible:z-50", | |
| "focus-visible:before:hidden", | |
| )} | |
| > | |
| <CellarServingFormatIcon | |
| size={32} | |
| type={value} | |
| className={cn( | |
| "fill-foreground overflow-visible", | |
| "size-8 @3xl:size-10", | |
| "group-data-[state=checked]/cellar-serving-format-item:fill-stone-950", | |
| )} | |
| /> | |
| <RadioGroup.Indicator className="sr-only" /> | |
| {cellarServingFormatValues.map((value) => ( | |
| <RadioGroup.Item | |
| key={value} | |
| value={value} | |
| aria-label={value} | |
| className={cn( | |
| "group/cellar-serving-format-item", | |
| "border-foreground relative flex items-center justify-center border-2 px-4", | |
| "py-4 `@3xl`:py-6", | |
| "before:bg-foreground before:absolute before:-inset-x-0.5 before:top-0 before:-bottom-1 before:z-[-2]", | |
| "first-of-type:rounded-l last-of-type:rounded-r", | |
| "first-of-type:before:rounded-l last-of-type:before:rounded-r", | |
| "data-[state=checked]:bg-primary data-[state=checked]:-bottom-0.5 data-[state=checked]:before:hidden", | |
| "focus-visible:bottom-0! focus-visible:z-50", | |
| "focus-visible:before:hidden", | |
| )} | |
| > | |
| <span className="sr-only">{value}</span> | |
| <CellarServingFormatIcon | |
| size={32} | |
| type={value} | |
| className={cn( | |
| "fill-foreground overflow-visible", | |
| "size-8 `@3xl`:size-10", | |
| "group-data-[state=checked]/cellar-serving-format-item:fill-stone-950", | |
| )} | |
| /> | |
| <RadioGroup.Indicator className="sr-only" /> |
🤖 Prompt for AI Agents
In `@src/app/_components/ui/cellar-serving-format-selector/index.tsx` around lines
77 - 103, The radio items rendered from cellarServingFormatValues (inside
RadioGroup.Item) are icon-only and need accessible labels for screen readers;
update the RadioGroup.Item elements to provide an accessible name by adding an
aria-label (or include a visually hidden span) derived from the option value (or
a mapping like a getLabelForServingFormat function) so each RadioGroup.Item
and/or the CellarServingFormatIcon has a clear label; keep the existing
RadioGroup.Indicator and visual markup but ensure the label string is
descriptive (e.g., "Bottle", "Can", etc.) and use the same unique identifiers
(RadioGroup.Item, CellarServingFormatIcon, cellarServingFormatValues) so screen
readers can distinguish each option.
Comment on lines
+151
to
+176
| <form action={moveAction}> | ||
| <input type="hidden" name="itemId" value={item.id} /> | ||
| <select | ||
| name="storageLocationId" | ||
| value={item.storageLocation?.id ?? ""} | ||
| onChange={(e) => { | ||
| const form = e.target.closest("form"); | ||
| if (form) { | ||
| const formData = new FormData(form); | ||
| formData.set("storageLocationId", e.target.value || ""); | ||
| moveAction(formData); | ||
| } | ||
| }} | ||
| className={cn( | ||
| "rounded border border-foreground/20 bg-transparent px-1.5 py-0.5 text-sm", | ||
| "focus:border-primary focus:outline-none", | ||
| )} | ||
| > | ||
| <option value="">{t("cellar.form.storageLocation.none")}</option> | ||
| {storageLocations.map((location) => ( | ||
| <option key={location.id} value={location.id}> | ||
| {location.name} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </form> |
There was a problem hiding this comment.
Storage location onChange directly invokes server action.
Calling moveAction(formData) directly bypasses React's form action handling and may cause issues with concurrent state updates. The form already has action={moveAction}, so submitting the form programmatically is safer.
🛠️ Proposed fix
onChange={(e) => {
const form = e.target.closest("form");
if (form) {
- const formData = new FormData(form);
- formData.set("storageLocationId", e.target.value || "");
- moveAction(formData);
+ form.requestSubmit();
}
}}🤖 Prompt for AI Agents
In
`@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/_components/cellar-item-card/index.tsx
around lines 151 - 176, The onChange handler for the storage location select is
directly calling moveAction(formData), bypassing the form's native submission;
update the handler to trigger a programmatic form submit instead (so the form's
action={moveAction} is used). Specifically, in the select's onChange replace the
FormData + moveAction call with locating the surrounding form
(e.target.closest("form")), updating the select/native input value if needed
(the select already has name="storageLocationId"), then call
form.requestSubmit() (or form.submit() if requestSubmit is not available) to let
the browser/React invoke moveAction via the form action; keep references to
moveAction, the select onChange handler, and the existing hidden input
name="itemId" intact.
Comment on lines
+192
to
+203
| export const createStorageLocationAction = async (name: string) => { | ||
| const user = await getCurrentUser(); | ||
| if (!user) { | ||
| throw new UnauthorizedStorageLocationError(); | ||
| } | ||
|
|
||
| const location = await createStorageLocation(name); | ||
|
|
||
| revalidatePath(generatePath(Routes.CELLAR, { username: user.username })); | ||
|
|
||
| return location; | ||
| }; |
There was a problem hiding this comment.
Missing error handling for createStorageLocation.
Unlike other actions, createStorageLocationAction doesn't catch errors from the domain call. Per the relevant code snippet, createStorageLocation can throw DuplicateStorageLocationError which would propagate as an unhandled exception.
🛠️ Proposed fix
+import { DuplicateStorageLocationError, UnauthorizedStorageLocationError } from "@/domain/storage-locations/errors";
export const createStorageLocationAction = async (name: string) => {
const user = await getCurrentUser();
if (!user) {
throw new UnauthorizedStorageLocationError();
}
+ try {
const location = await createStorageLocation(name);
revalidatePath(generatePath(Routes.CELLAR, { username: user.username }));
return location;
+ } catch (error) {
+ if (error instanceof DuplicateStorageLocationError) {
+ throw error; // or return { error: "Location already exists" }
+ }
+ console.error("Failed to create storage location:", error);
+ throw error;
+ }
};🤖 Prompt for AI Agents
In `@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/actions.ts
around lines 192 - 203, createStorageLocationAction currently calls
createStorageLocation without handling domain errors; wrap the
createStorageLocation call in a try/catch, explicitly catch
DuplicateStorageLocationError and surface it (e.g., throw it or convert it to
the same error shape other actions use), keep the existing
UnauthorizedStorageLocationError behavior from getCurrentUser, and rethrow any
other unexpected errors so they propagate; update createStorageLocationAction
(and its use of getCurrentUser, revalidatePath, generatePath, Routes.CELLAR) to
follow the same error-handling pattern as the other actions.
Comment on lines
+59
to
+65
| // Redirect to canonical username | ||
| if (user.username !== username) { | ||
| redirect({ | ||
| href: generatePath(Routes.CELLAR, { username: user.username }), | ||
| locale, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Missing return before redirect causes unreachable code execution.
The canonical username redirect is missing a return statement. Without it, execution continues past the redirect call, potentially causing unintended behavior or errors.
🐛 Proposed fix
// Redirect to canonical username
if (user.username !== username) {
- redirect({
+ return redirect({
href: generatePath(Routes.CELLAR, { username: user.username }),
locale,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Redirect to canonical username | |
| if (user.username !== username) { | |
| redirect({ | |
| href: generatePath(Routes.CELLAR, { username: user.username }), | |
| locale, | |
| }); | |
| } | |
| // Redirect to canonical username | |
| if (user.username !== username) { | |
| return redirect({ | |
| href: generatePath(Routes.CELLAR, { username: user.username }), | |
| locale, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/page.tsx
around lines 59 - 65, The redirect to the canonical username is missing a return
so execution continues after calling redirect; update the conditional that
compares user.username and username to return the redirect call (i.e., return
redirect(...)) so that after calling redirect (with generatePath(Routes.CELLAR,
{ username: user.username }) and locale) the function exits and no further code
runs.
Comment on lines
+45
to
+48
| export const moveItemSchema = z.object({ | ||
| itemId: z.string(), | ||
| storageLocationId: z.string().nullable(), | ||
| }); |
There was a problem hiding this comment.
Schema expects null but form sends empty string.
moveItemSchema defines storageLocationId as z.string().nullable(), but the select element in CellarItemCard sends an empty string "" when "None" is selected. This will fail validation since "" is not null.
🐛 Proposed fix - transform empty string to null
export const moveItemSchema = z.object({
itemId: z.string(),
- storageLocationId: z.string().nullable(),
+ storageLocationId: z.preprocess(
+ (val) => (val === "" ? null : val),
+ z.string().nullable(),
+ ),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const moveItemSchema = z.object({ | |
| itemId: z.string(), | |
| storageLocationId: z.string().nullable(), | |
| }); | |
| export const moveItemSchema = z.object({ | |
| itemId: z.string(), | |
| storageLocationId: z.preprocess( | |
| (val) => (val === "" ? null : val), | |
| z.string().nullable(), | |
| ), | |
| }); |
🤖 Prompt for AI Agents
In `@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/schemas.ts
around lines 45 - 48, moveItemSchema currently declares storageLocationId as
z.string().nullable(), but the select sends "" for "None", which fails
validation; update the storageLocationId entry in moveItemSchema to
preprocess/transform empty string into null (e.g., use z.preprocess to convert
val === "" to null before validating with z.string().nullable()) so that the
form's "" maps to null and validation succeeds.
Comment on lines
+57
to
+66
| if (cellarItemId) { | ||
| try { | ||
| await adjustCellarItemQuantity(cellarItemId, -1); | ||
| revalidatePath( | ||
| generatePath(Routes.CELLAR, { username: user.username }), | ||
| ); | ||
| } catch { | ||
| console.error("Failed to decrement cellar item quantity"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Include the error object in the log for debugging.
The nested catch block logs a static message without the actual error, making it harder to debug failures in production.
🔧 Proposed fix
try {
await adjustCellarItemQuantity(cellarItemId, -1);
revalidatePath(
generatePath(Routes.CELLAR, { username: user.username }),
);
- } catch {
- console.error("Failed to decrement cellar item quantity");
+ } catch (error) {
+ console.error("Failed to decrement cellar item quantity", error);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (cellarItemId) { | |
| try { | |
| await adjustCellarItemQuantity(cellarItemId, -1); | |
| revalidatePath( | |
| generatePath(Routes.CELLAR, { username: user.username }), | |
| ); | |
| } catch { | |
| console.error("Failed to decrement cellar item quantity"); | |
| } | |
| } | |
| if (cellarItemId) { | |
| try { | |
| await adjustCellarItemQuantity(cellarItemId, -1); | |
| revalidatePath( | |
| generatePath(Routes.CELLAR, { username: user.username }), | |
| ); | |
| } catch (error) { | |
| console.error("Failed to decrement cellar item quantity", error); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@src/app/`[locale]/(business)/(without-header)/breweries/[brewerySlug]/beers/[beerSlug]/review/actions.ts
around lines 57 - 66, The catch block that handles failures from
adjustCellarItemQuantity(cellarItemId, -1) currently logs a static message;
update the catch to accept the thrown error (e.g., catch (err)) and include that
error in the log so you call console.error with a descriptive message plus the
error object (reference: adjustCellarItemQuantity, revalidatePath, generatePath,
Routes.CELLAR, cellarItemId) to make runtime failures debuggable.
Comment on lines
+834
to
+841
| "form": { | ||
| "servingFormat": { | ||
| "label": "Serving format", | ||
| "values": { | ||
| "BOTTLE": "Bottle", | ||
| "CAN": "Can" | ||
| } | ||
| }, |
There was a problem hiding this comment.
Serving format translations are incomplete.
Only BOTTLE and CAN are defined, but the ServingFrom enum also includes DRAFT, GROWLER, and CASK. This affects both the filter dropdown and any form that uses these translations.
🔧 Proposed fix
"servingFormat": {
"label": "Serving format",
"values": {
"BOTTLE": "Bottle",
- "CAN": "Can"
+ "CAN": "Can",
+ "DRAFT": "Draft",
+ "GROWLER": "Growler",
+ "CASK": "Cask"
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "form": { | |
| "servingFormat": { | |
| "label": "Serving format", | |
| "values": { | |
| "BOTTLE": "Bottle", | |
| "CAN": "Can" | |
| } | |
| }, | |
| "form": { | |
| "servingFormat": { | |
| "label": "Serving format", | |
| "values": { | |
| "BOTTLE": "Bottle", | |
| "CAN": "Can", | |
| "DRAFT": "Draft", | |
| "GROWLER": "Growler", | |
| "CASK": "Cask" | |
| } | |
| }, |
🤖 Prompt for AI Agents
In `@src/lib/i18n/translations/en.json` around lines 834 - 841, Translation set
for serving formats is missing entries for ServingFrom enum values DRAFT,
GROWLER, and CASK; add keys under the JSON object "form.servingFormat.values"
for "DRAFT", "GROWLER", and "CASK" with appropriate English strings (e.g.,
"Draft", "Growler", "Cask") so UI dropdowns and forms that rely on the
ServingFrom enum render correctly; ensure keys exactly match the enum symbol
names (DRAFT, GROWLER, CASK) to align with lookup code that uses those
identifiers.
Comment on lines
+866
to
+870
| "addToCellar": { | ||
| "title": "Add to Cellar", | ||
| "submit": "Add to Cellar", | ||
| "submitting": "Adding..." | ||
| } |
There was a problem hiding this comment.
Missing success translation key.
The AddToCellarModal component references cellar.addToCellar.success with a {beerName} placeholder (in toast.success), but this key is not defined in the translations.
🔧 Proposed fix
"addToCellar": {
"title": "Add to Cellar",
"submit": "Add to Cellar",
- "submitting": "Adding..."
+ "submitting": "Adding...",
+ "success": "{beerName} added to cellar"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "addToCellar": { | |
| "title": "Add to Cellar", | |
| "submit": "Add to Cellar", | |
| "submitting": "Adding..." | |
| } | |
| "addToCellar": { | |
| "title": "Add to Cellar", | |
| "submit": "Add to Cellar", | |
| "submitting": "Adding...", | |
| "success": "{beerName} added to cellar" | |
| } |
🤖 Prompt for AI Agents
In `@src/lib/i18n/translations/en.json` around lines 866 - 870, Add the missing
translation key used by AddToCellarModal: define "cellar.addToCellar.success" in
the en.json translations with a string containing the {beerName} placeholder
(e.g., a success message like "Added {beerName} to your cellar"). This ensures
the toast.success call in AddToCellarModal can resolve
cellar.addToCellar.success with the {beerName} variable.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/schemas.ts:
- Around line 31-32: The schema treats empty form strings as actual values (""
gets coerced to 0 for purchasePrice and fails length check for
purchaseCurrency); wrap each with a preprocess that converts empty string to
undefined before validation so optional fields validate as empty/not-provided:
apply a preprocess that returns undefined when the input is "" for the
purchasePrice and purchaseCurrency schema entries (referencing the purchasePrice
and purchaseCurrency symbols) and keep the existing
z.coerce.number().positive().optional() and z.string().length(3).optional()
validators as the inner schemas.
🧹 Nitpick comments (2)
src/app/_components/share-button/index.tsx (1)
55-55: Redundant size specification.
size={24}andclassName="size-6"both set the icon to 24px. Consider keeping only one to avoid potential inconsistencies if one is updated later.🔧 Suggested fix (pick one approach)
Using only the Tailwind class (preferred for consistency with the rest of the codebase):
- {children ? children : <Share2Icon size={24} className="size-6" />} + {children ? children : <Share2Icon className="size-6" />}Or using only the
sizeprop:- {children ? children : <Share2Icon size={24} className="size-6" />} + {children ? children : <Share2Icon size={24} />}prisma/migrations/20260201154542_add_cellar/down.sql (1)
4-22: Schema prefix inconsistency between up and down migrations.The up migration creates tables without a schema prefix (e.g.,
"storage_locations"), while the down migration uses explicit"public"prefix (e.g.,"public"."storage_locations"). This works if the defaultsearch_pathispublic, but the inconsistency could cause issues in environments with different schema configurations.Consider using consistent schema qualification in both files for clarity and robustness.
Comment on lines
+31
to
+32
| purchasePrice: z.coerce.number().positive().optional(), | ||
| purchaseCurrency: z.string().length(3).optional(), |
There was a problem hiding this comment.
Empty string from form inputs will fail validation unexpectedly.
With z.coerce.number(), an empty string becomes 0 (since Number("") === 0), which then fails the .positive() check. Similarly, an empty string for purchaseCurrency fails the .length(3) check. If form inputs send empty strings for unfilled optional fields, users will see validation errors for fields they intentionally left blank.
🐛 Proposed fix - preprocess empty strings to undefined
- purchasePrice: z.coerce.number().positive().optional(),
- purchaseCurrency: z.string().length(3).optional(),
+ purchasePrice: z.preprocess(
+ (val) => (val === "" ? undefined : val),
+ z.coerce.number().positive().optional(),
+ ),
+ purchaseCurrency: z.preprocess(
+ (val) => (val === "" ? undefined : val),
+ z.string().length(3).optional(),
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| purchasePrice: z.coerce.number().positive().optional(), | |
| purchaseCurrency: z.string().length(3).optional(), | |
| purchasePrice: z.preprocess( | |
| (val) => (val === "" ? undefined : val), | |
| z.coerce.number().positive().optional(), | |
| ), | |
| purchaseCurrency: z.preprocess( | |
| (val) => (val === "" ? undefined : val), | |
| z.string().length(3).optional(), | |
| ), |
🤖 Prompt for AI Agents
In `@src/app/`[locale]/(business)/(with-header)/users/[username]/cellar/schemas.ts
around lines 31 - 32, The schema treats empty form strings as actual values (""
gets coerced to 0 for purchasePrice and fails length check for
purchaseCurrency); wrap each with a preprocess that converts empty string to
undefined before validation so optional fields validate as empty/not-provided:
apply a preprocess that returns undefined when the input is "" for the
purchasePrice and purchaseCurrency schema entries (referencing the purchasePrice
and purchaseCurrency symbols) and keep the existing
z.coerce.number().positive().optional() and z.string().length(3).optional()
validators as the inner schemas.
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
2 participants
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.
Implements a comprehensive beer cellar system allowing users to:
purchase date, price, and storage location
Includes:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.