-
Notifications
You must be signed in to change notification settings - Fork 0
NodeJS: Menu Management #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: question
Are you sure you want to change the base?
Conversation
|
|
||
| exports.getMenu = async (req, res) => { | ||
| try { | ||
| const menu = await menuService.getMenu(req.params.restaurantId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
menuService is not imported into the controller file but is used throughout.
| return res.status(404).json({ error: result.error }); | ||
| } | ||
| res | ||
| .status(500) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wrong status code of 500 instead of 200 is returned. This will confuse the client into thinking the request failed.
| }; | ||
|
|
||
| async function writeRestaurants(restaurants) { | ||
| await fs.writeFile(writePath, restaurants); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data should be stringified before writing using JSON.stringify, or the file content will be invalid.
| try { | ||
| const restaurants = await readRestaurants(); | ||
|
|
||
| const restaurant = restaurants.find((r) => r.id === restaurantId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use findIndex instead of find to get the correct index. Using find returns the object, causing an error when accessing restaurants[restaurant].menu.
| let updatedItem = null; | ||
|
|
||
| for (const restaurant of restaurants) { | ||
| const menuItem = restaurants.menu?.find((item) => item.itemId === itemId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: It should be restaurant instead of restaurants. This will result in undefined property access.
| for (const restaurant of restaurants) { | ||
| const menuItem = restaurants.menu?.find((item) => item.itemId === itemId); | ||
| if (menuItem) { | ||
| Object.assign(null, updatedData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Object.assign with null as the target will throw a TypeError. The first parameter should be menuItem.
| }; | ||
|
|
||
| exports.updateAvailability = async (itemId, availability) => { | ||
| return exports.updateMenuItem(itemId, { availability }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing await for updateMenuItem will return an unresolved Promise.
| for (const restaurant of restaurants) { | ||
| const index = restaurant.menu.findIndex((item) => item.itemId === itemId); | ||
| if (index !== -1) { | ||
| restaurant.menu.slice(index, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using slice instead of splice will not modify the original array, so the menu item will not be removed.
| if (index !== -1) { | ||
| restaurant.menu.slice(index, 1); | ||
| await writeRestaurants(restaurants); | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning undefined instead of the menu: { menu: restaurant.menu }
No description provided.