Open
Conversation
Customers Index Controller Action & Tests
Videos Index Controller Action and Tests
…and ChangeDateFieldsToDateDatatype
…ntals#check_in action
…elds are returned for a valid rental
…ncrements available_inventory, and sets the due_date to nil
…ustomer or video does not exist
… rental_data params
…ry, and update due_date logic to video model method
…s_checked_out_count is < 1
…o total_inventory
…e_inventory would exceed total_inventory
…ed due_date from rental_params
…ion-tests Customer and Rental Validations Tests
Video Validations Tests
Rental Check Out Tests and Action
Rental Check In Tests and Action
Video Store APIMajor Learning Goals/Code Review
Functional Requirements
Overall Feedback
Additional FeedbackGreat work! Y'all built a fully functional API! Code Style Bonus AwardsWas the code particularly impressive in code style for any of these reasons (or more...?)
|
jmaddox19
reviewed
Jun 10, 2020
| end | ||
|
|
||
| Rental.check_out(rental.customer_id, rental.video_id) | ||
| # For some reason, we needed to reload the customer and video in order to get the updated counts |
There was a problem hiding this comment.
This is what I'd expect here. :)
You updated the values in the database but need to reload to see the new values reflected in the scope of the variables in this method.
| return | ||
| else | ||
| render json: { | ||
| errors: rental.errors.messages |
There was a problem hiding this comment.
There's actually no chance that the request could be bad at this point because the customer_id and video_id have already been verified on line 9. Hence you don't have a unit test that verifies this case.
| customer.reload | ||
| video.reload | ||
|
|
||
| rental.due_date = Time.now + 7.days |
There was a problem hiding this comment.
Minor: This could have been nice to add to the check_out method as well
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Assignment Submission: Video Store API
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.
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.
Reflection
/rentalsendpoint, we deduced that this could be a bridge entity between the Customer and Video entities, and that there would be a many-to-many relationship between Customer and Video through Rentals. We checked our assumptions about entity attributes against the seed data./customers&/videosendpoints? What does the time complexity depend on? Explain your reasoning.POST /rentals/check-inendpoint? What does the time complexity depend on? Explain your reasoning.