Grading PR

.eslintrc

@@ -1,5 +1,8 @@
 {
-   "settings": {
+  "parserOptions": {
+    "sourceType": "module"
+  },
+  "settings": {
     "ecmascript": 6
   },
   "ecmaFeatures": {

README.md

@@ -1,7 +1,12 @@
 # Lunch Time!
 
 ## USERS:
-How many times have you looked glumly at yet another lunch from the same old place and wondered where your classmates were getting theirs?  Surely there must be enough recommendations that it would be worth seeing them all in one place.
+How many times have you looked glumly at yet another lunch from the same old 
+place and wondered where your classmates were getting theirs?  
+Surely there must be enough recommendations that it would be 
+worth seeing them all in one place.
+
+// Don't make your markdown lines so long :)
 
 **LunchPad** allows you to do just that.  Anyone working, studying or otherwise routinely occupying themselves at a given location can start a **LunchPad** community -- or join one that already exists -- and invite others to join them in posting brief reports on where they have had lunch.  Users log in and instantly see the most recent lunch experiences for their community, including time required, price and quality of the fare, and whether or not it was a worthwhile pick.  Experiences can be grouped either by vendor or by user, and **LunchPad** allows you to save your favorite users in order to quickly access just their experiences.  Have someone whose recommendations you especially value?  Just add them to your favorites.
 

lib/app.js

@@ -1,4 +1,4 @@
-'use strict';
+
 
 const express = require('express');
 const app = express();
@@ -13,6 +13,7 @@ const users = require('./routes/users');
 const experiences = require('./routes/experiences');
 const vendors = require('./routes/vendors');
 
+// why start all your routes with "lunch"?
 app.use('/lunch/auth', auth);
 app.use('/lunch/community', ensureAuth, communities);
 app.use('/lunch/experiences', ensureAuth, experiences);

lib/auth/ensure-auth.js

@@ -1,4 +1,4 @@
-'use strict';
+
 
 const tokenSvc = require('./token');
 
@@ -11,7 +11,7 @@ module.exports = function getEnsureAuth() {
         req.user = payload;
         next();
       })
-      .catch(err => { //eslint-disable-line
+      .catch(() => {
         next({ code: 403, error: 'Unauthorized, bad token'});
       });
   };

lib/auth/token.js

@@ -1,4 +1,4 @@
-'use strict';
+
 
 const jwt = require('jsonwebtoken');
 const sekrit = process.env.APP_SECRET || 'lunchmeat';
@@ -8,6 +8,9 @@ module.exports = {
     return new Promise((resolve, reject) => {
       const payload = {
         id: user._id,
+        // add this so you don't have to fetch to 
+        // go from user to their community
+        communityId: user.communityId,
         roles: user.roles
       };
 

lib/error-handler.js

@@ -1,4 +1,4 @@
-'use strict';
+
 
 module.exports = function errorHandler(err, req, res, next) { //eslint-disable-line
   const code = err.code || 500;

lib/models/community.js

@@ -1,8 +1,8 @@
-'use strict';
+
 const mongoose = require('mongoose');
 const Schema = mongoose.Schema;
 
-const communitySchema = new Schema({ //eslint-disable-line
+const communitySchema = new Schema({
   name: {type: String, required: true}
 });
 

lib/models/experience.js

@@ -1,17 +1,15 @@
-'use strict';
+
 
 const mongoose = require('mongoose');
 const Schema = mongoose.Schema;
 
 const schema = new Schema({
   communityId: {
     type: Schema.Types.ObjectId,
-    // required: true,
     ref: 'Community'
   },
   userId: {
     type: Schema.Types.ObjectId,
-    // required: true,
     ref: 'User'
   },
 
@@ -27,7 +25,6 @@ const schema = new Schema({
   time: {
     type: String,
     required: true,
-    // default: Date.now
   },
 
   howFast: {

lib/models/user.js

@@ -1,20 +1,25 @@
-'use strict';
+
 const mongoose = require('mongoose');
 const Schema = mongoose.Schema;
 const bcrypt = require('bcryptjs');
 
-const userSchema = new Schema({ //eslint-disable-line
+const userSchema = new Schema({
   username: {type: String, required: true},
   password: {type: String, required: true},
   roles: {type: [String], default: 'user'},
   communityId: {
     type: Schema.Types.ObjectId,
     ref: 'Community'
   },
+  // why store the name as well?
   communityName: {
     type: String
   },
-  favoriteUsers: {type: Array}
+  // be explicit on what type of array
+  favoriteUsers: [{ 
+    type : Schmea.Types.ObjectId, 
+    ref: 'User' 
+  }]
 });
 
 userSchema.methods.generateHash = function(password) {

lib/models/vendor.js

@@ -1,12 +1,11 @@
-'use strict';
+
 const mongoose = require('mongoose');
 const Schema = mongoose.Schema;
 
-const vendorSchema = new Schema({ //eslint-disable-line
+const vendorSchema = new Schema({
   name: {type: String, required: true},
   Address: {type: String},
   cuisine: {type: String}
-
 });
 
 module.exports = mongoose.model('vendor', vendorSchema);

lib/routes/auth.js

@@ -1,15 +1,14 @@
-'use strict';
+
 
 const express = require('express');
 const router = express.Router(); //eslint-disable-line
 const bodyParser = require('body-parser').json();
 const token = require('../auth/token');
 const User = require('../models/user');
-// const Community = require('../models/community');
 
 router
   .post('/signup', bodyParser, (req, res, next) => {
-    const { username, password } = req.body; //eslint-disable-line
+    const { username, password } = req.body;
     delete req.body.password;
 
     if(!username || !password) {
@@ -18,6 +17,7 @@ router
         error: 'username and password required'
       });
     }
+
     User.find({ username })
       .count()
       .then(count => {
@@ -34,20 +34,6 @@ router
       .catch(next);
   })
 
-  //   Community.find({ name: community })
-  //     .then(comm => {
-  //       if (comm.length === 0) {
-  //         const newComm = new Community({name: community});
-  //         return newComm.save();
-  //       } else {
-  //         return comm[0];
-  //       }
-  //     })
-  //     .then(community => {
-  //     .catch(next);
-  // })
-
-
   .post('/signin', bodyParser, (req, res, next) => {
     const { username, password } = req.body;
     delete req.body.password;

lib/routes/communities.js

@@ -1,83 +1,83 @@
-'use strict';
+
 
 const express = require('express');
 const router = express.Router(); //eslint-disable-line
 const bodyParser = require('body-parser').json();
 const Experience = require('../models/experience');
 const Community = require('../models/community');
-const token = require('../auth/token'); //eslint-disable-line
 const User = require('../models/user');
 
 router
-  .get('/advance', (req, res, next) => { 
+  // calledAhead is a query param, not a resource path.
+  // DRY: don't repeat the same get code twice :(
+  .get('/', (req, res, next) => {
+    // a seperate middleware function 
+    // separtes making query filter from main data fetch
+
+    // I would add the communityId to info in jwt payload, 
+    // so don't have to get the fetch user's communityId...
     User
       .findById(req.user.id)
-      .then(user => {
-        return user.communityId;        
-      })
-      .then(id => {
-        Experience
-          .find({communityId: id, calledAhead: true})
-          .populate('userId', 'username')
-          .populate('communityId', 'name')
-          .limit(25)
-          .sort([['_id', -1]])
-          .then(experiences => {
-            experiences.forEach(item => {
-              item.postedOn = item._id.getTimestamp();
-            });
-            res.send(experiences);
-          })
-          .catch(next);
-      })
-      .catch(next); 
-  })
-
-  .get('/:id', (req, res, next) => { 
-
+      // don't retrieve more than you need...
+      .select(communityId)
+      .lean()
+      .then(user => user.communityId)
+      .then(communityId => {
+        const filter = req.filter = { communityId };
+        const { calledAhead } = req.query;
+        if(calledAhead === 'true') filter.calledAhead = true;
+        else if(calledAhead === 'false') filter.calledAhead = false;
+        next();
+      });
+  }, (req, res, next) => {
     Experience
-      .find({communityId: req.params.id})
+      .find(filter)
       .populate('userId', 'username')
       .populate('communityId', 'name')
       .limit(25)
+      // are you trying to time sort?
       .sort([['_id', -1]])
       .then(experiences => {
         experiences.forEach(item => {
+          // if you add timestamp option to schema,
+          // it would auto add item.createOn.
+          // Otherwise, move this to a virtual property on the model...
           item.postedOn = item._id.getTimestamp();
         });
         res.send(experiences);
       })
       .catch(next);
   })
-
-
-  .post('/join', bodyParser, (req, res, next) => {
+
+  // This should be put to /:id/users
+  .put('/:id/users', bodyParser, (req, res, next) => {
+    // Use _id, not name to "id"entify resources
     const {name} = req.body;
-    let communityId;
-    let communityName;
-    Community.find({name})
-    .then(commArr => {
-      if(commArr.length === 0) { 
+    // use findOne to only get one. Better yet, use Id.
+    // Then you could skip this find alltogether
+    Community.findOne({name})
+    .then(community => {
+      if(!community) { 
         throw {
+          // probably better as a 404
           code: 400,
           error: `Community ${name} does not yet exist!`
         };
-      } else {
-        communityId = commArr[0]._id;
-        communityName = commArr[0].name;
-        return User.findByIdAndUpdate(req.user.id, {communityId, communityName}, {new:true}); 
       }
+      // throw short-circuits, no need for "else"
+
+      const { communityId, communityName } = community;
+      return User.findByIdAndUpdate(req.user.id, {communityId, communityName}, {new:true}); 
     })
-    .then(user => { //eslint-disable-line
+    .then(() => {
+      // odd return...
       res.send({communityId});
     })
-    .catch(err => {
-      next(err);
-    });
+    .catch(next);
   })
 
   .post
-    ('/create', bodyParser, (req, res, next) => {
+    ('/', bodyParser, (req, res, next) => {
       const {name} = req.body;
       Community.find({name})
         .then(commArr => {
@@ -94,7 +94,8 @@ router
               communityName = newComm.name;
               return User.findByIdAndUpdate(req.user.id, {communityId, communityName}, {new:true});
             })
-            .then(user => { //eslint-disable-line
+            .then(() => {
+              // should send back community object here...
               res.send({communityId});
             });
         })

lib/routes/experiences.js

@@ -1,4 +1,4 @@
-'use strict';
+
 
 const express = require('express');
 const router = express.Router(); //eslint-disable-line
@@ -7,25 +7,21 @@ const Experience = require('../models/experience');
 const User = require('../models/user');
 
 router
-  .get('/:username', (req, res, next) => {
-
-    const usrName = req.params.username;
-    User
-      .find({username: usrName})
-      // .populate('userId', 'usrName')
-      // .lean()
+  // 
+  .get('/', (req, res, next) => {
+    // is this supposed to be for current user?
+    // or any user?
+    // Again, use id's, not names!
+    const userId = req.query.user;
+    Experience.find({userId})
+      .populate('userId', 'username')
+      // sort was in wrong place
       .sort([['_id', -1]])
-      .then(user => {
-        Experience.find({userId: user[0]._id})
-        .populate('userId', 'username')
-        .then(experiences => {
-          experiences
-            .sort([['_id', -1]])
-            .forEach(item => {
-              item.postedOn = item._id.getTimestamp();
-            });
-          res.send(experiences);
+      .then(experiences => {
+        experiences.forEach(item => {
+          item.postedOn = item._id.getTimestamp();
         });
+        res.send(experiences);
       })
       .catch(next);