The project2-movie-api from jamiemiller8

Error Handling

If I make a request for a movie that's not in your DB, the API just hangs. I.e. if I make a request to /movie/Princess%20Bride, it should maybe return an error that the movie isn't found.

Update Movie throws an error

When I send a PUT request to /movie/:id, I get the following error:

ReferenceError: req is not defined
    at updateMovie (/Users/zakk/ga/wdi/grading/02-project/jamie/controllers/movie.js:20:38)
    at Layer.handle [as handle_request] (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/route.js:137:13)
    at Route.dispatch (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/route.js:112:3)
    at Layer.handle [as handle_request] (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/layer.js:95:5)
    at /Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/index.js:281:22
    at param (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/index.js:354:14)
    at param (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/index.js:365:14)
    at Function.process_params (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/index.js:410:3)
    at next (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/index.js:275:10)
    at Function.handle (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/index.js:174:3)
    at router (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/index.js:47:12)
    at Layer.handle [as handle_request] (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/index.js:317:13)
    at /Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/index.js:284:7
    at Function.process_params (/Users/zakk/ga/wdi/grading/02-project/jamie/node_modules/express/lib/router/index.js:335:12)

Careful with id as a property

project2-movie-api/models/Genre.js

Line 5 in 4c7646f

id: Number,

Mongoose is automatically going to create a property called id, which may clash with this one

Root Route

project2-movie-api/index.js

Lines 11 to 13 in 4c7646f

    
           app.get("/", (request, response) => { 
        
               response.send("Hello World"); 
        
             });

Having a root route like this is a really great idea (great thinking!). Maybe consider having it send documentation to the user? I think that would be the best, but the other option is to redirect them to another route.

Add entity to route path

project2-movie-api/routes/genre.js

Line 9 in 4c7646f

router.get('/:name', genreController.getByName);

We typically get a resource by it's id, so I think people would expect something like /genre/:id to work. Then, to get a document by some other property, we'd add that to the URL (i.e. genre/name/:name). That might be a cleaner way of doing your paths.

Movie routes

project2-movie-api/routes/movie.js

Lines 5 to 24 in 4c7646f

    
           // get list of movies 
        
           router.get('/', movieController.index); 
        
           // get movie by id 
        
           router.get('/:id', movieController.getById); 
        
           //get movie by title 
        
           router.get('/title/:title', movieController.getByTitle); 
        
           //get genre of movie by the movie's id 
        
           router.get('/:id/genre', movieController.getGenre); 
        
           //create a new movie 
        
           router.post('/', movieController.createNewMovie); 
        
           //update a movie 
        
           router.put('/:id', movieController.updateMovie); 
        
           //delete a movie 
        
           router.delete('/:id', movieController.deleteMovie);

The comments you have here are great 👍
Getting a move by ID and by title is 🔥
The way you've done a movie's genre is great! Another cool way to do this is to populate the genre object

Repo organization tips

You've got a lot of stuff in the root of your respository. Consider reorganizing it a bit so the repo looks a little cleaner.

One thing you could do is move the json data and axios/ directory into a data/ directory.

Movie routes

project2-movie-api/routes/movie.js

Lines 5 to 24 in 4c7646f

    
           // get list of movies 
        
           router.get('/', movieController.index); 
        
           // get movie by id 
        
           router.get('/:id', movieController.getById); 
        
           //get movie by title 
        
           router.get('/title/:title', movieController.getByTitle); 
        
           //get genre of movie by the movie's id 
        
           router.get('/:id/genre', movieController.getGenre); 
        
           //create a new movie 
        
           router.post('/', movieController.createNewMovie); 
        
           //update a movie 
        
           router.put('/:id', movieController.updateMovie); 
        
           //delete a movie 
        
           router.delete('/:id', movieController.deleteMovie);

The comments you have here are great 👍
Getting a move by ID and by title is 🔥
The way you've done a movie's genre is great! Another cool way to do this is to populate the genre object

Person routes

project2-movie-api/routes/person.js

Lines 8 to 12 in 4c7646f

    
           //get person by name 
        
           router.get('/:name', personController.getByName); 
        
           //get person by id 
        
           router.get('/:id', personController.getById);

These two routes are identical! The fact that you've given them separate parameters wont make a difference, any route that would match against either of these will always just match against the first one (because it's first).

Maybe change the route to get a document by it's name to add name to the path?

router.get('/name/:name', personController.getByName);

Delete commented out code!

You don't want to have commented out code in your repo!

project2-movie-api/models/Movie.js

Lines 14 to 18 in 4c7646f

    
           //run_time: Number,  
        
           //revenue: Number, 
        
           //tagline: String, 
        
           //budget: Number, 
        
           //overview: String

Query Params

As a cool bonus, one thing you could do is implement query parameters for your urls. This would let you query for any document based on any property.

So to get by a movie name, you'd make a GET request to /movie?name=Princess%20Bride

We didn't cover these, so this would be a cool bonus!

Person Schema

project2-movie-api/models/Person.js

Lines 4 to 15 in 4c7646f

    
           const PersonSchema = new Schema ({ 
        
               results:[{ 
        
                   name: String, 
        
                   known_for:[{ 
        
                       id: Number, 
        
                       title: String, 
        
                       genre_ids:[{ 
        
                           type: Number 
        
                       }] 
        
                   }] 
        
               }] 
        
           });

This is a really interesting way of defining this model. It might make more sense to have name and known_for at the root of your schema, rather than nested under results. Also, for your genre_ids property, you probably want to use another ref.

Add setup instructions to the README

You currently don't have any documentation on how to get the project setup locally if someone (i.e. an instructor or a potential employer) wanted to do that.

Can you add setup instructions to your README?

jamiemiller8 / project2-movie-api Goto Github PK

project2-movie-api's People

Contributors

Watchers

project2-movie-api's Issues

Error Handling

Update Movie throws an error

Careful with id as a property

Root Route

Add entity to route path

Movie routes

Repo organization tips

Movie routes

Person routes

Delete commented out code!

Query Params

Person Schema

Add setup instructions to the README

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	app.get("/", (request, response) => {
	response.send("Hello World");
	});

	// get list of movies
	router.get('/', movieController.index);

	// get movie by id
	router.get('/:id', movieController.getById);

	//get movie by title
	router.get('/title/:title', movieController.getByTitle);

	//get genre of movie by the movie's id
	router.get('/:id/genre', movieController.getGenre);

	//create a new movie
	router.post('/', movieController.createNewMovie);

	//update a movie
	router.put('/:id', movieController.updateMovie);

	//delete a movie
	router.delete('/:id', movieController.deleteMovie);

	//get person by name
	router.get('/:name', personController.getByName);

	//get person by id
	router.get('/:id', personController.getById);

	//run_time: Number,
	//revenue: Number,
	//tagline: String,
	//budget: Number,
	//overview: String

	const PersonSchema = new Schema ({
	results:[{
	name: String,
	known_for:[{
	id: Number,
	title: String,
	genre_ids:[{
	type: Number
	}]
	}]
	}]
	});