Skip to content

Abdul kader w3 prep node #23

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

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

Conversation

Abdulkaderr
Copy link

No description provided.

const response = await request.post('/weather').send({});
expect(response.status).toBe(400);
expect(response.body).toEqual({
weatherText: 'City name is required!'

Choose a reason for hiding this comment

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

Fix the match in API response:

Update it to:

error: 'City name is required!'

Choose a reason for hiding this comment

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

exposing an API key in your source code is a security risk 🚨

Anyone can steal and misuse it.

The best practice is to hide the API key.

npm install dotenv

Move API key to a .env file (create this in your project root):

and write in it:

API_KEY=afc0f7157bda937505e237c68802afa5

then you can import the dotenv to securely import that api key. Something like this below.

import dotenv from 'dotenv';
dotenv.config();

export const keys = {
API_KEY: process.env.API_KEY
};

Choose a reason for hiding this comment

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

There is a little error in throwing an error in the catch block:

Instead of throw new Error(...), you should console.error(...).
Throwing an error in the catch block will cause an unhandled rejection if not caught elsewhere.

Choose a reason for hiding this comment

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

Missing express.json() Middleware

You are handling POST requests (/register, /login, /logout), which typically require a request body.
Without app.use(express.json());, req.body will be undefined.

Choose a reason for hiding this comment

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

  1. Instead of storing usersDatabase as a constant, call database.getUsers() every time in getUserByUsername() and getUserById().

  2. You are using bcrypt, but the correct package is bcryptjs if you are using ESM imports (import ... from "bcrypt").
    The bcrypt package requires import * as bcrypt from "bcrypt"; in ESM or const bcrypt = require("bcrypt") in CommonJS.

Copy link
Author

Choose a reason for hiding this comment

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

Hello, can you please delete my branch? because I uploaded by mistake here its prep exer and not assignment

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