diff --git a/.eslintrc b/.eslintrc index 4659c5e..e49ef47 100644 --- a/.eslintrc +++ b/.eslintrc @@ -1,5 +1,8 @@ { - "settings": { + "parserOptions": { + "sourceType": "module" + }, + "settings": { "ecmascript": 6 }, "ecmaFeatures": { diff --git a/README.md b/README.md index 4ef4ee8..2e31d7f 100644 --- a/README.md +++ b/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. diff --git a/lib/app.js b/lib/app.js index 8876bcd..45c8ff7 100644 --- a/lib/app.js +++ b/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); diff --git a/lib/auth/ensure-auth.js b/lib/auth/ensure-auth.js index a6af2da..a77c7b2 100644 --- a/lib/auth/ensure-auth.js +++ b/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'}); }); }; diff --git a/lib/auth/ensure-role.js b/lib/auth/ensure-role.js deleted file mode 100644 index e69de29..0000000 diff --git a/lib/auth/token.js b/lib/auth/token.js index 254ef53..2c39818 100644 --- a/lib/auth/token.js +++ b/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 }; diff --git a/lib/error-handler.js b/lib/error-handler.js index 4ee579b..f8911e0 100644 --- a/lib/error-handler.js +++ b/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; diff --git a/lib/models/community.js b/lib/models/community.js index 6ea1b9e..cdbb6c9 100644 --- a/lib/models/community.js +++ b/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} }); diff --git a/lib/models/experience.js b/lib/models/experience.js index 5b1ed90..e30c047 100644 --- a/lib/models/experience.js +++ b/lib/models/experience.js @@ -1,4 +1,4 @@ -'use strict'; + const mongoose = require('mongoose'); const Schema = mongoose.Schema; @@ -6,12 +6,10 @@ 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: { diff --git a/lib/models/user.js b/lib/models/user.js index 55e665c..a49d742 100644 --- a/lib/models/user.js +++ b/lib/models/user.js @@ -1,9 +1,9 @@ -'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'}, @@ -11,10 +11,15 @@ const userSchema = new Schema({ //eslint-disable-line 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) { diff --git a/lib/models/vendor.js b/lib/models/vendor.js index 3a28186..d06c0bb 100644 --- a/lib/models/vendor.js +++ b/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); diff --git a/lib/routes/auth.js b/lib/routes/auth.js index 40aa6dc..7ec7ff0 100644 --- a/lib/routes/auth.js +++ b/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; diff --git a/lib/routes/communities.js b/lib/routes/communities.js index 24b7623..13c33c4 100644 --- a/lib/routes/communities.js +++ b/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}); }); }) diff --git a/lib/routes/experiences.js b/lib/routes/experiences.js index e989b37..03d0b72 100644 --- a/lib/routes/experiences.js +++ b/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); diff --git a/lib/routes/users.js b/lib/routes/users.js index 5cad425..6abc50d 100644 --- a/lib/routes/users.js +++ b/lib/routes/users.js @@ -1,28 +1,26 @@ -'use strict'; + const express = require('express'); const router = express.Router(); //eslint-disable-line -// const Experience = require('../models/experience'); -// const Community = require('../models/community'); -const token = require('../auth/token'); //eslint-disable-line const User = require('../models/user'); const bodyParser = require('body-parser').json(); router - .get('/favorite', (req, res, next) => { //eslint-disable-line + // Use "me" to indicate that the /users/:id is current user + .get('/me/favorites', (req, res, next) => { + // you could have done this much more cleanly with .aggregate User .findById(req.user.id) + .select('favoriteUsers') + .lean() .then(user => { - return Promise.all( - user.favoriteUsers.map(fav => { - return User.findById(fav); - }) - ); + // use $in to select favs in one go! + return User + .find({ _id: { $in: user.favoriteUsers }}) + .select('username') + .lean(); }) - .then(promiseReturn => { - let favNames = promiseReturn.map(user => { - return {username:user.username}; - }); + .then(favNames => { res.send(favNames); }) .catch(next); @@ -36,6 +34,7 @@ router .catch(next); }) + // This should be a query against /users .get('/id/:name', (req, res, next) => { User .find({ username: req.params.name }) @@ -48,8 +47,9 @@ router }) - .put('/favorite', bodyParser, (req, res, next) => { + .put('/favorites', bodyParser, (req, res, next) => { let favUser; + // use id's! User.find({username: req.body.username}) .then(user => { favUser = user[0]._id; diff --git a/lib/routes/vendors.js b/lib/routes/vendors.js index 70b7c1d..1048d18 100644 --- a/lib/routes/vendors.js +++ b/lib/routes/vendors.js @@ -1,4 +1,4 @@ -'use strict'; + const express = require('express'); // const mongoose = require('mongoose'); @@ -24,15 +24,20 @@ router }); }) .then(vendors => { + // boo, O(n^2)! :( + // Remember using a hash map? O(n log n) return vendors.filter((item, i) => { return vendors.indexOf(item) === i; }); + // Doesn't make a lot of sense to do this anyway... }) .then(uniqVendors => { return Promise.all( uniqVendors.map(vendor => { + // this whole thing should be part of a single aggregate pipeline return Experience.aggregate([ { $match: { name: vendor } }, + // limit line length to fit so you can see ALL code { $group: { _id: '$name', howFast: { $avg: '$howFast' }, cost: { $avg: '$cost' }, worthIt: { $avg: '$worthIt' } } } ]) .exec(); @@ -46,4 +51,21 @@ router .catch(next); }); +// compare above to: +router.get('/', (req, res, next) => { + Experience + .aggegate([ + { $match: { communityId: req.user.communityId }}, + { $group: { + _id: '$name', + howFast: { $avg: '$howFast' }, + cost: { $avg: '$cost' }, + worthIt: { $avg: '$worthIt' } + }} + ]) + .exec() + .then(experiences => res.send(experiences)) + .catch(next); +}) + module.exports = router; \ No newline at end of file diff --git a/lib/setup-mongoose.js b/lib/setup-mongoose.js index d969297..b38d8f4 100644 --- a/lib/setup-mongoose.js +++ b/lib/setup-mongoose.js @@ -1,4 +1,4 @@ -'use strict'; + const mongoose = require( 'mongoose' ); diff --git a/package.json b/package.json index 49d6da7..0fb3df2 100644 --- a/package.json +++ b/package.json @@ -5,9 +5,9 @@ "main": "index.js", "engines": {"node": "6.8.1"}, "scripts": { + "pretest": "npm run lint", "test": "MONGODB_URI=mongodb://localhost/lunch-test mocha --recursive", "lint": "eslint .", - "pretest": "npm run lint", "start": "node server.js" }, "repository": { @@ -30,7 +30,6 @@ "dependencies": { "bcryptjs": "^2.3.0", "body-parser": "^1.15.2", - "dotenv": "^2.0.0", "express": "^4.14.0", "jsonwebtoken": "^7.1.9", "mongoose": "^4.6.7", diff --git a/public/scripts/.eslintrc b/public/scripts/.eslintrc new file mode 100644 index 0000000..74e0826 --- /dev/null +++ b/public/scripts/.eslintrc @@ -0,0 +1,5 @@ +{ + "parserOptions": { + "sourceType": "script" + } +} diff --git a/server.js b/server.js index 6438b7b..8b350cf 100644 --- a/server.js +++ b/server.js @@ -1,4 +1,4 @@ -'use strict'; + const app = require('./lib/app'); const http = require('http'); diff --git a/test/before-after.js b/test/before-after.js index 764ca50..1d460bd 100644 --- a/test/before-after.js +++ b/test/before-after.js @@ -1,4 +1,4 @@ -'use strict'; + const connection = require('../lib/setup-mongoose'); const db = require('./db'); diff --git a/test/db.js b/test/db.js index 03c7d32..62ec41f 100644 --- a/test/db.js +++ b/test/db.js @@ -1,5 +1,3 @@ -'use strict'; - const connection = require('mongoose').connection; const state = require('mongoose/lib/connectionstate'); diff --git a/test/e2e/auth.api.test.js b/test/e2e/auth.api.test.js index 5749c42..2ab5ffb 100644 --- a/test/e2e/auth.api.test.js +++ b/test/e2e/auth.api.test.js @@ -1,4 +1,4 @@ -'use strict'; + const chai = require('chai'); const chaiHttp = require('chai-http'); diff --git a/test/e2e/crud.api.test.js b/test/e2e/crud.api.test.js index 5d605b3..d6eed9d 100644 --- a/test/e2e/crud.api.test.js +++ b/test/e2e/crud.api.test.js @@ -1,4 +1,4 @@ -'use strict'; + const chai = require('chai'); const chaiHttp = require('chai-http'); const assert = chai.assert; @@ -127,6 +127,7 @@ describe('testing experience endpoints', () => { .get('/lunch/community/' + user.communityId) .set('authorization', user.token) .then(res => { + // vague test, be more explicit assert.isArray(res.body); assert.isOk(res.body); done(); diff --git a/test/test.js b/test/test.js deleted file mode 100644 index f042232..0000000 --- a/test/test.js +++ /dev/null @@ -1,10 +0,0 @@ -// 'use strict'; - -// const chai = require('chai'); //eslint-disable-line - - -// describe('it works', () => { -// it('should work', () => { - -// }); -// }); diff --git a/test/unit_testing/community.test.js b/test/unit_testing/community.test.js index 8066c82..6aab04d 100644 --- a/test/unit_testing/community.test.js +++ b/test/unit_testing/community.test.js @@ -1,4 +1,4 @@ -'use strict'; + const Community = require('../../lib/models/community'); const assert = require('chai').assert; @@ -13,15 +13,4 @@ describe('creates the full community model', () => { done(); }); }); - - // it('should require location field', done => { - // const comm = new Community ({ - // name: 'chipotle' - // }); - - // comm.validate(err => { - // assert.isOk(err, 'name field is required'); - // done(); - // }); - // }); }); \ No newline at end of file diff --git a/test/unit_testing/experience.test.js b/test/unit_testing/experience.test.js index 4f9c5f8..5fff857 100644 --- a/test/unit_testing/experience.test.js +++ b/test/unit_testing/experience.test.js @@ -1,4 +1,4 @@ -'use strict'; + const Experience = require('../../lib/models/experience'); const assert = require('chai').assert; @@ -16,20 +16,6 @@ describe('Creates full Model for experiences', () => { }); }); - // it('should require time field', done => { - // const experience = new Experience({ - // name: 'test user', - // howfast: 3, - // calledAhead: true, - // time: new Date() - // }); - - // experience.validate(err => { - // assert.isNotOk(err, 'time is required'); - // done(); - // }); - // }); - it('should require howfast field', done => { const experience = new Experience({ name: 'test user', diff --git a/test/unit_testing/user.test.js b/test/unit_testing/user.test.js index 3441f47..7068911 100644 --- a/test/unit_testing/user.test.js +++ b/test/unit_testing/user.test.js @@ -1,4 +1,4 @@ -'use strict'; + const User = require('../../lib/models/user'); const assert = require('chai').assert; @@ -35,7 +35,9 @@ describe('it creates a full user model', () => { username: 'test user', communityId: '582a52f8eaee951a90b97839' }); - assert.isArray(user.roles); + // be explicit + assert.equal(user.roles.length, 1); + assert.equal(user.roles[0], 'user'); done(); }); }); diff --git a/test/unit_testing/vendor.test.js b/test/unit_testing/vendor.test.js index 2c8726c..3a382b6 100644 --- a/test/unit_testing/vendor.test.js +++ b/test/unit_testing/vendor.test.js @@ -1,4 +1,4 @@ -'use strict'; + const Vendor = require('../../lib/models/vendor'); const assert = require('chai').assert;