Project 3 Review Pull _-AKK-_

app/categories.js

@@ -1,4 +1,4 @@
-const categories = [
+const categories = [ // a duplicate file of the same name, fix your pathing instead
     'General Knowledge',
     'Entertainment: Books',
     'Entertainment: Film',

app/models/categories.js

@@ -1,4 +1,4 @@
-const categories = [
+const categories = [ // love that you have this exported as it's own bit, but wish you had comments explaining what this is / why
     'General Knowledge',
     'Entertainment: Books',
     'Entertainment: Film',

app/models/example.js

@@ -1,5 +1,5 @@
 const mongoose = require('mongoose')
-
+// remove unused models 
 const exampleSchema = new mongoose.Schema(
 	{
 		title: {

app/models/question.js

@@ -22,7 +22,7 @@ const questionSchema = new mongoose.Schema(
         category: {
             type: String,
             required: true,
-            enum: categories
+            enum: categories // YAS
         },
         type: {
             type: String,
@@ -42,7 +42,7 @@ const questionSchema = new mongoose.Schema(
 )
 
 module.exports =  questionSchema
-
+// love seeing these comments/ notes to self / thinking out loud 
 // leaderboard might be subdocument
 // virtuals could aggregate scores for leaderboard
 // timer will be virtual using new Date()

app/models/user.js

@@ -42,9 +42,9 @@ const userSchema = new mongoose.Schema(
 )
 
 userSchema.virtual('username').get(function() {
-	return this.email.slice(0, this.email.indexOf('@'))
+	return this.email.slice(0, this.email.indexOf('@'))// nice!
 })
-
+// remove zombie comment ( love that you pasted in sample data )
 // example of playerStats
 // [
 // 	{
@@ -59,9 +59,9 @@ userSchema.virtual('username').get(function() {
 
 userSchema.virtual('scoreTotal').get(function() {
 	if (this.playerStats.length !== 0) {
-		return this.playerStats.reduce((total, field) => {
+		return this.playerStats.reduce((total, field) => { // very nice !
 			return total + field['score']
-		}, 0)
+		}, 0)// careful of our indentation here - we are using an implied '()' for the return, but you did so consistently which i like ! 
 	}
 	else {
 		return 0

app/routes/game_routes.js

@@ -34,12 +34,12 @@ const router = express.Router()
 // /games
 router.get('/games',  (req, res, next) => {
     Game.find()
-
+        // inconsistent white space here
         .populate('owner')
-        .then((games) => {
+        .then((games) => { // why not use short hand with implicit return like below ? choose 1 and be consistent 
             return games.map((game) => game.toObject())
         })
-        .then((games) => res.status(200).json({games: games}))
+        .then((games) => res.status(200).json({games: games})) // why not use long hand with explicit return here like above ?
         .catch(next)
 })
 
@@ -52,7 +52,7 @@ router.get('/games/:id',  (req, res, next)=> {
     Game.findById(req.params.id)
         .populate('owner')
         .then(game => {
-
+            // inconsistent white space here
             res.status(200).json({game: game})
         })
         .then(handle404)
@@ -66,7 +66,7 @@ router.post('/games', requireToken, (req, res, next) => {
     req.body.game.owner = req.user.id
     // setting game owner to current user
     // req.body.games.owner = req.user.id
-    console.log("the game",req.body)
+    console.log("the game",req.body) // comment out logs 
     Game.create(req.body.game)
         .then((game) => {
             res.status(201).json({ game: game})
@@ -79,7 +79,7 @@ router.post('/games', requireToken, (req, res, next) => {
 /////////// maybe remove removeBlanks and handle on client side?
 router.patch('/games/:id', removeBlanks, (req, res, next) => {
     delete req.body.game.owner
-
+ // inconsistent white space here
     Game.findById(req.params.id)
         .then(handle404)
         .then((game) => {
@@ -97,7 +97,7 @@ router.delete('/games/:id', requireToken, (req, res, next) => {
     Game.findById(req.params.id)
         .then(handle404)
         .then((game) => {
-
+            // inconsistent white space here
             requireOwnership(req, game)
             game.deleteOne()
         })

app/routes/question_routes.js

@@ -50,13 +50,13 @@ router.post('/questions/:gameId', requireToken, (req, res, next) => {
 /// /games/<game_id>/<question_id>
 router.patch('/questions/:gameId/:questionId', (req, res, next) => {
     const { gameId, questionId } = req.params
-
+    // inconsistent white space here
     Game.findById(gameId)
         .then(handle404)
         .then((game) => {
             // console.log(game.questions)
             const theQuestion = game.questions.id(questionId)
-
+            // inconsistent white space here
 
             theQuestion.set(req.body.question)
             return game.save()
@@ -79,7 +79,7 @@ router.delete('/questions/:gameId/:questionId',  (req, res, next) => {
             theQuestion.remove()
             return game.save()
         })
-        .then(game => res.sendStatus(204))
+        .then(game => res.sendStatus(204)) // unused variable
         .catch(next)
 })
 

app/routes/user_routes.js

@@ -16,7 +16,7 @@ const BadParamsError = errors.BadParamsError
 const BadCredentialsError = errors.BadCredentialsError
 
 const User = require('../models/user')
-const user = require('../models/user')
+const user = require('../models/user') // < duplicate import bad, declaring our model Class variable as lower case is against BP
 
 // passing this as a second argument to `router.<verb>` will make it
 // so that a token MUST be passed for that route to be available
@@ -70,7 +70,7 @@ router.post('/sign-up', (req, res, next) => {
 router.post('/sign-in', (req, res, next) => {
 	const pw = req.body.credentials.password
 	let user
-
+// inconsistent white space here
 	// find a user based on the email that was passed
 	User.findOne({ email: req.body.credentials.email })
 		.then((record) => {

package.json

@@ -1,5 +1,5 @@
 {
-  "name": "express-auth-mongoose-boilerplate",
+  "name": "express-auth-mongoose-boilerplate", 
   "version": "1.0.0",
   "description": "Auth boilerplate for mongoose express using token auth",
   "main": "server.js",

server.js

@@ -68,7 +68,7 @@ app.use(express.urlencoded({ extended: true }))
 app.use(requestLogger)
 
 // register route files
-app.use(exampleRoutes)
+app.use(exampleRoutes) // remove example routes if we aren't using them 
 app.use(userRoutes)
 app.use(gameRoutes)
 app.use(questionRoutes)