Final project feedback

app.js

@@ -11,7 +11,7 @@ var express = require('express'),
     passport = require('passport');
     cookieParser = require('cookie-parser');
 
-mongoose.connect('mongodb://localhost/MakerSlot');
+mongoose.connect('mongodb://localhost/MakerSlot'); // every user gets their own local database?
 
 var app = module.exports = express.createServer();
 
@@ -23,7 +23,7 @@ app.use(express.bodyParser());
 app.use(express.methodOverride());
 app.use(cookieParser());
 app.use(express.static(__dirname + '/public'));
-app.use(session({secret: 'weremakingawebsiteforseveral3dprinters', cookie:{}, saveUninitialized: false, resave: false}));
+app.use(session({secret: 'weremakingawebsiteforseveral3dprinters', cookie:{}, saveUninitialized: false, resave: false})); // hide the secret in the process.env?
 app.use(passport.initialize());
 app.use(passport.session());
 app.use(app.router);
@@ -45,16 +45,15 @@ app.get('/deletePrint', routes.deletePrint)
 
 //Forum routes
 app.get('/forum', routes.dex)
-app.get('/getPosts', routes.getforumposts)
+app.get('/getPosts', routes.getforumposts) // most of your route names are camelcase (editForm) -- but this one's lowercase. Consistency!
 
 app.post('/newPost', routes.newforumpost)
 
 //Login/logout routes
 app.get('/login', userroutes.login)
 app.get('/logout', userroutes.logout)
 app.get('/check', function(req,res){
-	console.log("checking");
-	console.log(req.session);
+	// get rid of debugging mechanisms before submitting code
 	res.send(req.user)
 })
 // Routes
