Skip to content

Conversation

@msfinnan
Copy link

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 looked at the seed data and thought about the relationships between the movies, customers, and rentals and decided that rentals belongs to movies and customers.
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) where n is the number or movies or customers in the database. This is for an unsorted display, adding a query parameter that sorts would change the Big O notation, depending on what the Big O is for .order.
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(n) because we are using find_by(customer id AND movie id) and we think find_by probably will iterate through all instances of rental to find the match.
Describe a set of positive and negative test cases you implemented for a model. In the customer model we checked to make sure a model is valid with a name, and not valid without a name.
Describe a set of positive and negative test cases you implemented for a controller. For the movie show action test, we tested that it can show a movie that has a valid id and will respond with not found and an errors hash if the id is not valid.
How does your API respond when bad data is sent to it? With an errors hash and a status not found.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We do not have any custom model methods.
Do you have any recommendations on how we could improve this project for the next cohort? Some of the wave requirements were vague, like what it means to check in and out a movie - but this was okay because we decided on it together.
Link to Trello We did not use a Trello for this project, we worked through the waves and did all the work together (no home work).
Link to ERD https://www.lucidchart.com/documents/edit/aa7f714c-0a33-4421-abc0-894a928c6ff3/0
Summary

msfinnan and others added 30 commits May 14, 2019 11:46
@tildeee
Copy link

tildeee commented May 24, 2019

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene x
Comprehension questions x
General
Business Logic in Models There's opportunity to pull some out of the controllers
All required endpoints return expected JSON data x
Requests respond with appropriate HTTP Status Code x
Errors are reported x
Testing
Passes all Smoke Tests x
Model Tests - all relations, validations, and custom functions test positive & negative cases x
Controller Tests - URI parameters and data in the request body have positive & negative cases x
Overall

Well done on this project, you two! FANTASTIC work on the sorting and pagination! The code is well-written, clean, and thorough.

I have a few comments on small places where I think code could be refactored.

Other than that, this is a great submission. Great work!

def checkout
movie = Movie.find_by(id: params[:movie_id])
customer = Customer.find_by(id: params[:customer_id])
rental = Rental.new(customer_id: params[:customer_id], movie_id: params[:movie_id], checkout_date: Date.today, due_date: Date.today + 7)
Copy link

Choose a reason for hiding this comment

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

It might be nice to utilize strong params method for making a new Rental!

def checkout
movie = Movie.find_by(id: params[:movie_id])
customer = Customer.find_by(id: params[:customer_id])
rental = Rental.new(customer_id: params[:customer_id], movie_id: params[:movie_id], checkout_date: Date.today, due_date: Date.today + 7)
Copy link

Choose a reason for hiding this comment

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

Determining the default checkout_date and due_date might be interesting business logic to pull into a model method!

elsif rental.save
customer.movies_checked_out_count += 1
customer.save
movie.available_inventory -= 1
Copy link

@tildeee tildeee May 24, 2019

Choose a reason for hiding this comment

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

Same as above-- the logic for incrementing movies_checked_out_count might be a good Customer instance method, and incrementing available_inventory might be good to pull into Movie

It ends up being a slim method, but I rather like the idea of this line looking like:

movie.check_out

and in movie.rb

def checkout
  self.available_inventory -= 1
end

it "sorts by id when not given sort query params" do
Movie.destroy_all
movie1 = Movie.create(title: "Test 1", inventory: 1)
movie1.id = 1
Copy link

Choose a reason for hiding this comment

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

Hm, it would be nice if this test could be refactored to not rely on manually setting movie1's id...

{
title: "Titanic",
overview: "Romantic & sad",
release_date: 1997 - 12 - 19,
Copy link

Choose a reason for hiding this comment

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

Watch out! I think that this literally evaluates to integer arithmetic, so this is 1966 hahaha

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