Conversation
| const getUserCart = (username, userStore) => { | ||
| // check if user exist in userStore | ||
| const user = userStore.find((user) => user.login.username === username); | ||
| if (!user || !user.cart) return {isFound: false, err: 500, message: errorMessages[500]} |
There was a problem hiding this comment.
Check your code alignment and spacing
| if (!user || !user.cart) return {isFound: false, err: 500, message: errorMessages[500]} | |
| if (!user || !user.cart) return { isFound: false, err: 500, message: errorMessages[500] } |
|
|
||
| const getUserCart = (username, userStore) => { | ||
| // check if user exist in userStore | ||
| const user = userStore.find((user) => user.login.username === username); |
There was a problem hiding this comment.
you dont need the brackets.
| const user = userStore.find((user) => user.login.username === username); | |
| const user = userStore.find(user => user.login.username === username); |
There was a problem hiding this comment.
This will actually crash your server if for some reason your store has no users, protect yourself with optional
| const user = userStore.find((user) => user.login.username === username); | |
| const user = userStore.find((user) => user?.login?.username === username); |
| const user = userStore.find((user) => user.login.username === username); | ||
| if (!user || !user.cart) return {isFound: false, err: 500, message: errorMessages[500]} | ||
|
|
||
| return {isFound: true, cart: user.cart}; |
|
|
||
| const user = userStore.find((user) => user.login.username === username && user.login.password === password); | ||
|
|
||
| if (!user || !username || !password) return {loginSuccess: false, err: 401, message: errorMessages[401]} |
There was a problem hiding this comment.
by this point consider using a lint rule for the spacing
| it("should GET all brands", (done) => { | ||
| sendChaiGet(server, "/api/brands", done, (err, res, done) => { | ||
| res.should.have.status(200) | ||
| res.body.should.be.an("array") |
There was a problem hiding this comment.
This is not really testing anything, if you change your actual code to return an empty array, this test wont fail and you will be introducing a severe production bug. Always test agains mocks to make sure you are returning the proper data.
| res.body[0].should.have.property("id"); | ||
| res.body[0].should.have.property("categoryId", "1"); | ||
| res.body[0].should.have.property("name"); | ||
| res.body[0].should.have.property("description"); | ||
| res.body[0].should.have.property("price"); | ||
| res.body[0].should.have.property("imageUrls"); |
There was a problem hiding this comment.
same as above, here its better but still can let pass some bugs.
| const token = req.header("token"); | ||
|
|
||
| // Validate token | ||
| const tokenValidation = validateToken(token, ACCESS_TOKENS); | ||
| if (!tokenValidation.isValid) return res.status(tokenValidation.err).send(tokenValidation.message); |
There was a problem hiding this comment.
DRY, you are repeating the same over and over in your authenticated apis.
For an idea, check step 5 in here https://medium.com/@prashantramnyc/authenticate-rest-apis-in-node-js-using-jwt-json-web-tokens-f0e97669aad3
2 participants
No description provided.