Skip to content
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

Severin & Jan | Carets #18

Open
wants to merge 42 commits into
base: master
Choose a base branch
from
Open

Severin & Jan | Carets #18

wants to merge 42 commits into from

Conversation

JNEdrozo
Copy link

@JNEdrozo JNEdrozo commented Nov 8, 2017

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 reviewed the requirements and realized that movies and customers have a many-to-many relationship, so we needed an intermediate database, which we called rentals. Rentals has the foreign keys: customer_id and movie_id.
Describe a set of positive and negative test cases you implemented for a model. For the custom Rental model method (is_overdue?), we tested that it returns true if a movie is overdue and false if not overdue.
Describe a set of positive and negative test cases you implemented for a controller. For the rental controller checkout action, for good input we checked that it creates a new rental object with checkout date set to current date. For bad input, we checked that the response is a bad request.
How does your API respond when bad data is sent to it? Our API will render a JSON error message and send a status code such as bad_request or not_found.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. For the Rental model, we created a return_movie method that is called in the rental controller method #check_in.This allows the model to adjust the movie's available inventory and the customer's movies_checked_out_count, which is more appropriate and easily testable in the model (versus the controller).
Do you have any recommendations on how we could improve this project for the next cohort? A demonstration of Postman debugging/console use would be very helpful as well as a group discussion of how to read Postman test inputs (what our code will be tested against). There are also mistakes in the README, such as saying that overdue should return a list of customers with overdue movies, when rental objects that are overdue is actually what the tests expect returned.
Link to Trello Utilized whiteboards for task tracking
Link to ERD https://github.com/JNEdrozo/VideoStoreAPI/blob/master/erd.pdf

…create tests after updating movie_params private method
@CheezItMan
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and commit messages, only Jan doing the commits so I assume you paired on her computer.
Comprehension questions Check
General
Business Logic in Models Yes, mostly in the Rental model.
All 3 required endpoints return expected JSON data Perfect across the board!
Requests respond with appropriate HTTP Status Code Check
Errors are reported Check
Testing
Passes all Smoke Tests Perfect
Model Tests - all relations, validations, and custom functions test positive & negative cases Check, well done!
Controller Tests - URI parameters and data in the request body have positive & negative cases Nicely done
Optionals
POST routes use URI parameter and request body to add a new record to the database Check
GET /customers shows how many movies are checked out by a customer Check
GET /movies/:id shows updated inventory Check
Overall Very nice work, well done!

status: :not_found
)
return
elsif rental.count > 1

Choose a reason for hiding this comment

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

This block looks like it could either be a model method or using an ennumerable.

Movie.find_by(id: rental2.movie_id).available_inventory.must_equal movie_count
end

it "does not decrease the customers movies_checked_out_count if insufficient inventory to checkout" do

Choose a reason for hiding this comment

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

Good negative tests

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.

2 participants