Connor's Personal API #33
Conversation
| success: renderMultipleGames | ||
| }); | ||
|
|
||
| // Displays new game form on click |
There was a problem hiding this comment.
next bunch of lines should be indented
| event.preventDefault(); | ||
| let formData = $(this).serialize(); | ||
|
|
||
| // console.log(formData); |
public/scripts/app.js
Outdated
| </div> | ||
| ` | ||
| $('.display-games').prepend(newHTML); | ||
| // console.log(game._id); |
There was a problem hiding this comment.
there's a bunch of these commented out console logs. I won't mark all of them
public/scripts/app.js
Outdated
| url: putURL, | ||
| data: updatedGameData, | ||
|
|
||
| // success: function(msg){ |
| endpoints: [ | ||
| {method: "GET", path: "/api", description: "Describes all available endpoints"}, | ||
| {method: "GET", path: "/api/profile", description: "Data about me"}, // CHANGE ME | ||
| {method: "POST", path: "/api/campsites", description: "E.g. Create a new campsite"} // CHANGE ME | ||
| {method: "GET", path: "/api/profile", description: "A little information about me"}, |
There was a problem hiding this comment.
add in all your routes for /api/boardgames
| if (err) { | ||
| console.log('ERROR from controller create', err); | ||
| } | ||
| if (game.image) { |
There was a problem hiding this comment.
Could think about doing this before create, so that you only have a single database transaction:
if (!req.body.image) {
req.body.image = 'images/generic-game-image.jpg';
}
db.Boardgame.create(...| game.image = 'images/generic-game-image.jpg'; | ||
| game.save(function(err, newGame) { | ||
| if (err) { | ||
| console.log("ERROR from update controller", err); |
There was a problem hiding this comment.
this is clearly copy-pasted from update, with now-incorrect comments
| // Updates edited game | ||
| function update (req, res) { | ||
| db.Boardgame.findById(req.params.id, function(err, foundGame) { | ||
| foundGame.title = req.body.title, |
There was a problem hiding this comment.
commas 😭 these should each be their own line of code, with a semicolon
| function destroy (req, res) { | ||
| db.Boardgame.findByIdAndRemove(req.params.id, function(err, deletedGame) { | ||
| if (err) { | ||
| console.log(err); |
There was a problem hiding this comment.
even if there's an error, would be nice to send back some response
|
|
||
|
|
||
| app.get('/api/boardgames', controllers.boardgames.index); | ||
| app.post('/api/newBoardgame', controllers.boardgames.create); |
There was a problem hiding this comment.
standard REST conventions say this should just be a POST to /api/boardgames
| <div class="col-md-6 col-md-offset-3 display-games"> | ||
| <!-- start of one game item --> | ||
| <!-- Test display with one game --> | ||
| <!-- <div class="card" style="width: 45rem;"> |
There was a problem hiding this comment.
This commented-out code is now captured in your app.js file; can be removed from here to keep this file less cluttered
| </div> | ||
|
|
||
| <div class="form-group row"> | ||
| <label for="example-text-input" class="col-2 col-form-label">Image</label> |
There was a problem hiding this comment.
label for is supposed to be for the id of the input the label points at--you need to add IDs to the inputs and make the for equal to that ID.
| }) | ||
|
|
||
| // Allows user to go back to main view from new game form | ||
| $('#cancel-btn').on('click', function(event) { |
There was a problem hiding this comment.
This callback and the above could use the same function (could prevent default on both and call toggle; then you'd only need to write such a function once)
| }) | ||
|
|
||
| // Creates new game on submit | ||
| $('.new-game-form').on('submit', function(event) { |
There was a problem hiding this comment.
This is the only longer, data-sending callback that you write inline--it would be better to be consistent, and define this method where you define handleEditGame/handleUpdateGame/etc.
|
|
||
| renderOneGame(updatedGame); | ||
|
|
||
| $('[data-game-id =' + gameId + ']')[0].scrollIntoView(); |
No description provided.