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

Alena & Aurora VideoStoreAPI #18

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

Conversation

auroralemieux
Copy link

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 looked at the columns and saw that rentals had a customer and a movie. Those were the only dependencies. We also looked at the fields that needed to be returned by the various methods and made sure they were in the schema.
List all the completed endpoints of your application. /customers, /movies/:title, /movies, /rentals/overdue, /rentals/:title/check-out
Describe a set of positive and negative test cases you implemented for a model. If a Rental doesn't have duedate, it won't save (validation). If there are no overdue rentals, then Rental.list_of_overdue returns an empty array.
Describe a set of positive and negative test cases you implemented for a controller. for Rentals, if the duedate is before today's date, it won't save, and will return an error. We also tested to make sure that a successful check-out creates a rental and decreases available inventory for the associated movie.
How does your API respond when bad data is sent to it? It returns error messages from the error hash.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. we have a custom model method in Rental called overdue?, which compares the rental's duedate with today. It was going to get called a lot and wasn't controller logic (interacting with DB or redirecting).
Do you have any recommendations on how we could improve this project for the next cohort? Maybe put part of the first chunk of optionals into the base requirement. We finished the base without feeling challenged on the testing (and there wasn't much to it anyway). It would have been nice to get a challenge without having to commit to the optionals. As it was, we didn't have time for the check-in method, but the checkout and overdue methods were great practice.
Link to Trello https://trello.com/b/Zy0PayiC/videostore-api
Link to ERD https://github.com/Spatterjaaay/VideoStoreAPI/blob/master/erd.pdf

Spatterjaaay and others added 30 commits May 9, 2017 13:33
…th pseudocode and created overdue custom method
…troller action we return all customers with overdue items
end

it "returns a json with count 0 if no movies" do
proc {

Choose a reason for hiding this comment

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

why is there a proc here?

@CheezItMan
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of requests with each partner contributing. Good commit messages.
Comprehension questions Check
General
Business Logic in Models Look like you started some methods like has_overdue. Good amount of business logic in the models. I do notice you have a lot of logic in RentalsController#index. Otherwise it looks good.
All 3 required endpoints return expected JSON data Check
Requests respond with appropriate HTTP Status Code Check
Errors are reported Check, good use of the Errors hash
Testing
Passes all Smoke Tests Check, except for checking that customers have movies checked out, and checking out/in a movie (as you noted in your PR)
Model Tests - all relations, validations, and custom functions test positive & negative cases Check, positive and negative tests. One test failed: "won't create rental without duedate" You've got a comparison operator that doesn't work.
Controller Tests - URI parameters and data in the request body have positive & negative cases Good tests for positive and negative cases. All working for the finished routes.
Optionals
POST routes use URI parameter and request body to add a new record to the database Check-out and check-in work.
GET /customers shows how many movies are checked out by a customer Check, but I would advise that the rental count be a calculated method, not an instance variable.
GET /movies/:title shows updated inventory Since check-out isn't working it's unable to be tested.
Overall Nice work overall. As you said in the comprehensive questions you got part of the way through the optionals and all the required routes.

@mcarson1111
Copy link

great ERD! It looks like you put a lot of initial planning and thought into it before beginning. That will always save you time and make for a better product in the end. Great job.

I agree that the number of commits is awesome, it took me a long time to catch on to how important that is.

I love the idea of the overdue being a model method, very clever; and well done getting to the optionals.

I also really appreciate how you were building tests and testing along the way. yay TDD!

I apologize for not having more specific feedback about the code; its been over a year since I've used these technologies, framework and language so I'm having a hard time navigating the files, but the things I mentioned above are going to be great foundational skills for whatever your working on/with.

I hope your other reviewer has more insight for code specific stuff.

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.

5 participants