Skip to content

Conversation

@geomsb
Copy link

@geomsb geomsb commented Nov 8, 2019

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 reviewed the seed data to see what fields were included and we read the project instructions to identify the different objects and their relationships.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning. O(n) for both because we are iterating over each movie or customer to return the json.
What is the Big-O time complexity of the POST /rentals/check-in endpoint? What does the time complexity depend on? Explain your reasoning. O(1) because we are looking up a specific rental and then we are creating the json with the information that we already have that is the id, and we don't have to iterate over the entire list of Rental.
Describe a set of positive and negative test cases you implemented for a model. Method: movie_avail? Positive: if a movie had available inventory greater than zero it would return true. Negative: if a movie had not available inventory greater than zero it would return false.
Describe a set of positive and negative test cases you implemented for a controller. In movie show controller. Positive: it would return a json with the information. Negative: it would return not found if the movie does not exist, with an error message.
How does your API respond when bad data is sent to it? With bad request
Describe one of your custom model methods and why you chose to wrap that functionality into a method. For rentals we did the set_due_date method which calculates seven days after the checkout. We put it on a method because that logic shouldn't be in the controller.
Do you have any recommendations on how we could improve this project for the next cohort? The smoke tests could be improved and it would be great to have a debug session using Postman.
Link to Trello We did it manually
Link to ERD https://www.lucidchart.com/documents/edit/6bb9f729-374f-44b3-9696-9d0ffa73557d/0_0?shared=true
Summary Building API actions are similar to building CRUD actions in web applications.

@jmaddox19
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene great!
Comprehension questions yup
General
Business Logic in Models Not as much as would be beneficial. This is most noticeable in the RentalsController. Breaking this code out into model methods is an area for improvement.
All required endpoints return expected JSON data checkout doesn't seem to work for me. Looks like it's an issue with avavilable_inventory not being defined on the movies from the seeds? Because of that, I can't test checkin either.
Requests respond with appropriate HTTP Status Code checkout doesn't seem to work for me. Because of that, I can't test checkin either. (see row above)
Errors are reported checking in a movie that hasn't been checked out results in an awkward error message in the API because that edge case wasn't handles. Creating a movie exposes error messages well :)
Testing
Passes all Smoke Tests 5 failing
Model Tests - all relations, validations, and custom functions test positive & negative cases no validation tests
Controller Tests - URI parameters and data in the request body have positive & negative cases yes
Overall Code looks good overall! It seems like there's some seeding issue that is unfortunately prohibiting me from verifying a lot of behavior. There are a couple tips in the rubric above. Still, you built an API! So awesome!

For the error I alluded to above, the exact error I'm seeing is:
NoMethodError (undefined method >=' for nil:NilClass): app/models/movie.rb:8:in movie_avail?'
app/controllers/rentals_controller.rb:15:in `check_out'

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.

3 participants