-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jonathan Mules' personal API #31
base: master
Are you sure you want to change the base?
Conversation
…orks, working on update features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good indentation! It makes reading your code a lot easier. I also really liked the picture you chose for the header.
console.log('body', req.body); | ||
|
||
// split at comma and remove and trailing space | ||
var movieTitles = req.body.movieTitles.split(',').map(function(item) { return item.trim(); } ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try finding a way to seperate the word from the comma's when displaying movie titles.
console.log(req.body) | ||
db.Director.findById(req.params._id, function(err, foundDirector) { | ||
|
||
if(err) { console.log(err); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bug doesn't allow update
app.get('/personal', controllers.personal.index) | ||
app.get('/director', controllers.director.index) | ||
app.post('/director', controllers.director.create) | ||
app.get('/director/:_id', controllers.director.show) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the time, we don't put underscores in these URL parameters. (That's just convention though.)
|
||
var DirectorSchema = new Schema({ | ||
name: String, | ||
movieTitles: [ String ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice use of an array in your schema!
console.log('director after POST', director); | ||
//render the server's response | ||
}); | ||
window.location.reload() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refreshing the page is the wrong way to display the updated data! I agree with the comment above that you want to render the server's response, not just refresh the page.
|
||
//update director info | ||
$('.edit-dir-form form').on('submit', function(e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You run this before the forms are on the page--you need to use event delegation, like you do for the couple of lines above. That's why your edit form is submitting the standard way; this code is never run.
No description provided.