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

Queues - Kayla and Laura - VideoStoreAPI #26

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

Queues - Kayla and Laura - VideoStoreAPI #26

wants to merge 34 commits into from

Conversation

ghost
Copy link

@ghost ghost commented May 15, 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 read over the instructions and looked at the seed file. After that we decided we needed a join table to connect movies and customers.
List all the completed endpoints of your application. /customers, /movies, and movies/:title
Describe a set of positive and negative test cases you implemented for a model. We tested if a customer with all the correct validations was valid and we tested if a customer with 'nil' as a name would not be valid.
Describe a set of positive and negative test cases you implemented for a controller. We made sure that the response from the movies url had all the correct required keys and, in that case that no data existed, the response from the url will be 'not found'.
How does your API respond when bad data is sent to it? Our API sends back a JSON message based on the error.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We do not have any.
Do you have any recommendations on how we could improve this project for the next cohort? We cannot think in one right now :)
Link to Trello https://trello.com/b/m2nMOIsT/video-store
Link to ERD We did our ERD on paper.

Kayla and others added 30 commits May 9, 2017 15:33
Created Rails API app.
Established model relationships.
Set up controllers.
Worked with Noam to send correct attributes based on the request.
If request is for all movies only title and release date are sent.
If request is for a specific movie overview, inventory, and available inventory are also sent.
A not found request returns an error message and correct status code.
@PilgrimMemoirs
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Well Done
Comprehension questions Well Done
General
Business Logic in Models n/a -
All 3 required endpoints return expected JSON data Well Done
Requests respond with appropriate HTTP Status Code Mostly Good - with indexes, it's common to return an empty array instead of giving an error message.
Errors are reported Well Done
Testing
Passes all Smoke Tests Well Done
Model Tests - all relations, validations, and custom functions test positive & negative cases Well Done - An additional Movie validation and test could have been for the inventory or or available_inventory be less than 0.
Controller Tests - URI parameters and data in the request body have positive & negative cases Well Done
Optionals
POST routes use URI parameter and request body to add a new record to the database n/a
GET /customers shows how many movies are checked out by a customer n/a
GET /movies/:title shows updated inventory n/a
Overall
Nice work getting the required tasks completed to expectations. Something to keep in mind, it's best to return an empty array and an ok status when calling for a collection of something (like with '/customers' and '/movies').
Be mindful of what kind of variables you're using - I see a lot of instance variables in the controller- we used them in rails when we were developing a web application and we wanted to pass data to the views. But we're not doing that anymore, so a local variable is more appropriate.
I know the optional post isn't complete, but a couple observations on what is there: It looks like you have controller code in your CustomerMovie model. Methods defined in movie are instance methods, meaning you can use the keyworld 'self' to refer to the instance you are calling it on, so you shouldn't have to call anything on Movie. The CustomerMovies controller also has an commented out initialize method, which is something you'd never have in the controller.

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.

4 participants