Skip to content

Conversation

@raisahv
Copy link

@raisahv raisahv 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 came up with the design for the ERD based on the project requirements, we knew that we needed separate models for Movies and Customers because they each had distinct attributes. We also knew that rentals needed to be linked to Movies and Customers with a one-to-many relationship because a Movie could have many rentals associated with and a Customer could create many rentals.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning. The Big-O time complexity of our /movies endpoint is O(n) because for the Movies#index function we return a list of movies by iterating through all instance of Movie once. The Big-O time complexity of our /customers endpoint is O(nlogn) where n is the number of Customer instances created. This complexity depends on the time complexity of Ruby's sort method because we provide the option for the user to ask for a sorted list of Customers within the Customers#index function. Ruby uses Quicksort which has a time complexity of n log n.
What is the Big-O time complexity of the POST /rentals/check-in endpoint? What does the time complexity depend on? Explain your reasoning. We used PATCH for our /rentals/check-in route. The Big-O time complexity of the PATCH /rentals/check-in route is O(n) where n is the number of Rental instances created because we use Ruby's find_by method to iterate through all instances of Rental to update the rental.
Describe a set of positive and negative test cases you implemented for a model. For the Movie model we have a validation on the Movie title field, so in our validation tests, we tested to see if an instance of a movie will be created when given a movie title (the movie should be created) and if an instance of movie will be created if not given a movie title (the movie should not be created).
Describe a set of positive and negative test cases you implemented for a controller. For the Movies Controller show action we implemented a positive test case to see if the show action will return with a JSON and success method when called on a valid instance of Movie. We implemented a negative test case to see if the show action will return a JSON with an error if called on an invalid instance of Movie.
How does your API respond when bad data is sent to it? If bad data is sent to our API it responds with a status of :bad_request and response code 400. For requests involving finding a Movie and Customer Ids, bad data will have a response of :not_found and code 404.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. One of our custom model methods for the Customer method is sort_by_type. We chose to use this custom method to implement sorting of the Customer instances because the user could pass in a variety of sort_types and we want the method to be flexible to each sort type. This was too much business logic for the controller so we implemented the functionality as a class method in the model.
Do you have any recommendations on how we could improve this project for the next cohort? It would be helpful if Postman's required params were listed in the project requirements so that we could better understand how to write our code to pass the smoke tests without having to read through the JavaScript. Or instead, it could have been helpful to have instructors point out how these params should be formatted before the project started. Also, we believe that the check-in path is better implemented with a PATCH, rather than POST request to better adhere to principles of DRY code. We suggest that the project README recommend using a PATCH verb for this method.
Link to Trello https://trello.com/b/Dt25mX8n/video-store-api
Link to ERD https://www.lucidchart.com/invitations/accept/6a8bd6b2-42f2-495b-b01e-01e483fc53a7
Summary 👍

raisahv and others added 30 commits November 5, 2019 11:40
Movies index, show, create controller actions and tests
Rentals model and controller updates
Refactored tests and added validation for Movie
raisahv and others added 28 commits November 7, 2019 10:01
Custom model methods for Customers model tests and methods
Added model initialize tests for Customer, Rental, Movie
Refactored to controller and model test code
Inventory mgmt
Added Postman collection of tests for Optionals
@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 In general looks great! See code comments for notes on even more opportunities for moving code to model methodes.
All required endpoints return expected JSON data yup
Requests respond with appropriate HTTP Status Code yup
Errors are reported yes, at least for the checkin edge case I tested :)
Testing
Passes all Smoke Tests Yes! Except the ones that are confused because you chose to do the right thing and make checkin a patch :)
Model Tests - all relations, validations, and custom functions test positive & negative cases yes! so good!
Controller Tests - URI parameters and data in the request body have positive & negative cases yes! so good!
Overall Great job! Just a couple tips in the rubric above. You built an API! So awesome!

rental.checkout_date = Date.today
rental.due_date = Date.today + 7

if rental.movie.available_inventory > 0 && rental.save

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Determining what is/isn't business logic is definitely a slowly acquired taste and y'all did a great job.
I wanted to add that the logic to determine whether something is safe to execute (in this case whether the rental is safe to save) is safest in its own model method so it's very clear where/how that judgement call is being made.
This is definitely a level beyond what we've taught so far but wanted to throw it out there.

render json: { ok: false, "errors" => ["unable to create rental"]}, status: :bad_request
return
end
if Movie.find(params[:movie_id]) && Customer.find(params[:customer_id])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 9-12 could appropriately be pulled out into 1 or 2 model methods. but that's getting on the nit-picky side for sure.

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