Skip to content

Shahab-homework#23

Open
Shahab13 wants to merge 1 commit into
pce-uw-jscript400:masterfrom
Shahab13:master
Open

Shahab-homework#23
Shahab13 wants to merge 1 commit into
pce-uw-jscript400:masterfrom
Shahab13:master

Conversation

@Shahab13

Copy link
Copy Markdown

No description provided.

@bwreid bwreid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

On the whole this looks good. Just a few things left to do!

Comment thread api/routes/books.js
const response = await Book.findById(book._id).select('-__v')
const status = 200
res.json({ status, response })
book.reserved.memberId = req.userData;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wont' this assign the memberId to be the entire object that is userData?

Comment thread api/routes/user.js
});
}
///
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks good but this function is quite long!

Comment thread api/routes/user.js
if (order) {
if (!order.admin) {
return res.status(401).json({
message: "The JWT token is for a user who is not an admin"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is just checking whether or not the User you just found is an admin, not that the JWT Token is for an admin.

Comment thread api/routes/books.js

console.log(message);
res.status(status).json({ status, message });
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This route yet to be implemented.

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