Skip to content

Conversation

@ChantalDemissie
Copy link

@ChantalDemissie ChantalDemissie commented May 16, 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 decided to make a model for customer, movie and rental and we knew that rentals would depend on a customer id and movie id. We also knew that rentals belonged to both customers and movies.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning. 0(n), where n is the number of customers/movies. the time complexity depends on the quantity of customers or movies, the more movies or customers the longer it would take.
What is the Big-O time complexity of the POST /rentals/check-in endpoint? What does the time complexity depend on? Explain your reasoning. 0 log(n) where n is the id of the movie and customer we are searching for. We are using a binary search to index the database. It depends on how many movies/customers are in the database, the more that there are the longer it takes to search through the database.
Describe a set of positive and negative test cases you implemented for a model. We tested the validations for movies. We tested that its valid if it has a title and invalid if it does not.
Describe a set of positive and negative test cases you implemented for a controller. For rentals we have a test that creates a new rental with valid rental data. In contrast it does not create a rental with invalid data
How does your API respond when bad data is sent to it? it returns a bad request status code and depending on the action a message of what specific data was wrong.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. TBD...we decided to keep check_in and check_out in the rentals controller with the intention to factor it into the rental model.
Do you have any recommendations on how we could improve this project for the next cohort? More practice with postman before, it had been many weeks since we first saw it. Directions clarification would be helpful, some instructions were unclear.
Link to Trello https://trello.com/b/bGAUtQXu/videostoreapi-steph-chantal
Link to ERD https://www.lucidchart.com/invitations/accept/be40fb42-a632-4030-88c8-9a59b7ca0557
Summary This was fun! a nice surprise pairing!

@tildeee
Copy link

tildeee commented May 23, 2019

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene x
Comprehension questions x
General
Business Logic in Models x
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 all required tests passing
Model Tests - all relations, validations, and custom functions test positive & negative cases It might be nice to have more tests around details, such as for Movie's available_inventory method, tests around checking: does it work when there are rentals with no return date, with a mix, etc.
Controller Tests - URI parameters and data in the request body have positive & negative cases x
Overall

Well done on this project!

I want to praise the tests on this project, particularly the controller tests. You all did a great job with the controller tests; I thought they had very good code style and were very clean and clear to read. I love the form of taking a valid set of data, turning it invalid in the tests, and then how all of the assertions were organized together.

Overall, great work

class RentalsController < ApplicationController
def check_out
rental = Rental.new(rental_params)
rental.return_date = Time.now + (60 * 60 * 24 * 7)
Copy link

@tildeee tildeee May 23, 2019

Choose a reason for hiding this comment

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

It might be worth refactoring this logic out into the Rental model, maybe as an instance method that called rental.set_default_return_date

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