Conversation
Kittease
force-pushed
the
feat/add-delete-review-action
branch
from
December 24, 2025 13:58
2fb4919 to
da87b66
Compare
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on January 5. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
Kittease
force-pushed
the
feat/add-delete-review-action
branch
from
December 26, 2025 19:14
da87b66 to
d56f1d8
Compare
Kittease
force-pushed
the
feat/add-delete-review-action
branch
from
January 31, 2026 15:22
d56f1d8 to
68b402b
Compare
📝 WalkthroughWalkthroughThis PR introduces a discriminated ActionResult for server actions, updates action functions to return structured results and localized translation keys, adds review deletion (with auth/ownership checks and image cleanup), refactors image/storage types and utilities, enhances ShareButton mobile UX, expands i18n messages/types, and updates several component typings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as DeleteReviewButton
participant Action as deleteReviewAction
participant Auth as getCurrentUser
participant DB as Prisma
participant Storage as StorageService
User->>UI: Click delete
activate UI
UI->>UI: Open confirmation dialog
User->>UI: Confirm
UI->>Action: deleteReviewAction(reviewId)
activate Action
par parallel fetch
Action->>Auth: getCurrentUser()
Action->>DB: findUnique(reviewId)
end
alt review not found
Action-->>UI: { success: false, translationKey: "reviewPage.actions.remove.errors.removeNotFound" }
else not authenticated
Action-->>UI: { success: false, translationKey: "common.errors.unauthorized" }
else not owner
Action-->>UI: { success: false, translationKey: "common.errors.forbidden" }
else authorized owner
Action->>DB: delete(reviewId)
alt delete success
par cleanup images
Action->>Storage: deleteFile(bucket, mainFile)
Action->>Storage: deleteFile(bucket, previewVariant1)
Action->>Storage: deleteFile(bucket, previewVariant2)
end
Action-->>UI: { success: true }
else delete failed
Action-->>UI: { success: false, translationKey: "reviewPage.actions.remove.errors.removeUnknown" }
end
end
deactivate Action
alt success
UI->>UI: show success toast, navigate
else failure
UI->>UI: show error toast with translationKey
end
deactivate UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate 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: 6
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 48-51: Verify whether the pinned versions of libphonenumber-js and
motion in package.json were intentionally changed: inspect git history (git log
-p package.json / git blame) for recent edits that touched these dependencies,
then if unintentional or safe to update, bump "libphonenumber-js" and "motion"
to their latest compatible releases (or at least to the latest patch/minor:
e.g., libphonenumber-js to 1.12.36 and motion to 12.29.2), run the test suite
and build, and run an npm/yarn audit and dependency tests to ensure no breakage;
if updating is unsafe, add a PR comment documenting why the older versions are
required and note any outstanding security risks so they can be tracked.
In `@src/app/_components/share-button/index.tsx`:
- Around line 57-86: The Drawer stays open because it is currently uncontrolled;
make the Drawer controlled by wiring its open state to the existing state used
by handleShare: pass open={open} and onOpenChange={setOpen} (or equivalent state
setter) to the Drawer component so setOpen(false) in handleShare actually closes
it; locate the Drawer element in the useDrawerOnMobile && isMobile branch and
add those props (ensuring the same open/setOpen state used elsewhere is imported
or in scope).
In `@src/app/`[locale]/(business)/(with-header)/friend-requests/actions.ts:
- Around line 13-16: Move the type-only import for ActionResult above the value
import: change the import order so "import type { ActionResult } from
'@/lib/action/types';" appears before "import { getCurrentUser } from
'@/lib/auth';" (keep the type import as an "import type" to satisfy ESLint
import/order).
In
`@src/app/`[locale]/(business)/(with-header)/users/[username]/_components/user-header/add-friend/actions.ts:
- Around line 3-6: ESLint import/order wants the type-only import before the
non-type imports; move the line "import type { ActionResult } from
'@/lib/action/types';" so it appears above the "import { getCurrentUser } from
'@/lib/auth';" (keep the "import { sendFriendRequest } from '@/domain/users';"
where it is), ensuring the type-only import uses "import type" and that imports
remain otherwise unchanged.
In `@src/lib/i18n/translations/fr.json`:
- Around line 665-666: Update the French translation for the delete confirmation
title by correcting the initial letter to include the uppercase accent: change
the "title" value currently set on the delete confirmation entry (the JSON key
"title" near the delete-confirmation strings) from "Etes-vous sûr de vouloir
supprimer cette critique ?" to the proper "Êtes-vous sûr de vouloir supprimer
cette critique ?", preserving the rest of the file encoding and punctuation.
In `@src/lib/images/types.ts`:
- Around line 1-9: Remove the unused Preview type declaration to eliminate dead
code: delete the "export type Preview = { name: PreviewName; image: Buffer; }"
definition from the module while keeping the PreviewName enum intact; search for
usages of "Preview" (e.g., in functions like createPreviews) to confirm none
exist and update imports/exports if any files reference that type before
committing the change.
🧹 Nitpick comments (6)
src/app/_components/share-button/index.tsx (1)
76-82: Consider adding an accessible label to the read-only input.The Input element lacks an
aria-labelattribute. For screen reader users, adding a label likearia-label={t("common.actions.shareLink")}would improve accessibility.src/lib/i18n/types.ts (1)
1-5: Fix import order per ESLint rules.The type import should come before the value import, and there should be no empty line within the import group.
🔧 Proposed fix
-import enMessages from "@/lib/i18n/translations/en.json"; - -import type { routing } from "@/lib/i18n"; +import type { routing } from "@/lib/i18n"; +import enMessages from "@/lib/i18n/translations/en.json";src/lib/storage/index.tsx (1)
3-11: Fix import order per ESLint rules.There should be no empty line within the import group.
🔧 Proposed fix
import { DeleteObjectCommand, PutObjectCommand, S3Client, } from "@aws-sdk/client-s3"; - import { config } from "@/lib/config"; + import type { StorageBuckets } from "@/lib/storage/constants";src/domain/reviews/index.ts (1)
18-42: Consider handling partial image deletion failures gracefully.If any
deleteFilecall fails,Promise.allrejects immediately and remaining deletions may not complete, leaving orphaned preview images. Consider usingPromise.allSettledto ensure all deletion attempts are made, or wrap with error handling.Additionally, if
pictureUrldoesn't match the expected bucket URL format,baseFileNamecould be incorrect.🛡️ Proposed fix using Promise.allSettled
if (deletedReview.pictureUrl) { const baseFileName = deletedReview.pictureUrl.replace( getBucketBaseUrl(StorageBuckets.REVIEW_PICTURES), "", ); - await Promise.all([ + await Promise.allSettled([ deleteFile({ bucketName: StorageBuckets.REVIEW_PICTURES, fileName: baseFileName, }), ...Object.values(PreviewName).map((previewName) => deleteFile({ bucketName: StorageBuckets.REVIEW_PICTURES, fileName: baseFileName.replace(`.jpg`, `_${previewName}.jpg`), }), ), ]); }src/app/[locale]/(business)/(without-header)/users/[username]/reviews/[reviewSlug]/_components/delete-review-dialog/actions.ts (1)
3-6: Fix import order per ESLint rules.The type import should come before value imports, and there should be no empty line within the import group.
🔧 Proposed fix
-import { deleteReview, getReviewById } from "@/domain/reviews"; -import { getCurrentUser } from "@/lib/auth"; - import type { ActionResult } from "@/lib/action/types"; +import { deleteReview, getReviewById } from "@/domain/reviews"; +import { getCurrentUser } from "@/lib/auth";src/app/[locale]/(business)/(without-header)/users/[username]/reviews/[reviewSlug]/page.tsx (1)
113-114: Consider parallelizing thegetCurrentUserfetch with the review fetch.Currently,
getCurrentUseris awaited aftergetReviewByUsernameAndSlug. These could be parallelized to reduce page load time, similar to how the server action does it.⚡ Proposed optimization
- const review = await getReviewByUsernameAndSlug(username, reviewSlug).catch( - () => notFound(), - ); + const [review, currentUser] = await Promise.all([ + getReviewByUsernameAndSlug(username, reviewSlug).catch(() => notFound()), + getCurrentUser(), + ]); const getFormattedPurchaseLocation = ( purchaseLocation: NonNullable<Review["purchaseLocation"]>, ) => { // ... existing code ... }; - const currentUser = await getCurrentUser();
Kittease
force-pushed
the
feat/add-delete-review-action
branch
from
January 31, 2026 23:16
68b402b to
0e52652
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/domain/reviews/index.ts`:
- Around line 18-42: The deleteReview function currently deletes the DB row then
calls deleteFile for the picture and previews, allowing S3 errors to bubble up
and make deleteReviewAction report failure; wrap the S3 cleanup in a try-catch
inside deleteReview so that any errors from deleteFile (and the Promise.all) are
caught and logged (use your logger) but not rethrown, ensuring the DB deletion
remains the overall success; keep references to StorageBuckets.REVIEW_PICTURES,
getBucketBaseUrl, PreviewName, and deleteFile when locating the code to modify.
- Line 37: The cleanup currently hardcodes `.jpg` when building preview
filenames (fileName: baseFileName.replace(`.jpg`, `_${previewName}.jpg`)), which
will break if formats change; update the logic to derive the extension from the
stored filename (e.g., use path.extname/baseName via path.parse on baseFileName)
and construct the preview name as `${name}_${previewName}${ext}` (or otherwise
extract and reuse the actual extension), replacing the hardcoded `.jpg` in the
cleanup code; reference optimizeImage(), previewName and baseFileName when
making the change so previews are deleted correctly regardless of image format.
In `@src/lib/i18n/types.ts`:
- Around line 1-4: Move the type-only import for "routing" before the value
import "enMessages" and remove the empty line between them so imports are
grouped correctly; specifically change the import order to import type { routing
} from "@/lib/i18n"; followed immediately by import enMessages from
"@/lib/i18n/translations/en.json"; ensuring the type-only import appears first
and there are no blank lines in that import group.
In `@src/lib/images/index.ts`:
- Around line 51-56: Reusing the single sharp instance (sharpInstance) for
concurrent resizes causes pipeline mutation so both outputs end up with the same
dimensions; fix by creating independent pipelines via sharpInstance.clone()
before each resize call (e.g., use sharpInstance.clone().resize(...) for
previewImage and sharpInstance.clone().resize(...) for twitterImage) so
previewImage and twitterImage are produced from separate pipelines.
🧹 Nitpick comments (2)
src/lib/storage/index.tsx (1)
9-9: Fix import ordering per ESLint.Static analysis indicates there should be no empty line within the import group.
🧹 Proposed fix
import { DeleteObjectCommand, PutObjectCommand, S3Client, } from "@aws-sdk/client-s3"; - import { config } from "@/lib/config";src/app/[locale]/(business)/(without-header)/users/[username]/reviews/[reviewSlug]/_components/delete-review-dialog/actions.ts (1)
3-6: Fix import ordering per ESLint.Static analysis indicates:
- No empty line within import group (Line 4)
- Type import should occur before regular imports
🧹 Proposed fix
"use server"; +import type { ActionResult } from "@/lib/action/types"; import { deleteReview, getReviewById } from "@/domain/reviews"; import { getCurrentUser } from "@/lib/auth"; - -import type { ActionResult } from "@/lib/action/types";
| ...Object.values(PreviewName).map((previewName) => | ||
| deleteFile({ | ||
| bucketName: StorageBuckets.REVIEW_PICTURES, | ||
| fileName: baseFileName.replace(`.jpg`, `_${previewName}.jpg`), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/domain/reviews/index.ts | head -50Repository: Zythogora/zythogora.com
Length of output: 2026
🏁 Script executed:
rg -n "pictureUrl|contentType|fileExtension|\.jpg|\.png|\.webp" --type ts -g '!node_modules' -C2 | head -100Repository: Zythogora/zythogora.com
Length of output: 5220
🏁 Script executed:
rg -n "picture|image|extension" --type ts -g '!node_modules' | grep -i "format\|extension\|type" | head -40Repository: Zythogora/zythogora.com
Length of output: 31047
🏁 Script executed:
rg -n "createPreviews|optimizedImage|compress|convert|webp|png" --type ts -g '!node_modules' -C2 | head -80Repository: Zythogora/zythogora.com
Length of output: 4484
🏁 Script executed:
fd -name "*.ts" -path "*image*" -o -name "*image*.ts" | head -20Repository: Zythogora/zythogora.com
Length of output: 297
🏁 Script executed:
cat -n src/lib/images/index.ts 2>/dev/null || cat -n src/domain/beers/index.ts | sed -n '320,380p'Repository: Zythogora/zythogora.com
Length of output: 2182
Format assumption is currently enforced by upload logic, but cleanup code hardcodes extensions.
All review images are converted to JPEG via optimizeImage() (src/lib/images/index.ts:27), and preview variants preserve JPEG format. However, the cleanup code in line 37 hardcodes .jpg extension, which creates a maintainability risk: if the image format changes in the future (e.g., to WebP or PNG), the cleanup logic would silently fail to delete preview variants. Consider extracting the extension from the stored filename or explicitly documenting this dependency.
🤖 Prompt for AI Agents
In `@src/domain/reviews/index.ts` at line 37, The cleanup currently hardcodes
`.jpg` when building preview filenames (fileName: baseFileName.replace(`.jpg`,
`_${previewName}.jpg`)), which will break if formats change; update the logic to
derive the extension from the stored filename (e.g., use path.extname/baseName
via path.parse on baseFileName) and construct the preview name as
`${name}_${previewName}${ext}` (or otherwise extract and reuse the actual
extension), replacing the hardcoded `.jpg` in the cleanup code; reference
optimizeImage(), previewName and baseFileName when making the change so previews
are deleted correctly regardless of image format.
Comment on lines
51
to
56
| const sharpInstance = sharp(imageBuffer); | ||
|
|
||
| const [previewImage, twitterImage] = await Promise.all([ | ||
| sharpInstance.resize({ width: 1200, height: 630 }).toBuffer(), | ||
| sharpInstance.resize({ width: 1200, height: 675 }).toBuffer(), | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
sharp clone method concurrent resizes pipeline mutation
💡 Result:
In Node.js sharp, a Sharp instance is a mutable processing pipeline. If you reuse the same instance and keep calling transforms (e.g., multiple .resize(...) calls), you’re mutating that one pipeline—so later operations can override/ignore earlier ones.
Key points:
- Use
.clone()to take a snapshot of the current pipeline so you can run multiple independent pipelines (e.g., concurrent resizes) that share the same input stream/buffer. The docs explicitly recommend cloning to create multiple output streams / processing pipelines from one input. (sharp.pixelplumbing.com) - Only one
resizecan occur per pipeline; previous calls toresizein the same pipeline are ignored. If you “reuse” a pipeline for different sizes, you can easily end up with the wrong output because the pipeline state has been mutated. (sharp.pixelplumbing.com)
Typical correct pattern for concurrent resizes:
const pipeline = sharp().rotate(); // shared “base” pipeline
readableStream.pipe(pipeline);
pipeline.clone().resize(800, 600).pipe(out1);
pipeline.clone().resize(200, 200).pipe(out2);This is the intended solution for “use the same base sharp instance for multiple transformation chains” (the feature that led to clone). (github.com)
Citations:
- 1: https://sharp.pixelplumbing.com/api-constructor?utm_source=openai
- 2: https://sharp.pixelplumbing.com/api-resize?utm_source=openai
- 3: Use the same base sharp instance for multiple transformation chains lovell/sharp#235
🏁 Script executed:
# Look for any sharp usage in the codebase to understand current patterns
fd -e ts -e js --type f | xargs grep -l "sharp" | head -20Repository: Zythogora/zythogora.com
Length of output: 90
🏁 Script executed:
cat -n src/lib/images/index.ts | sed -n '40,70p'Repository: Zythogora/zythogora.com
Length of output: 943
Use clone() for independent sharp pipelines.
Reusing a single sharp instance for concurrent resizes mutates the pipeline state, causing both operations to use the same dimensions. The sharp API requires clone() to create independent pipeline snapshots for concurrent processing.
🛠️ Suggested fix
const sharpInstance = sharp(imageBuffer);
const [previewImage, twitterImage] = await Promise.all([
- sharpInstance.resize({ width: 1200, height: 630 }).toBuffer(),
- sharpInstance.resize({ width: 1200, height: 675 }).toBuffer(),
+ sharpInstance.clone().resize({ width: 1200, height: 630 }).toBuffer(),
+ sharpInstance.clone().resize({ width: 1200, height: 675 }).toBuffer(),
]);🤖 Prompt for AI Agents
In `@src/lib/images/index.ts` around lines 51 - 56, Reusing the single sharp
instance (sharpInstance) for concurrent resizes causes pipeline mutation so both
outputs end up with the same dimensions; fix by creating independent pipelines
via sharpInstance.clone() before each resize call (e.g., use
sharpInstance.clone().resize(...) for previewImage and
sharpInstance.clone().resize(...) for twitterImage) so previewImage and
twitterImage are produced from separate pipelines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.