Skip to content

Sunglasses.io - Mark Smyth#183

Open
XTruckDriver wants to merge 44 commits intoprojectshft:masterfrom
XTruckDriver:master
Open

Sunglasses.io - Mark Smyth#183
XTruckDriver wants to merge 44 commits intoprojectshft:masterfrom
XTruckDriver:master

Conversation

@XTruckDriver
Copy link

No description provided.

summary: "all brands offered in the store"
produces:
- application/json
responses:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no example of what it should return:

items:
              $ref: '#/definitions/Brands'

const brandId = req.params.id;
const targetBrand = brands.filter(brand => brand.id === brandId);
if (targetBrand.length === 0) {
return res.status(404).json({ error: 'Invalid brand ID' });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this message is too technical, imagine you get this as web page user. Something better would be:

Suggested change
return res.status(404).json({ error: 'Invalid brand ID' });
return res.status(404).json({ error: 'Brand not found' });

const brandName = targetBrand[0].name;
const brandProducts = products.filter(product => product.categoryId === brandId)
if (brandProducts.length === 0) {
return res.status(422).json({ error: `No products found for ${brandName}` });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong code, also, its an error that there are not product? you can just return an empty array, but its not really an error.
Check documentation for error 422
422 Unprocessable Content

const { username, password } = req.body;
const user = users.find(user => user.login.username === username && user.login.password === password);
if (!user) {
return res.status(401).json({ error: 'Invalid login info' });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the login is incorrect, its 403 not 401. Check documentation - there's a subtle difference between the 2

});

app.get('/me/cart', verifyToken, (req, res) => {
const currentUser = users.find(user => user.login.username === req.username);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you checking this here? Shouldn't be part of your verifyToken?

const productId = req.params.productId;
const product = products.find(product => product.id === productId);
if (!product) {
return res.status(404).json({ error: 'Invalid Product ID' });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is 400, not 404


const existingItemIndex = user.cart.findIndex(item => item.productId === productId);
if (existingItemIndex < 0) {
return res.status(404).json({ error: 'Product not in cart' });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is 400, not 404

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants