Judy day 12#15
Judy day 12#15JudyVue wants to merge 12 commits intocodefellows-seattle-javascript-401d10:masterfrom
Conversation
|
|
||
| const Promise = require('bluebird'); | ||
| const createError = require('http-errors'); | ||
| const debug = require('debug')('note:storage'); |
There was a problem hiding this comment.
You should name your debug module after your model -- so it'll be like 'person:storage'.
| 'use strict'; | ||
|
|
||
| const createError = require('http-errors'); | ||
| const debug = require('debug')('note:error-middleware'); |
There was a problem hiding this comment.
You should name your debug module after your model -- so it'll be like 'person:storage'.
| Person.updatePerson = function(id, person) { | ||
| debug('updatePerson'); | ||
| if (!id) return Promise.reject(createError(400, 'bad request')); | ||
| if (!person) return Promise.reject(createError(400, 'bad request')); |
| "description": " lab 11 single resource express api ======", | ||
| "main": "index.js", | ||
| "scripts": { | ||
| "start": "DEBUG='note*' node server.js", |
There was a problem hiding this comment.
if you change your debug require statements to use the name of your model (which is a best practice), remember to change your scripts to reflect that.
| "morgan": "^1.7.0", | ||
| "node-uuid": "^1.4.7" | ||
| } | ||
| } |
There was a problem hiding this comment.
You should manually go in and delete dependencies that are not in the right spot. devDependencies should include chai, eslint, and mocha. Everything else is a regular dependency, and shouldn't be duplicated in dev dependencies.
| personRouter.delete('/api/person/:id', function(req, res, next){ | ||
| debug ('hit route DELETE /api/person'); | ||
| Person.deletePerson(req.params.id) | ||
| .then (() => res.sendStatus(204)) |
There was a problem hiding this comment.
Good job handling the delete response :)
|
|
||
| Person.updatePerson(req.params.id, req.body) | ||
| .then ((person) => res.json(person)) | ||
| .catch (next); |
There was a problem hiding this comment.
catch(next) is doing the same thing as catch((err) => next(err)) but you should be consistent in your style... so if you write it one way in one route, keep writing it that way.
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
you are currently not making use of your error-handling middleware. You'll need an app.use where you pass in your error-handling middleware after your router.
| let updatedPerson = {name: 'updated', age: '45'}; | ||
| request.put(`${url}/api/person/${this.tempPerson.id}`) | ||
| .send(updatedPerson) | ||
| .end((err, res) => { |
There was a problem hiding this comment.
You should do error handling in a test where you expect everything to go well (200):
if (err) return next(err);
| }); | ||
| // * TODO: `POST` - test 200, response body like `{<data>}` for a post request with a valid body | ||
| describe('testing for POST request with valid body', function(){ | ||
| before(done => { |
There was a problem hiding this comment.
you don't need to create a person in your database to set up your test environment for POST tests, as you'll be posting a person and you don't need that person to exist beforehand to properly test that route.
| }); | ||
| // `POST` - test 400, responds with 'bad request' for if no `body provided` or `invalid body` | ||
| describe('testing post for bad request if no body provided', function(){ | ||
| before(done => { |
There was a problem hiding this comment.
Same as above - you won't need to create a person here or delete one (as no person will be created through your POST test if it's a bad request).
| //DELETE - test 404, for a DELETE request with an invalid or missing id | ||
| //404 for missing id because DELETE /api/<simple-resource-name>/ is not a route | ||
| describe('testing DELETE for request with invalid/missing id', function(){ | ||
| before(done => { |
There was a problem hiding this comment.
No need to create a person if you're testing a 404 -- nothing will be deleted if the request is bad or the person is not found.
2 participants
No description provided.