Skip to content

Jeremy#24

Open
bwreid wants to merge 5 commits into
pce-uw-jscript400:masterfrom
Jeremyjdz13:master
Open

Jeremy#24
bwreid wants to merge 5 commits into
pce-uw-jscript400:masterfrom
Jeremyjdz13:master

Conversation

@bwreid

@bwreid bwreid commented Aug 10, 2019

Copy link
Copy Markdown
Collaborator

No description provided.

@bwreid bwreid left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Overall, it's looking really good! Just a few of the routes are not quite working as expected.

Comment thread api/models/users.js
const schema = mongoose.Schema({
username: String,
password: String,
admin: false

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think you want this to be something like:

admin: {
  type: Boolean,
  default: false
}

Comment thread api/routes/auth.js
const payload = jwt.verify(token, SECRET_KEY)
const user = await Users.findOne({ _id: payload.id }).select('-__v -password')
if (user.admin != true) throw new Error(`The JWT token is for a user who is not an admin ${jwtstatus}`)
if (!user) throw new Error(`User cannot be found ${nullUserStatus}`)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You might want this error to go above the previous error. You'll always end up skipping this error, I believe, in the current case.

Comment thread api/routes/auth.js
if (!user) throw new Error(`User cannot be found ${nullUserStatus}`)

const {username, admin} = req.body
const userUpdate = await Users.findOne({username})

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Instead, you should probably be searching for the user by the req.params.id.

Comment thread api/routes/books.js
// const isAdmin = (user.admin === true)
// const adminStatus = 404
// if(!isAdmin) throw new Error(`You do not have access ${adminStatus}`)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

What was happening here that wasn't working? My guess would be the user.admin === true bit...?

Comment thread api/routes/books.js
const payload = jwt.verify(token, SECRET_KEY)
const user = await Users.findOne({ _id: payload.id }).select('-__v -password')

const book = await Book.findById(id)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Looks like this one still needs more work.

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