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

Karinna & Kat - VideoStoreAPI - Octos #8

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

Conversation

kseastman
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 what the JSON included, and created the models with those attributes except for registered_at for customers, which we figured we could save after creation into the "created_at" timestamp column, and display in JSON using a node.
Describe a set of positive and negative test cases you implemented for a model. Positive cases for Movie#request_query (but it's in the application_record model): it works when provided a params hash with sort, p and n, and in any combination (including none). Negative cases: if the params hash had a key we did not want, it was ignored, and if sort p or n had an invalid value, it would be ignored.
Describe a set of positive and negative test cases you implemented for a controller. Movies#index positive cases: responds with success, sends a JSON with a collection of movies and their required fields. Negative cases: responds with not_found when no available movies, and includes errors with the key "movie" in them.
How does your API respond when bad data is sent to it? We respond with bad request and return nil from our method, which renders a JSON hash with errors in it.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We already talked about request_query, which was a good one and did this thing, but we also used a custom method for movie#available which calculated the available copies for an instance of movie using the inventory for that movie minus the count of currently rented movies that had not been checked back in yet.
Do you have any recommendations on how we could improve this project for the next cohort? Go over the smoke tests together in class in a cursory way together as a class.
Link to Trello https://trello.com/b/dv53mb5N/videostore
Link to ERD https://www.lucidchart.com/documents/view/e7bd380e-e2ca-4a65-91db-9899da54421c/0

kseastman and others added 30 commits May 7, 2018 12:51
…because it's part of the assignment and we thought it was cute.
@kariabancroft
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good
Comprehension questions Yes
General
Business Logic in Models Yes - you did a really nice exploring different ways to move logic in to the models for this
All required endpoints return expected JSON data Yes - nice job exploring rabl for the JSON responses
Requests respond with appropriate HTTP Status Code Yes
Errors are reported Yes
Testing
Passes all Smoke Tests Yes
Model Tests - all relations, validations, and custom functions test positive & negative cases Yes! Nice job
Controller Tests - URI parameters and data in the request body have positive & negative cases Yes, good
Overall You did a really nice job with this. I especially like the way you built common methods to handle building certain objects using the sorting and paging parameters.


instance = self.create!(cust_data)

if hash["registered_at"]

Choose a reason for hiding this comment

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

This seems like good logic, but why is it after the create!?

def index
@customers = Customer.request_query(params, Customer)

if @customers.empty?

Choose a reason for hiding this comment

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

Generally an empty set of data isn't an error, but rather it would just return an empty array indicating that there was no data available

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