VideoStoreAPI - Edges - Katrina & Amber Lynn #18
Conversation
Video StoreWhat We're Looking For
|
|
|
||
| def find_rental(movie) | ||
| self.rentals.each do |rental| | ||
| return rental if rental.movie == movie |
There was a problem hiding this comment.
This could be simplified to:
def find_rental(movie)
self.rentals.find_by(movie: movie) || false
end| end | ||
|
|
||
| def movies_checked_out_count | ||
| return self.rentals.where(checkin_date: nil).count |
|
|
||
| def is_available? | ||
| return self.movie.available_inventory > 0 if self.movie | ||
| return false |
There was a problem hiding this comment.
Very minor quibble, but I think the logic here would be more clear in the opposite order:
def is_available?
return false unless self.movie
self.movie.available_inventory > 0
end|
|
||
| it "renders bad_request status code and returns JSON with error messages if the rental was not successfully saved" do | ||
|
|
||
| # QUESTION: not sure how to setup the controller test and make saving the rental fail. Checkout date and due date is not a required validation until trying to save upon checkout. The rental.save method would fail if checkout_date and due_date somehow returns nil or false upon checkout, and therefore return the error messages...but this is difficult to test in the controller tests using the post checkout_path bc valid params would always save the rental. |
There was a problem hiding this comment.
Yeah, this is a difficult one. I actually think this question could be answered by reconsidering how we define different error responses.
Right now we're trying to use a 400 Bad Request response to indicate that we weren't able to successfully save the Rental model with the check-in date. This makes sense, because we've only ever written code to handle saving failures as Bad Request.
However, that's not because Bad Request is inherently the correct response code when we fail to save to the database. It's because in those previous cases we were anticipating the failure was a result of validation errors -- ultimately due to bad data from the user.
In this case we can be pretty certain from the controller code, that the save would never fail due to validations (the only validations are "presence" for attributes which we explicitly set with values not derived from user data). As such, Bad Request is not the correct response for that case.
Instead, I would argue that a 500 Internal Server Error is the correct response code for that case. It's generally never a good thing to return a 500, usually it means there was an error that got raised but was never rescued. And that's pretty much the scenario we're in with this action.
Saving the Rental model to the database should never fail under any expected circumstances, so the only situations where it would fail are inherently exceptional (e.g. the database server is unreachable or cannot store more data).
It's probably possible to design a test to trigger one of these cases and verify that a 500 response is send back, but:
- the logic for doing that is actually part of Rails and not any code we've written ourselves
- it is, by definition, an unpredictable situation... so it's probably not worth building a specific test case for
3 participants
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.