-
Notifications
You must be signed in to change notification settings - Fork 25
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
Tor & Wenjie - VideoStore - Octos #6
base: master
Are you sure you want to change the base?
Conversation
Implemented tests for customer model validations
Implemented tests for controller movies, all passed
… rental, wrote tests for additional validations
Modified tests for rental controller Added another column in rental model
…column for customers
Implemented tests for business logic
…nventory and wrote model tests
Video StoreWhat We're Looking For
|
def same_name_and_same_phone | ||
customers = Customer.where(name: name) | ||
|
||
if customers.any? {|customer| customer.phone == phone} |
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.
Rails supports this natively:
validates :phone, uniqueness: { scope: :name }
def has_enough_inventory? | ||
unless self.movie.available_inventory > 0 | ||
errors[:quantity] << 'Not enough inventory for this rental' | ||
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.
Nice work on this custom validation
def self.find_checked_out_movie(movie_id, customer_id) | ||
rental = Rental.where(movie_id: movie_id, customer_id: customer_id, check_in_date: nil) | ||
return rental.empty? ? nil : rental.first | ||
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.
Good use of model methods here. I might take it a step further and have something like Rental.checkout(movie_id, customer_id)
and Rental.checkin(movie_id, customer_id)
, which encapsulate all of the work in one place. You could even wrap the whole thing in a transaction.
def self.update_movie_and_customer(rental) | ||
|
||
if rental.check_in_date.nil? | ||
new_inventory = rental.movie.available_inventory - 1 |
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.
This feels like it should be an instance method, since it requires an instance to run.
|
||
it 'returns bad request and error text for bad data' do | ||
rental_data = { | ||
movie_id: movies(:one).id, |
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.
What if the movie doesn't have enough inventory left in stock?
it "return bad request and error if the rental DNE" do | ||
rental = rentals(:one) | ||
rental_data = { | ||
movie_id: rental.movie_id + 100, |
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.
What about a rental that has already been checked in? What if there are multiple rentals for the same movie-customer pair?
it "returns nil if the rental DNE" do | ||
rental = rentals(:one) | ||
movie_id = rental.movie_id + 100 | ||
customer_id = rental.customer_id |
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.
What if the rental has been checked in?
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