-
Notifications
You must be signed in to change notification settings - Fork 25
Melissa & Naheed VideoStoreAPI #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
controller setup
setup database, models, controllers
fixed controller bugs
movie model testing, all pass
tests for customer models
fix relations in models
movie controller tests, all passing
customer controller tests passing
require :movie on params
add due date to rentals
add migration
schema changes, updated yml
refactor movies test
started rental model test
add rental controller tests
refactor rental controller tests
Rental tests passing
add check out method
add check in method
add customer to rental check/in/out methods, change column
add tests for check in and check out model methods
Video StoreWhat We're Looking For
Hello! Good work on this project-- If I adjust one line in your code, then all smoke tests from both waves pass! I see a lot of missing work around rental controller tests, though. Ultimately, the code that's written is great-- the finished unit tests and finished controller tests look great. I particularly like the refactoring y'all did to make the model logic more robust. I want to make sure y'all feel good about these learning goals, so let me know if you have any questions:
|
| private | ||
|
|
||
| def movie_params | ||
| params.require(:movie).permit(:title, :overview, :release_date, :inventory) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Rails when we used forms, params would come back populated with a nested structure that had movie in it. In this API, we won't have that nested structure, so we need to take out the .require('movie') bit. When we do this, then the rest of Wave 2 starts working :)
|
|
||
| checked_out = customer.movies_checked_out_count | ||
| customer.update(movies_checked_out_count: checked_out - 1) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! These are wonderful methods. I love that the logic is in Rental, and it very much delegates movie logic to movie, and customer logic to customer. This is perfect! Keep it up, this is great.
| end | ||
| it "decrements available_inventory by one" do | ||
| rental = Rental.new(check_out_date: "Wed, 29 Apr 2015 07:54:14 -0700", check_in_date: "Wed, 6 May 2015 07:54:14 -0700", due_date: "Wed, 6 May 2015 07:54:14 -0700", customer_id: customers(:one).id, movie_id: movies(:one).id) | ||
| binding.pry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Y'all had one last uncommented binding.pry
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
POST /rentals/check-inendpoint? What does the time complexity depend on? Explain your reasoning.