@@ -74,6 +73,7 @@ app.post('/userCreate', passport.authenticate('local-signup',{
 app.post('/submit', function(req,res){console.log("stahp")})
 
 //Run verification code and import its important functions.
+// should this happen at the top? you use the "verification" variable earlier on.
 var verification = require('./verification.js');
 
 

models/forumModel.js

@@ -1,6 +1,6 @@
 var mongoose = require('mongoose');
 
-// Create a wiki page schema
+// Create a wiki page schema <-- make sure you update comments to match new context if you're reusing code :)
 var forumSchema = mongoose.Schema({
     user: String,
     content: String,

models/printModel.js

@@ -23,11 +23,11 @@ var printSchema = mongoose.Schema({
     printMass: Number,
     dateAndTime: String,
     finish: String, //we need to add functionality to this
-    duration: Number, //In minutes //and probably remove this
+    duration: Number, //In minutes //and probably remove this <- I think you should -- either this or finish time
     ninjaApproval: Boolean,
     printer: String,
     problems: String
 
 });
 
-module.exports  = mongoose.model('print', printSchema);
+module.exports = mongoose.model('print', printSchema);

models/printerModel.js

@@ -1,9 +1,9 @@
 var mongoose = require('mongoose');
 
-// Create a wiki page schema
+// Create a wiki page schema <-- really, a wiki?
 var printerSchema = mongoose.Schema({
     name: String,
     status: String
 });
 
-module.exports  = mongoose.model('printer', printerSchema);
+module.exports  = mongoose.model('printer', printerSchema);

models/userModel.js

@@ -11,6 +11,7 @@ var userSchema = mongoose.Schema({
     email: String
 });
 
+// nice use of schema methods!
 // methods ======================
 // generating a hash
 userSchema.methods.generateHash = function(password) {
@@ -22,4 +23,4 @@ userSchema.methods.validPassword = function(password) {
     return bcrypt.compareSync(password, this.password);
 };
 
-module.exports  = mongoose.model('user', userSchema);
+module.exports  = mongoose.model('user', userSchema);

public/javascript/forum.js

@@ -1,6 +1,9 @@
 var forummodule = angular.module('forummodule', []);
+// I'm 100% ok with you using angular and 100% ok with you using straight-up jquery...
+// but I'm not ok with a combination of the two :/ Refactor to adhere to one design pattern?
 
-
+// I also can't tell how much of this file is in use -- looks like a good portion of it
+// is from the todo app.
 function mainController($scope, $http) {
     $scope.formData = {};
     $scope.commentFormData = {};

public/javascript/home.js

@@ -2,7 +2,10 @@ var onSuccess = function(data, status) {
   console.log('added event');
   window.location = "/schedule";
 };
-
+// The clientside global scope includes *ALL* of your javascript files -- and you're
+// naming your onSuccess and onError callbacks the same thing in three different files.
+// Those functions WILL overlap -- you need to rename so they don't!
+
 var onError = function(data, status) {
   console.log("status", status);
   console.log("error", data);

public/javascript/logs.js

@@ -7,7 +7,7 @@ var onSuccess = function(data, status) {
 	window.location = "/";
 }
 
-var onSuccess2 = function(data, status) {
+var onSuccess2 = function(data, status) { // yep, your onSuccess functions will conflict -- this is a workaround, but could you pick a more semantic name?
 	window.location = "/";
 }
 

public/javascript/main.js

@@ -1,21 +1,21 @@
 //populates calendar, autofills form for edits
 var onSuccess = function(data, status) {
   $(document).ready(function(){
-    $('input[value='+data.purpose+']').prop('checked',true); 
+    $('input[value='+data.purpose+']').prop('checked',true);
     $("#name").replaceWith('<input type="text" class="form-control" id="name" value="'+data.name+'">');
     $("#email").replaceWith('<input type="text" class="form-control" id="email" value="'+data.email+'">');
     $("#part").replaceWith('<input type="text" class="form-control" id="part" value="'+data.part+'">');
     $("#whatclass").replaceWith('<input type="text" class="form-control" id="whatclass" value="'+data.classes+'">');
     $("#mass").replaceWith('<input type="text" class="form-control" id="mass" value='+data.printMass+'>');
     $("#whenprint").replaceWith('<input type="text" class="form-control" id="whenprint" value="'+data.dateAndTime+'">');
     $("#whendone").replaceWith('<input type="text" class="form-control" id="whendone" value="'+data.finish+'">');
-    
+
     var hours = Math.floor(data.duration/60);
     var minutes = data.duration%60;
-    
+
     $("input[name=hours]").replaceWith('<input type="number" class="form-control numberentry" name="hours" min="0" max="50" value="'+hours+'">')
     $("input[name=minutes]").replaceWith('<input class="form-control numberentry" type="number" name="minutes" min="0" max="59" value="'+minutes+'">')
-    
+
     //change the data into something the program can use to find the html elements
     if(data.ninjaApproval == true) {
       var currentBoolean = "yes";
@@ -69,9 +69,9 @@ $(document).ready(function() {
           height: 800,
           firstDay: 1,
           defaultView: 'agendaWeek'
-        
+
     })
-    
+
     var calendar = $('#calendar').fullCalendar('getCalendar');
     calendar.on('dayClick', function(date, jsEvent, view) {
       alert('clicked on ' + date.format());
@@ -98,7 +98,7 @@ $("#printFormEdit").submit(function(event) {
     var hoursDur = $("input[name=hours]").val();
     var minutesDur = $("input[name=minutes]").val();
     var approved = $("input[name=approved]:checked").val();
-    console.log(approved);
+    // remove debugging mechanisms!
     var printer = $("input[name=printer]:checked").val();
     var duration = parseInt(hoursDur)*60 + parseInt(minutesDur);
     var problems = $("#problems").val();
@@ -132,6 +132,6 @@ function delPrint() {
       $.get("deletePrint", {id: clicked})
       .done(onSuccess4)
       .error(onError);
-    }   
-  } 
+    }
+  }
 }

routes/index.js

@@ -1,11 +1,12 @@
-Print = require("../models/printModel")
-Forum = require("../models/forumModel")
-Announce = require("../models/announcementModel")
-Printer = require("../models/printerModel")
-Users = require("../models/userModel")
+// make sure you use `var`; be consistent w/ your semicolons (a linter could help)
+var Print = require("../models/printModel");
+var Forum = require("../models/forumModel");
+var Announce = require("../models/announcementModel");
+var Printer = require("../models/printerModel");
+var Users = require("../models/userModel");
 var path = require('path'); //path allows the creation of paths (with /) from individual names
 
-routes = {} 
+routes = {}
 
 
 routes.index = function(req, res){
@@ -26,33 +27,31 @@ routes.getPrinters = function(req,res) {
 //creates new printer and returns all printers
 routes.newPrinter = function(req, res){
   Users.findOne({name: req.user}, function (err, person) {
-    if(err){
+    if (err) {
       res.send(err)
     };
-    if(person.isNinja == true){
-
+    // be consistent w/ indentation & spacing -- fixed here, an issue throughout
+    if (person.isNinja == true) {
+      Printer.create({
+        name: req.body.name,
+        status: req.body.status
+      }, function(err, printers) {
+        if (err) {
+          res.send(err)
+        };
 
-    Printer.create({
-      name: req.body.name,
-      status: req.body.status
-    }, function(err, printers) {
+        Printer.find(function(err, printer) {
           if (err) {
             res.send(err)
           };
-
-          Printer.find(function(err, printer) {
-            if (err) {
-              res.send(err)
-            };
           res.json(printer);
-          });
-        }
-    );
-  }
-})
+        });
+      });
+    }
+  });
 };
 
-//edit printer 
+//edit printer
 routes.editPrinter = function(req,res){
   Printer.update({
       _id : req.body.id},{$set:{
@@ -181,7 +180,7 @@ routes.editAnnouncement = function(req,res){
 //deletes an announcement
 routes.deleteAnnouncement = function(req,res){
   //removes a print by id, then grabs all the remaining prints and returns them so they can be posted onto the calendar and it can be rerendered.
-  
+
   Users.findOne({name: req.user}, function (err, person) {
     if(err){
       res.send(err)
@@ -232,7 +231,7 @@ routes.newforumpost = function(req, res){
   Forum.create({
     content: req.body.content,
     title: req.body.title,
-    user: req.user 
+    user: req.user
   }, function(err, forum) {
         if (err) {
           res.send(err)
@@ -295,6 +294,7 @@ routes.editPrint = function(req,res){
   //Updates a print through id, though that could be altered to whatever, probably even on-click, which we should see about doing.
   //returns all the prints in the database including the edited one as well as the edited text alone.
   Print.update({
+      // Is there a reason why you can't just $set:req.query? do you have to unpack in place like this?
       _id : req.query.id},{$set:{name: req.query.name,
         email: req.query.email,
         part: req.query.part,
@@ -336,9 +336,7 @@ routes.getPrints = function(req,res){
       res.send(err)
     var events = [];
     for (var i = 0;i<prints.length; i++) {
-      // var title = prints[i].name + 'printing on ' + prints[i].printer//this will be printer name + user
-      // var start = prints[i].dateAndTime
-      // var end = prints[i].endTime
+      // get rid of dead code!
       var event = {title: prints[i].name + ' printing on ' + prints[i].printer, start: prints[i].dateAndTime, end: prints[i].finish, id: prints[i]._id};
       events.push(event)
     }

routes/userroutes.js

@@ -1,6 +1,6 @@
 var path = require('path'); //path allows the creation of paths (with /) from individual names
 
-routes = {}
+var routes = {};
 //Allows logging in and out of user
 routes.login = function(req, res){
 	res.sendfile('/views/login.html', { root: path.join(__dirname, '../') });

verification.js

@@ -3,7 +3,7 @@ var passport = require('passport')
 var LocalStrategy = require('passport-local')
 var bcrypt = require('bcryptjs')
 
-//Both of these functions are required to make passport work. 
+//Both of these functions are required to make passport work.
 //We didn't need them to actually do anything, so they don't.
 passport.serializeUser(function(user, done) {
   done(null, user);
@@ -26,8 +26,8 @@ passport.use('local-signin', new LocalStrategy(
         done(null, false)
       }
       else if (!user.validPassword(password)) {
-      	//Password doesn't match 
-        return done(null, false); 
+      	//Password doesn't match
+        return done(null, false);
       }
       else {
       	//User is valid
@@ -50,12 +50,12 @@ passport.use('local-signup', new LocalStrategy(
         return done(null, false)
       }
       else{
-        userinfo = req.body
-        user = new User()
-        user.name = userinfo.username;
-        user.email = userinfo.email;
+        userinfo = req.body // redundant... you can just use req.body (see below)
+        var user = new User()
+        user.name = req.body.username;
+        user.email = req.body.email;
         user.isNinja = false;
-    
+
         user.save(function(err){
           if (err){
             // req.session.error = "Error saving user to database"
@@ -71,7 +71,7 @@ passport.use('local-signup', new LocalStrategy(
 // test authentication
 //"next" is the actual function for the route you protect with authentication
 var ensureAuthenticated = function(req, res, next) {
-  if (req.isAuthenticated()) { 
+  if (req.isAuthenticated()) {
     return next(req, res); }
   else{
     res.redirect('/login');
@@ -82,4 +82,8 @@ var ensureAuthenticated = function(req, res, next) {
 //This entire file is run when it is first called, so this is
 //the only export that es actually needed.
 functions = {"ensureAuthenticated": ensureAuthenticated}
-module.exports = functions
+module.exports = functions
+
+// You could also just:
+// modules.exports.ensureAuthenticated = ensureAuthenticated
+// Does the same thing as the previous two lines

views/home.html

@@ -62,7 +62,7 @@
                   <!-- <img class="card-img-top" src='../public/images/model.jpg' alt="Card image cap"> -->
                   <h4 class="card-title">Announcements</h4>
                   <p class="card-text">What's happening in 3D-print land.</p>
-                  <p class="card-text"><small class="text-muted">Last updated yesterday</small></p>
+                  <p class="card-text"><small class="text-muted">Last updated yesterday</small></p><!-- once again... hard-coded probably isn't the way to go in the long term -->
                   <a href="#" class="btn btn-primary">Tell Me</a>
                 </div>
               </div>
@@ -89,7 +89,7 @@ <h4 class="card-title">Cool things</h4>
         </div>
       </div>
     </div>
-    <iframe width="420" height="315" src="http://10.26.8.22:8080/?action=stream"></iframe> 
+    <iframe width="420" height="315" src="http://10.26.8.22:8080/?action=stream"></iframe> <!-- I can't reach this address... do you need to be running a server which you aren't currently running for this to work? -->
 
   </body>
 </html>