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

Feedback #1

Open
wants to merge 8 commits into
base: feedback
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
# news-aggregator-api-BurningMoltres/.gitignore
node_modules
.tap
.tap
.env
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<!-- news-aggregator-api-BurningMoltres/README.md -->
[![Open in Visual Studio Code](https://classroom.github.com/assets/open-in-vscode-2e0aaae1b6195c2367325f4f02e2d04e9abb55f0b24a779b69b11b9e10269abc.svg)](https://classroom.github.com/online_ide?assignment_repo_id=17675965&assignment_repo_type=AssignmentRepo)
26 changes: 20 additions & 6 deletions app.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,31 @@
// news-aggregator-api-BurningMoltres/app.js

Choose a reason for hiding this comment

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

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.

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

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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

const UserLogin=require('./routes/UserLogin');

Choose a reason for hiding this comment

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

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

const UserPreferences=require('./routes/UserPreferences');

Choose a reason for hiding this comment

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

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

require('dotenv').config()

Choose a reason for hiding this comment

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

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.


app.use(express.json());
app.use(express.urlencoded({ extended: true }));

app.listen(port, (err) => {
if (err) {
return console.log('Something bad happened', err);
}
console.log(`Server is listening on ${port}`);
});
//verify jwttoken

Choose a reason for hiding this comment

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

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.


mongoose.connect(process.env.MONGO_URL).then(()=>{
console.log("Connected To MongoDb");
app.listen(port, (err) => {
if (err) {
return console.log('Something bad happened', err);
}
console.log(`Server is listening on ${port}`);
});
})

Choose a reason for hiding this comment

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

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


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


module.exports = app;

Choose a reason for hiding this comment

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

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

31 changes: 31 additions & 0 deletions models/user.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// news-aggregator-api-BurningMoltres/models/user.js
const mongoose=require('mongoose');
const courseSchema =new mongoose.Schema({

Choose a reason for hiding this comment

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

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

username:{
type:String,
required:true,
minLength:5,
maxLength:255,
},
password:{
type:String,
required:true,
minLength:5,
maxLength:255
},
email:{
type:String,
required:true,
minLength:5,
maxLength:255
},
preferences:{
type:[],

Choose a reason for hiding this comment

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

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.

required:true,
minLength:2,

Choose a reason for hiding this comment

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

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

maxLength:255
}
})

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

Choose a reason for hiding this comment

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

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.

Loading