Conversation
…dating the server
| <script src="scripts/app.js"></script> | ||
| </head> | ||
| <body> | ||
|
|
| // console.log("Created new campsite", campsite._id) | ||
| // process.exit(); // we're all done! Exit the program. | ||
| // }) | ||
| const db = require("./models"); |
mnfmnfm
left a comment
mnfmnfm
left a comment
There was a problem hiding this comment.
Overall, this looks really good. You made it through all the requirements and have reasonable styling. Nice work.
server.js
Outdated
| {method: "GET", path: "/api/profile", description: "About me"}, | ||
| {method: "GET", path: "/api/birds", description: "Show all my favorite birds"}, | ||
| {method: "POST", path: "/api/birds", description: "Add a new favorite bird"}, | ||
| {method: "PUT", path: "/api/birds", description: "Modify a favorite bird"}, |
There was a problem hiding this comment.
These routes are actually (correctly!) at /api/birds/:id, not at /api/birds as this documentation says.
views/index.html
Outdated
|
|
||
| <!-- VENDOR SCRIPTS --> | ||
| <script src="//ajax.googleapis.com/ajax/libs/jquery/2.2.2/jquery.min.js"></script> | ||
| <script src="http://ajax.googleapis.com/ajax/libs/jquery/2.2.2/jquery.min.js"></script> |
There was a problem hiding this comment.
The fact that you load this over http always, rather than using https, means that your site doesn't work the first time on Heroku, and the user has to manually click "allow unsecure scripts".
| // console.log("Created new campsite", campsite._id) | ||
| // process.exit(); // we're all done! Exit the program. | ||
| // }) | ||
| const db = require("./models"); |
seed.js
Outdated
| console.log("successfully created birdSeed"); | ||
| }); | ||
|
|
||
| process.exit(); //so it does not hang there |
There was a problem hiding this comment.
Having this at the end here is dangerous! It means that you might exit the process before the data is finished saving. This line should go INSIDE your callback (i.e. line 108/109), so that you only exit the process after the data is saved.
public/styles/styles.css
Outdated
| background-size: contain; | ||
| } | ||
|
|
||
| #bird-form { |
| function handelEditBird(e) { | ||
| e.preventDefault(); | ||
| let birdId = $(this).closest(".bird").data("bird-id"); | ||
| let $bird = $(this).closest(".bird"); |
There was a problem hiding this comment.
These two lines do the same bunch of work, and you could put them in the opposite order to great effect!
let $bird = $(this).closest(".bird");
let birdId = $bird.data("bird-id");|
|
||
| //change the bird name in the span to become input with the value of the current bird name | ||
| let birdName = $bird.find(".bird-name").text(); | ||
| $bird.find(".bird-name").html(`<input class="edit-bird-name" value="${birdName}"></input>`); |
There was a problem hiding this comment.
similarly, could do less work here:
let $birdName = $bird.find(".bird-name");
let oldName = $birdName.text();
$birdName.html(`...`)| $("[data-bird-id=" + birdId +"]").remove(); | ||
| renderBird(updatedBird); | ||
|
|
||
| $("[data-bird-id=" + birdId + "]")[0].scrollIntoView(); |
| //handling delete button of a bird | ||
| $("#birds").on("click", ".delete-bird", handleDeleteBird); | ||
|
|
||
| function renderAllBirds(birds) { |
| db.Bird.findById(req.params.birdId, function(err, bird){ | ||
| if (err) { | ||
| console.log(`Did not find bird id: ${req.params.birdId} in db`); | ||
| return; |
There was a problem hiding this comment.
You should probably send a response here, not just return from the function; as is, your app.js file would be waiting forever, for a response that would never come, if there were an error. Instead, you could send a 404.
There was a problem hiding this comment.
And this goes for all of your error-handling: you should send a response, not just console.log.
3 participants
Seeding was working for awhile. But later on it does not quite update in heroku any more.