From bcc2146ca01e4655f60a76bda56e7efc34ad6559 Mon Sep 17 00:00:00 2001
From: yunhsincynthiachen <yun-hsin.chen@students.olin.edu>
Date: Mon, 24 Apr 2017 11:37:26 -0400
Subject: [PATCH] twotter feedback complete

---
 twoter/app.js                     |  2 ++
 twoter/authentication.js          |  1 +
 twoter/feedback.txt               | 13 +++++++++++++
 twoter/package.json               |  7 ++++++-
 twoter/public/javascripts/main.js |  3 ++-
 twoter/routes/twote.js            |  6 ++++--
 6 files changed, 28 insertions(+), 4 deletions(-)
 create mode 100644 twoter/feedback.txt

diff --git a/twoter/app.js b/twoter/app.js
index 522decaf..ffeeb57b 100644
--- a/twoter/app.js
+++ b/twoter/app.js
@@ -1,3 +1,4 @@
+//You should section and comment this file into packages, middleware, mongo-related stuff, auth stuff, routes stuff, etc.
 var express = require("express");
 var path = require("path");
 var logger = require("morgan");
@@ -58,6 +59,7 @@ app.get("/",function(req,res){
   }
 });
 
+//Your local login doesn't work, because you aren't hooking your login form to anything.
 app.get("/login", twote.login);
 app.get("/twotes", twote.twoteshome);
 app.post("/postTwote", twote.addTwote);
diff --git a/twoter/authentication.js b/twoter/authentication.js
index f95415da..928a1df6 100644
--- a/twoter/authentication.js
+++ b/twoter/authentication.js
@@ -1,3 +1,4 @@
+//I like that you kept this all in a different file!
 var passport = require('passport');
 var FacebookStrategy = require('passport-facebook').Strategy;
 
diff --git a/twoter/feedback.txt b/twoter/feedback.txt
new file mode 100644
index 00000000..c11e3770
--- /dev/null
+++ b/twoter/feedback.txt
@@ -0,0 +1,13 @@
+Homework Feedback:
+
+Functionality:
+- Completion: You are missing a lot of your dependencies in your package.json file. You should "npm install --save <package>" Most of the features work, except your local login page. You styled your login page and it looks super nice, and I like that you cleaned up your twotes page to be split up into divs.  (17/20)
+- Bug Free: Remove your log statements when you are in production. You didn't have local login working, but everything else is good. (10/10)
+
+Quality:
+- Good Coding Practices: Nice folder structure. The different functions that you use is good. (10/10)
+- Readability: Try to have more detailed comments for your route functions and in your main.js. Your functions and variables are named well. (8/10)
+
+Also, I ended up using my own auth info, but don't forget to add teaching team as collabs to the developer site, so we can access your stuff.
+
+Good work!!!!!!
diff --git a/twoter/package.json b/twoter/package.json
index a42a86d9..14488cb2 100644
--- a/twoter/package.json
+++ b/twoter/package.json
@@ -8,7 +8,12 @@
     "cookie-parser": "^1.3.3",
     "express": "^4.10.6",
     "express-handlebars": "^1.1.0",
-    "morgan": "^1.5.1"
+    "express-session": "^1.15.2",
+    "mongoose": "^4.9.5",
+    "morgan": "^1.5.1",
+    "passport": "^0.3.2",
+    "passport-facebook": "^2.1.1",
+    "passport-local": "^1.0.0"
   },
   "devDependencies": {},
   "scripts": {
diff --git a/twoter/public/javascripts/main.js b/twoter/public/javascripts/main.js
index 2c9a201b..6e0887f8 100644
--- a/twoter/public/javascripts/main.js
+++ b/twoter/public/javascripts/main.js
@@ -1,3 +1,4 @@
+//Comment the different components of this file, and remove unnecessary console log statements
 var onError = function(err,status){
   console.log("status",status);
   console.log("error",err);
@@ -14,7 +15,7 @@ $("#twote-out").submit(function(event){
     user_id: user_id,
   }).done(function(data){
     var div = "<div id='"+data._id+"'div><div class='"+data.user_id+"'><p>"+data.text+"--"+data.user+"</p></div><input class='delete-button' id='"+data._id+"' type='button' value='Delete'></div>";
-    $('#allTwotes').prepend(div);
+    $('#allTwotes').prepend(div); //nice use of prepend
   }).error(onError)
 });
 
diff --git a/twoter/routes/twote.js b/twoter/routes/twote.js
index 27e599f1..6a7e3f29 100644
--- a/twoter/routes/twote.js
+++ b/twoter/routes/twote.js
@@ -1,3 +1,4 @@
+//Be sure to comment what each route does. Good simple routes though!
 var Twotes = require('../models/twoteModel.js');
 var Users = require('../models/userModel.js');
 var routes = {};
@@ -10,11 +11,11 @@ routes.twoteshome = function(req, res){
   var username = (req.session.passport.user.displayName);
 
   Users.count({username: username}, function(err, i){
-    if (i==0){
+    if (i==0){ //good use of checking if username exists
       var newUser = new Users({username:username});
       newUser.save(function(err){
         if (err){
-          res.sendStatus(500);
+          res.sendStatus(500); //nice!
           return;
         }
         Twotes.find(function(err,twotes){
@@ -35,6 +36,7 @@ routes.twoteshome = function(req, res){
             })
           }
         })
+        //Remove this if not using:
         //res.send(newUser);
       })
     } else {