Skip to content

Conversation

sahanamurthy
Copy link

@sahanamurthy sahanamurthy commented May 13, 2017

Video Store API

Congratulations! You're submitting your assignment!
If you didn't get to the functionality the question is asking about, reply with what you would have done if you had completed it.

Comprehension Questions

Question Answer
Explain how you came up with the design of your ERD, based on the seed data. We thought it would be best to treat the rentals as a join table since the customers and rentals have a many to many relationship.
List all the completed endpoints of your application. get “/customers”, to: “customers#index”, as: “customers”
get “/movies”, to: “movies#index”, as: “movies”
get “/movies/:title”, to: “movies#show”, as: “movie”
post “/rentals/:title/check-out”, to: “movies#checkout”, as: “checkout”
Describe a set of positive and negative test cases you implemented for a model. Positive test case: Movies model, testing that a movie is valid with all attributes. Negative test case: a movie is invalid without a title.
Describe a set of positive and negative test cases you implemented for a controller. Positive test case for Movies controller: movies show method returns a movie that exists. Negative test case: movies show method returns status content_not_found for a movie that doesn't exist
How does your API respond when bad data is sent to it? It sends a 204 no content status.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We created a custom model method for setting the available inventory to the inventory count when a movie is initialized if there is no existing available inventory. We tried to set it initially through defaults in the model but then realized it was cleaner to handle this as a custom model method.
Do you have any recommendations on how we could improve this project for the next cohort? None really
Link to Trello https://trello.com/b/15ZrxZys/videostoreapi
Link to ERD https://www.lucidchart.com/documents/edit/eb307c4d-4e6a-4be4-8a32-29176bf5b786

@droberts-sea
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes
General
Business Logic in Models You've got plenty of validations, which is a good start. One thing that jumps out at me is the code in MoviesController#checkout, can you think of any way to move this logic into the model somehow? Perhaps adding a method to the Movie model checkout(customer).
All 3 required endpoints return expected JSON data yes
Requests respond with appropriate HTTP Status Code yes - good work
Errors are reported no - When there's a problem, in addition to sending back a HTTP status code your API should send back some JSON with a list of errors, so that the client can understand what went wrong and correct their behavior. See the "Error Handling" section under "Wave 2" in the project README for an example of what this might look like.
Testing
Passes all Smoke Tests yes
Model Tests - all relations, validations, and custom functions test positive & negative cases yes - good work
Controller Tests - URI parameters and data in the request body have positive & negative cases Mostly there - CustomersController and the first two actions of MoviesController look really good! I'd like to see a little more coverage around failure cases for MoviesController#checkout. Interesting questions include "What happens if the movie or customer doesn't exist" and "What happens if the movie has 0 inventory left in stock".
Optionals
POST routes use URI parameter and request body to add a new record to the database yes
GET /customers shows how many movies are checked out by a customer yes
GET /movies/:title shows updated inventory yes
Overall

Great work overall! It sounds like the two of you worked hard on test coverage this time around, and it's definitely paid off. Your checkout action is missing some error handling and corresponding tests, but since this was optional anyway I'm not too worried about it. Keep up the good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants