Milestone3 and Milestone3.5 - #67
Conversation
|
Hi Lauren, |
sfwathen
left a comment
There was a problem hiding this comment.
Hey Lauren,
Sorry for the late response, I reached out to you on slack but you don't seem to have seen it. Could I have you resubmit milestone 4 after testing all of your routes and making sure they work. I ran into a few errors in your backend code and was unable to get a response from your get recipe by name route and your two put routes. If you have trouble testing please do reach out, postman should be a serious help though.
Additionally, your milestone 3.5 is not working 100%, particularly with the external recipes from Ryan's api. I gave some suggestions on what's going wrong and highly suggest you look back at the milestone 3.5 doc to get that working correctly. That being said, I would prioritize milestone 4 and 5 beforehand. Getting the external recipes working is not technically a requirement of milestone 5 so I would prioritize getting everything working with 4 then 5 so that you can get closer to being put on a project team!
Let me know if you have any questions.
Thanks,
Sam
| app.post("/recipe", async (req, res) => { | ||
| const { name, image, description, ingredients, instructions} = req.body | ||
| let recipe = new Recipe({ | ||
| name, | ||
| image, | ||
| description, | ||
| ingredients, | ||
| instructions | ||
| }) | ||
| try { | ||
| recipe = await recipe.save() | ||
| res.send(`Recipe for ${name} added to collection`) | ||
| } catch(error: any) { | ||
| res.status(500).send(error.message) | ||
| console.log(`error is ${error.message}`) | ||
| } | ||
| }); |
There was a problem hiding this comment.
this can and should go in your routes/recipe.ts file
| router.put("/:name/ingredient", async (req, res) => { | ||
| const name = req.params.name; | ||
| const ingredient = req.body.newIngredient; | ||
| const recipes = await Recipe.findOne({name: name}); | ||
| if (recipes){ | ||
| recipes.ingredients = [...recipes.ingredients, ingredient]; | ||
| await recipes.save(); | ||
| res.send("Ingredient added"); | ||
| } | ||
| else{ | ||
| res.send("Failed to add new ingredient"); | ||
| } | ||
| }); | ||
|
|
||
| router.put("/:name/instruction", async (req, res) => { | ||
| const name = req.params.name; | ||
| const instruction = req.body.newInstruction; | ||
| const recipes = await Recipe.findOne({name: name}); | ||
| if (recipes){ | ||
| recipes.instructions = [...recipes.instructions, instruction]; | ||
| await recipes.save(); | ||
| res.send("Instruction added"); | ||
| } | ||
| else{ | ||
| res.send("Failed to add new instruction"); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
these can and should go in your routes/recipe.ts file
| const express = require("express"); // 1. includes Express | ||
| const app: Express = express(); // 2. initializes Express | ||
| app.use(express.json()); | ||
| const mongoose = require('mongoose') |
There was a problem hiding this comment.
use the import syntax instead of require
|
|
||
| import recipeRoutes from "./routes/recipe"; | ||
|
|
||
| app.use("/recipe", recipeRoutes) |
There was a problem hiding this comment.
by adding "/recipe as the route here it will prepend it to all of the routes in the file. So if you put it here you should leave it off in all of the routes in the file. Otherwise you need to go to localhost:3001/recipe/recipe/... to access any of the routes, which is redundant
| const mongoose = require('mongoose') | ||
| const connection_url = 'mongodb+srv://newUser:newPassword@cluster0.66l23vd.mongodb.net/RecipesDB?retryWrites=true&w=majority' | ||
|
|
||
| import recipeRoutes from "./routes/recipe"; |
There was a problem hiding this comment.
put all imports at the top of your file
| } | ||
|
|
||
| function RecipePage() { | ||
| const [externalRecipes, setExternalRecipes] = useState<Recipe[]>(recipeData) |
There was a problem hiding this comment.
ideally, we don't want to fetch all of our recipes when we go to each individual recipe page, instead we want to only grab the one who's name is specified in the urlParams. This is why in the docs we have a useEffect with a conditional inside. We either want to make a call to the database if we are finding an external recipe, or search through our local data using .find if its local.
| useEffect(() => { | ||
| fetch("https://localhost:3001/recipe") | ||
| .then((res) => res.json()) | ||
| .then((data) => setExternalRecipes([...externalRecipes, ...data])) |
There was a problem hiding this comment.
if we instead had a useState variable to hold the single recipe for the page, we could call that setRecipe function with the result of your call to the find recipe by name route you made on the backend.
| let param = useParams() | ||
| console.log(param.name) | ||
|
|
||
| let recipe = externalRecipes.find((r) => r.name === param.name) |
There was a problem hiding this comment.
we should do this inside of the conditional of the useEffect, as shown in the milestone3.5 docs. Because useEffect is asynchronous these lines of code are running before externalRecipes is updated, which is why the pages of your external recipes are throwing errors.
| const [allIngredients, setAllIngredients] = useState( | ||
| recipe ? recipe.ingredients : [] | ||
| ) | ||
|
|
||
| const [allInstructions, setAllInstructions] = useState( | ||
| recipe ? recipe.ingredients : [] | ||
| ) |
There was a problem hiding this comment.
Always put all the useState declarations at the top of the function together.
| interface Recipe { | ||
| name: string | ||
| image: string | ||
| description: string | ||
| } |
There was a problem hiding this comment.
you may have noticed that you're used this interface or something similar in several places in your code. Instead of repeating all of that code, we can put this in a separate file, call it types.ts or something similar, then import this interface wherever you need it!
Milestone: Milestone 3 and Milestone3.5
Developer: Lauren Allen
Pull Request Summary
To turn in milestone 3 and updated to turn in milestone 3.5
Pull Request Checklist
Reviewer Checklist - PULL REQUEST REVIEWER ONLY
IMPORTANT: The rest of the sections in this checklist should only be filled out by authorized pull request reviewers. If you are the individual template contributor, do not fill out the rest of the fields or check the boxes.
NOTE: Milestones can only be completed when all boxes are checked.