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

Aruna && Monalisa #7

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

Aruna && Monalisa #7

wants to merge 34 commits into from

Conversation

aruna79
Copy link

@aruna79 aruna79 commented May 11, 2018

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 used the customer and movie fields we had currently available, and knew we would like to create a join table to contain rentals since it would otherwise require a many-to-many relationship between customers and movies.
Describe a set of positive and negative test cases you implemented for a model. A rental could be created with valid data, and could not be created with invalid data.
Describe a set of positive and negative test cases you implemented for a controller. In the movie controller it returns success if the movie is found and returns 404 if the movie is not found
How does your API respond when bad data is sent to it? If bad data, responds with bad request. If data doesn't exist responds with not found.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We made a available inventory method in our movies model. This is used before a movie is create which takes the inventory and sets the default available inventory to be the same. We wanted all of our movie objects to have the same fields and for it to be automatically set.
Do you have any recommendations on how we could improve this project for the next cohort?
Link to Trello https://trello.com/b/STrxbjiM/videostoreapi
Link to ERD https://www.lucidchart.com/documents/edit/9a94a5db-d7a8-45d7-9b5a-a2052fd1d64c/0

@CheezItMan
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits, and good commit messages, both teammates contributing.
Comprehension questions Check
General
Business Logic in Models You've got the checkout business logic in the rental controller rather than the models.
All required endpoints return expected JSON data Check
Requests respond with appropriate HTTP Status Code Check
Errors are reported Check
Testing
Passes all Smoke Tests Check
Model Tests - all relations, validations, and custom functions test positive & negative cases Check
Controller Tests - URI parameters and data in the request body have positive & negative cases Check
Overall Nice work, you hit all the learning goals. Excellent job!

validates :release_date, presence: true
validates :inventory, presence: true

def available_inventory

Choose a reason for hiding this comment

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

What purpose does this method serve?

return
end
rental = Rental.new(rental_params)
rental.checkout_date = Date.today

Choose a reason for hiding this comment

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

You're doing business logic in the controllers that should be done in model methods.

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.

3 participants