David's work on the personal api#24
Conversation
There was a problem hiding this comment.
Overall API looks good, just needs to get EDIT function to save changes made along with the other two sections of the page to work. Just needs to add functionality to the TV and Project section of the page. Upon adding movie pictures, work on adjusting the picture automatically to the browser size. Also when editing the input values on the selected movie, find a way to keep the image so that way when changes are finalized the image isn't broken
mnfmnfm
left a comment
mnfmnfm
left a comment
There was a problem hiding this comment.
While this project does get most of Create/Read/Update working, it's missing Delete, and the frontend functionality is quite broken--editing doesn't display updated data, although it's saved correctly, and images are not saved/displayed correctly for any of your data.
| @@ -0,0 +1,11 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
You really need to .gitignore these files--they're taking over your code, and always claim that you're using a RUBY_MODULE.
| @@ -0,0 +1,6 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
so many files that aren't necessary for your code to run
| @@ -0,0 +1,14 @@ | |||
| // // GET /api/albums | |||
There was a problem hiding this comment.
This commented out code doesn't need to be here
| db.Movie.create(req.body) | ||
| // if (err) { console.log('error', err); } | ||
| console.log("hello"); | ||
| res.json(req.body); |
There was a problem hiding this comment.
Here, you should use a callback, and actually send the saved movie, rather than sending back whatever was sent by the client.
db.Movie.create(req.body, function(err, savedMovie) {
res.json(savedMovie);
})|
|
||
| function show(req, res) { | ||
| // send back all albums as JSON | ||
| db.Movie.findById({_id: req.params.movieid}, function(err, allMovies) { |
There was a problem hiding this comment.
findById just takes in the ID as the first argument, not an object.
|
|
||
| .edit-movies { | ||
| width: 30%; | ||
| margin: 0 0 0 25px; |
There was a problem hiding this comment.
The fact that you have these same styles on a bunch of classes makes me think you could have used one more class (like .movie-card or something) that was applied to .edit-movies and to .save-movies and to .delete-movies...
| var personalData = []; | ||
|
|
||
| // var db = require('./models'); | ||
| personalData.push({name: 'David Jue'}); |
There was a problem hiding this comment.
Again, rather than use seeded DB data, this should have been a hardcoded object.
| }) | ||
| }); | ||
|
|
||
| app.get('/api/profile', function apiProfile(req, res) { |
There was a problem hiding this comment.
You do all the work to put your profile information in the database, but still use this hardcoded object. You don't need all of that profile DB code.
| </div> | ||
| <div class="row nav-styles"> | ||
| <div class="col-sm-3"></div> | ||
| <div class="col-sm-2 text-center"> |
There was a problem hiding this comment.
Again, having these 3 icons where 2 of them did nothing is a confusing structure for your site.
| <div class="col-sm-3"></div> | ||
| </div> | ||
| <div class="row form-row"> | ||
| <!--<div class="col-md-8 col-md-offset-2">--> |
3 participants
Please review my code.