Feedback #1

github-classroom · 2025-01-11T21:16:21Z

👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.
In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
Click the Files changed or Commits tab to see all of the changes pushed to the default branch since the assignment started. Your teacher can see this too.

Notes for teachers

Use this PR to leave feedback. Here are some tips:

Click the Files changed tab to see all of the changes pushed to the default branch since the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.
Click the Commits tab to see the commits pushed to the default branch. Click a commit to see specific changes.
If you turned on autograding, then click the Checks tab to see the results.
This page is an overview. It shows commits, line comments, and general comments. You can leave a general comment below.
For more information about this pull request, read “Leaving assignment feedback in GitHub”.

Subscribed: @BurningMoltres

airtribe-bot · 2025-01-12T17:56:56Z

app.js

@@ -1,17 +1,31 @@
+// news-aggregator-api-BurningMoltres/app.js


It's generally a good practice to avoid leaving commented-out code (like the comment at line 1) in your committed files, unless it's necessary for documentation or temporary use. Consider removing it if it's not needed.

airtribe-bot · 2025-01-12T17:56:56Z

app.js

 const express = require('express');
+const mongoose=require('mongoose');


Ensure consistent spacing around the assignment operator for readability. Suggest changing const mongoose=require('mongoose'); to const mongoose = require('mongoose');.

airtribe-bot · 2025-01-12T17:56:56Z

app.js

 const app = express();
 const port = 3000;
+const UserRegistration=require('./routes/UserRegistration');


Ensure consistent spacing around the assignment operator for readability. Suggest changing const UserRegistration=require('./routes/UserRegistration'); to const UserRegistration = require('./routes/UserRegistration');.

airtribe-bot · 2025-01-12T17:56:56Z

app.js

 const app = express();
 const port = 3000;
+const UserRegistration=require('./routes/UserRegistration');
+const UserLogin=require('./routes/UserLogin');


Ensure consistent spacing around the assignment operator for readability. Suggest changing const UserLogin=require('./routes/UserLogin'); to const UserLogin = require('./routes/UserLogin');.

airtribe-bot · 2025-01-12T17:56:56Z

app.js

 const app = express();
 const port = 3000;
+const UserRegistration=require('./routes/UserRegistration');
+const UserLogin=require('./routes/UserLogin');
+const UserPreferences=require('./routes/UserPreferences');


Ensure consistent spacing around the assignment operator for readability. Suggest changing const UserPreferences=require('./routes/UserPreferences'); to const UserPreferences = require('./routes/UserPreferences');.

airtribe-bot · 2025-01-12T17:56:56Z

app.js

+const UserRegistration=require('./routes/UserRegistration');
+const UserLogin=require('./routes/UserLogin');
+const UserPreferences=require('./routes/UserPreferences');
+require('dotenv').config()


It's a good practice to handle possible errors when configuring the dotenv package, though not strictly necessary, it might improve robustness if it is ensured that the environment is properly loaded.

airtribe-bot · 2025-01-12T17:56:56Z

app.js

-    }
-    console.log(`Server is listening on ${port}`);
-});
+//verify jwttoken


The comment //verify jwttoken can be misleading or confusing without implementation. Consider adding the actual middleware or logic for JWT token verification or remove this comment to keep the code clean.

airtribe-bot · 2025-01-12T17:56:56Z

app.js

+        }
+        console.log(`Server is listening on ${port}`);
+    });
+})


Consider handling errors that may occur while connecting to MongoDB to ensure any connection issues are logged or managed appropriately.

airtribe-bot · 2025-01-12T17:56:57Z

app.js

+
+app.use('/registerUser',UserRegistration);
+app.use('/userLogin',UserLogin);
+app.use('/preferences',UserPreferences);


 module.exports = app;


Add a newline at the end of the file to adhere to POSIX standards and improve compatibility with various tools.

airtribe-bot · 2025-01-12T17:56:59Z

models/user.js

