Skip to content
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

Week8 #9

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Week8 #9

wants to merge 2 commits into from

Conversation

Maarimar
Copy link
Owner

No description provided.

Mariana Osorio Lozano added 2 commits May 17, 2023 15:13
Copy link

@jdevries3133 jdevries3133 left a comment

Choose a reason for hiding this comment

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

Look fantastic overall! I do think that the login request handler looks unfinished. Please follow up with me on slack or here on github so that we can get this PR merged!

Comment on lines +9 to +18
try {
await connectDB(process.env.MONGO_URI)
await Product.deleteMany()
await Product.create(jsonProducts)
console.log('Success!!!!')
process.exit(0)
} catch (error) {
console.log(error)
process.exit(1)
}

Choose a reason for hiding this comment

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

How would the program be different if this try / catch was not here?

Comment on lines +9 to +18
}
console.log(username, !password)
const id = new Date().getDate()
const token = jwt.sign({id, usernamen}, process.env.JWT_SECRET,{expiresIn: '30d'} )

res.send('Fake login/register/signup Route')
};

const dashboard = async (req,res)=>{
const luckyNumber = Math.floor(Mathe.random()*100)

Choose a reason for hiding this comment

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

Suggested change
}
console.log(username, !password)
const id = new Date().getDate()
const token = jwt.sign({id, usernamen}, process.env.JWT_SECRET,{expiresIn: '30d'} )
res.send('Fake login/register/signup Route')
};
const dashboard = async (req,res)=>{
const luckyNumber = Math.floor(Mathe.random()*100)
}
console.log(username, !password)
const id = new Date().getDate()
const token = jwt.sign({id, usernamen}, process.env.JWT_SECRET,{expiresIn: '30d'} )
res.send('Fake login/register/signup Route')
};
const dashboard = async (req,res)=>{
const luckyNumber = Math.floor(Math.random()*100)

I spy a typo!

Make sure you are testing your code always!!!!!!!!!!


const dashboard = async (req,res)=>{
const luckyNumber = Math.floor(Mathe.random()*100)
res.status(200).json({msg: `Hello, Mariana Osorio`, secret:`Here is your authorazied data, your lucky number is ${luckyNumber}`})

Choose a reason for hiding this comment

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

Suggested change
res.status(200).json({msg: `Hello, Mariana Osorio`, secret:`Here is your authorazied data, your lucky number is ${luckyNumber}`})
res.status(200).json({msg: `Hello, Mariana Osorio`, secret:`Here is your authorized data, your lucky number is ${luckyNumber}`})

nit: "authorized"

const id = new Date().getDate()
const token = jwt.sign({id, usernamen}, process.env.JWT_SECRET,{expiresIn: '30d'} )

res.send('Fake login/register/signup Route')

Choose a reason for hiding this comment

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

Are you having trouble at this point? Let me know if you have any questions – I think this assignment looks not totally complete because of this, but let me know if I'm missing something. This is the main reason I'm going to submit with status, "request changes," this week. Can we discuss if you're stuck here, and I can help you if that's the case?

Also, feel free to message me on slack, I'll be much quicker to respond!

const authenticationMiddleware = async (req, res, next) => {
const authHeader = req.headers.authorization

if (!authHeader || !authHeader.startsWith('Bearer ')) {

Choose a reason for hiding this comment

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

Fun fact, just like in math class where you can factor values our of an expression by division;

4x + 6y -> (2x + 3y) * 2

You can always factor double-negatives out of boolean expressions. In this case, the simplified version is in my suggestion

Suggested change
if (!authHeader || !authHeader.startsWith('Bearer ')) {
if (!(authHeader && authHeader.startsWith('Bearer '))) {

Choose a reason for hiding this comment

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

Sometimes this refactor can make the expression easier to read. For example, in the second version, my mind very easily translates that into, "if there is not an auth header and the auth header starts with bearer." Which, more simply, is, "if there's not an auth header starting with Bearer ."

Anytime we're bringing the code closer to our own internal thoughts, we're making it more readable, which is great!

Copy link

@jdevries3133 jdevries3133 left a comment

Choose a reason for hiding this comment

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

Look fantastic overall! I do think that the login request handler looks unfinished. Please follow up with me on slack or here on github so that we can get this PR merged!

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