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

Julia & Victoria -- Carets #12

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

Julia & Victoria -- Carets #12

wants to merge 32 commits into from

Conversation

vsawchuk
Copy link

@vsawchuk vsawchuk 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 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. The movies controller can find a movie with a valid id, and will return a 404 status code if the movie id was invalid.
How does your API respond when bad data is sent to it? Typically we send back a 400 or 404 response code depending on the situation.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We added the checkout_count as an instance method in the Customer model because we didn't want to store this logic in the controller nor as a field in our customers table in Postgres. This reduces storage requirements for our database.
Do you have any recommendations on how we could improve this project for the next cohort?
Link to Trello We don't have one ¯_(ツ)_/¯ (ps this emoji is mostly joking, but we just followed the directions in order)
Link to ERD Customers -
julia_victoria_erd

|

vsawchuk and others added 30 commits November 6, 2017 11:20
…index method in customers controller to use this method
@CheezItMan
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits, good commit messages, both team members contributed
Comprehension questions Check
General
Business Logic in Models Good number of method in the models dealing with checkout, inventory etc.
All 3 required endpoints return expected JSON data Check
Requests respond with appropriate HTTP Status Code Check
Errors are reported Check
Testing
Passes all Smoke Tests Yep, plus the extras
Model Tests - all relations, validations, and custom functions test positive & negative cases You're missing some negative tests and tests for edge cases, left some notes in your code.
Controller Tests - URI parameters and data in the request body have positive & negative cases Overall good, left some notes in your code.
Optionals
POST routes use URI parameter and request body to add a new record to the database Well done!
GET /customers shows how many movies are checked out by a customer Check!
GET /movies/:id shows updated inventory Check
Overall Well done, some minor issues with your testing, but very nice work especially on the extras.


describe Customer do

it "a customer object can be saved to the database" do

Choose a reason for hiding this comment

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

So no validations for Customers?

class Customer < ApplicationRecord
has_many :rentals

def self.index_customers

Choose a reason for hiding this comment

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

Interesting method to create the JSON, why not use a serializer?


describe Movie do

it "a movie object can be saved to the database" do

Choose a reason for hiding this comment

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

negative tests? What about when the inventory, or available inventory is 0. Can you create a movie with negative inventory?

body["id"].must_be_instance_of Integer
end

it "cannot save a duplicate movie (same title)" do

Choose a reason for hiding this comment

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

What about if the value posted is missing some needed fields?

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