@@ -0,0 +1,31 @@
+// news-aggregator-api-BurningMoltres/models/user.js
+const mongoose=require('mongoose');
+const courseSchema =new mongoose.Schema({


The variable courseSchema should be renamed to userSchema to accurately reflect its purpose of defining a user schema rather than a course schema.

airtribe-bot · 2025-01-12T17:56:59Z

models/user.js

+        maxLength:255
+    },
+    preferences:{
+        type:[],


The type for preferences should be an array of a specific type, such as Array with an item schema to enforce type consistency. Consider changing it to an array with a defined type like type: [String] instead of an empty array.

airtribe-bot · 2025-01-12T17:56:59Z

models/user.js

+    preferences:{
+        type:[],
+        required:true,
+        minLength:2,


The minLength and maxLength validation properties are not applicable to arrays in Mongoose. Consider using minItems and maxItems for arrays instead.

airtribe-bot · 2025-01-12T17:56:59Z

models/user.js

+})
+
+const User=mongoose.model("User",courseSchema);
+module.exports=User;


Ensure there is a newline at the end of the file to follow the POSIX standard for text files. This can help avoid potential issues with version control systems or text processing tools.

airtribe-bot · 2025-01-12T17:57:01Z

routes/UserLogin.js

+const jwt=require("jsonwebtoken");
+
+router.post("/", async (req, res, next) => {
+  console.log(req.body);


Using console.log for debugging in production code is not advisable. Consider using a logging library to handle different logging levels, such as winston or morgan, that can be configured for different environments.

airtribe-bot · 2025-01-12T17:57:02Z

routes/UserLogin.js

+    typeof password !== "string" ||
+    password.length === 0 ||
+    typeof email !== "string" ||
+    username.length === 0 


Duplicate condition check for typeof email !== "string" || username.length === 0. The check username.length === 0 is repeated here, it should likely be email.length === 0 to validate the email input correctly.

airtribe-bot · 2025-01-12T17:57:02Z

routes/UserLogin.js

+
+if(!isPasswordValid)
+{
+    return res.status(400).json({message:'Invalid email or password'});


The error message states 'Invalid email or password', but the logic currently only checks username and password. Consider updating the logic to validate the email as part of the login process or update the error message to reflect the actual validation checks.

airtribe-bot · 2025-01-12T17:57:02Z

routes/UserLogin.js

+        email:userCredentials.email},
+        process.env.JWT_SECRET
+    );
+    res.send({token})


The response should have a consistent structure. Consider wrapping the token in an object properly labeled, for instance: res.json({ token: token }) to maintain consistency in JSON responses and improve readability.

airtribe-bot · 2025-01-12T17:57:04Z

routes/UserPreferences.js

+//middleware function to verify jwt
+const verifyJWT = (req, res, next) => {
+  const token = req.headers["authorization"];
+  console.log(token);


Using console.log for logging can expose sensitive information like tokens. It's better to use a logging library with appropriate log levels and ensure that sensitive data is not logged in production environments.

airtribe-bot · 2025-01-12T17:57:04Z

routes/UserPreferences.js

+    req.email = decodedToken;
+    return next();
+  }
+  next();


If no token is provided, the next() function is called without setting req.email. Consider sending an error response if the token is missing or invalid, to ensure proper error handling.

airtribe-bot · 2025-01-12T17:57:04Z

routes/UserPreferences.js

+  const token = req.headers["authorization"];
+  console.log(token);
+  if (token) {
+    const decodedToken = jwt.verify(token, process.env.JWT_SECRET);


The jwt.verify method should be wrapped in a try-catch block to handle cases where the token might be invalid or expired, preventing the server from crashing.

airtribe-bot · 2025-01-12T17:57:04Z

routes/UserPreferences.js

+  try {
+    let email = req.email.email;
+    const userId = await User.findOne({ email });
+    if (userId.preferences.length <= 0) {


The method userId.preferences.length <= 0 can be simplified to userId.preferences.length === 0 for better clarity.

airtribe-bot · 2025-01-12T17:57:04Z

routes/UserPreferences.js

+      return res.status(404).json("Preferences data not available");
+    }
+
+    res.status(400).json(userId.preferences);


The HTTP status code here should be 200 (OK) instead of 400, which indicates a bad request, as it's returning user preferences successfully.

airtribe-bot · 2025-01-12T17:57:04Z

routes/UserPreferences.js

+        let updatedPreferences=req.body.preferences;
+        console.log(typeof updatedPreferences);
+        const userId = await User.findOne({ email });
+        if (userId.preferences.length <= 0 || updatedPreferences.length < 0 ) {


The condition updatedPreferences.length < 0 should be updatedPreferences.length === 0 or updatedPreferences.length <= 0, as a length can never be negative.

airtribe-bot · 2025-01-12T17:57:04Z

routes/UserPreferences.js

+        }
+         await User.findOneAndUpdate({email:email},{preferences:updatedPreferences},{new:true}) 
+        console.log(updatedPreferences);
+        res.status(400).json("New Preferences Updated");


The HTTP status code here should be 200 (OK) instead of 400, which indicates a bad request, as it's confirming the update operation successfully.

airtribe-bot · 2025-01-12T17:57:07Z

routes/UserRegistration.js

+
+
+router.post('/',async (req,res,next)=>{
+console.log(req.body);


Consider removing or adjusting the console.log statement in line 10 to avoid exposing sensitive information in production environments.

airtribe-bot · 2025-01-12T17:57:07Z

routes/UserRegistration.js

+  // Create a new user
+  const newUser = new User({ username, password: hashedPassword,email,preferences });
+
+   await User.create(newUser);


Consider using newUser.save() instead of User.create(newUser) as it may be more intuitive when working with mongoose schema instances.

airtribe-bot · 2025-01-12T17:57:07Z

routes/UserRegistration.js

+});
+
+
+


Ensure consistent spacing between sections of the code for better readability. There appears to be excessive blank lines here.

airtribe-bot · 2025-01-12T17:57:07Z

routes/UserRegistration.js

+
+
+
+module.exports=router;


Add a newline at the end of the file to follow POSIX standards. This can help prevent issues with version control and concatenation of files.

github-classroom bot and others added 4 commits January 11, 2025 21:16

Setting up GitHub Classroom Feedback

aa2719f

add online IDE url

9ca6083

/register,/login and token generation

24080b8

preferences api route and jwt verfication function completed

f35043a

airtribe-bot reviewed Jan 12, 2025

View reviewed changes

BurningMoltres added 4 commits January 13, 2025 03:35

fetchUsers with PromiseAll setup

85e18dc

api speed

7f20285

regex for email and schema checks

ea3a187

removal of unwanted consoles

5adc471

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feedback #1

Feedback #1

github-classroom bot commented Jan 11, 2025 •

edited

Loading

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

airtribe-bot Jan 12, 2025

		@@ -1,17 +1,31 @@
		// news-aggregator-api-BurningMoltres/app.js

		const express = require('express');
		const mongoose=require('mongoose');



		router.post('/',async (req,res,next)=>{
		console.log(req.body);




		module.exports=router;

Feedback #1

Are you sure you want to change the base?

Feedback #1

Conversation

github-classroom bot commented Jan 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-classroom bot commented Jan 11, 2025 •

edited

Loading