Sam & Hannah -- Octos #17

samanthaberk · 2018-05-11T23:00:42Z

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 knew that movies would have customers and vice versa, but through rentals from the seed data, so we decided to make rentals act like a join table. Based off of the API descriptions, we knew it would be more than a join table, so we used a has_many :through relationship.
Describe a set of positive and negative test cases you implemented for a model.	For decrement_movies_count we tested that the customer_movie_count would decrease by 1. For the negative case, we tested that it would not decrease if movie_count was already at 0.
Describe a set of positive and negative test cases you implemented for a controller.	On the movies controller, for the show action, we tested in a positive case that it would return the json for a specific movie that exists, and that it would return errors when the movie doesn't exist.
How does your API respond when bad data is sent to it?	We send back clear error messages describing what is wrong.
Describe one of your custom model methods and why you chose to wrap that functionality into a method.	We chose to wrap increment_available_inventory for the Movie class in a method so we could easily call it in the controller for the update method when a movie is checked in.
Do you have any recommendations on how we could improve this project for the next cohort?	It took us some experimenting to understand how to run all tests in Postman. Based on our in-class intro, it seemed like we would need to add data in params which took up time. More guidance on this would have been helpful.
Link to Trello	https://trello.com/b/vDg4DddI/videostoreapi
Link to ERD	https://www.lucidchart.com/documents/edit/084a5e22-5692-4a76-9356-3cac1845eebf/0

…method TDD

…ventory; edited inventory validation to no longer be greater than zero

… pass postman smoke tests

…es controller tests

…idation for available inventory to be only on create

…movie, wrote tests, commented out method as it isn't working

…hod from model to controller

… that passes said tests (in that order)

…. also wrote tests

kariabancroft · 2018-05-17T06:36:04Z

Video Store

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	Good
Comprehension questions	Yes
General
Business Logic in Models	Yes - did a good job setting and updating some necessary fields
All required endpoints return expected JSON data	Yes
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, good
Controller Tests - URI parameters and data in the request body have positive & negative cases	Yes, nice job with tests for rental on inventory as well as bad movie and customer ids
Overall	You did a nice job with this. The main logic that I thought could be consolidated was some of the inventory and status updates on all of the objects related to checking in and checking out a rental. This would simplify the controller actions and ensure that this logic is always grouped together.

kariabancroft · 2018-05-17T05:50:11Z

app/models/customer.rb

+  validates :name, presence: true
+  validates :movies_checked_out_count, presence: true, numericality: { only_integer: true }
+
+  def decrement_movies_checked_out_count


Is it known that this will do nothing if there aren't any checked out?

kariabancroft · 2018-05-17T05:53:14Z

app/models/movie.rb

@@ -0,0 +1,27 @@
+class Movie < ApplicationRecord
+  before_validation :set_available_inventory_default, on: :create


Nice use of the before_ action. You could also use before_create for the same purpose.

kariabancroft · 2018-05-17T06:13:56Z

app/controllers/rentals_controller.rb

+    movie = Movie.find_by(id: rental_params[:movie_id])
+    customer = Customer.find_by(id: rental_params[:customer_id])
+
+    if movie.nil?


What if the customer doesn't exist?

kariabancroft · 2018-05-17T06:17:52Z

app/controllers/rentals_controller.rb

+    end
+
+    if rental.save
+      movie.decrement_available_inventory


It might be logical to move all of this updating logic into a rental model method. One thing to keep in mind in the future is that when an action requires many model updates, you may want to combine these into a transaction. If the rental save succeeds, then you also want to ensure that the movie and customer object updates work, otherwise you could end up in an odd state where the movie inventory is updated, but the customer's isn't.

kariabancroft · 2018-05-17T06:27:57Z

test/models/rental_test.rb

+require "test_helper"
+require 'pry'
+
+describe Rental do


You should also have a test that verifies the logic written in the before_create block

samanthaberk and others added 30 commits May 7, 2018 14:41

create new rails api app

786aa86

create customers controller

f3beb43

Create customer model

1886154

Add relations to customer class

2e52676

added in movies controller and model

3421d2d

resolved merge cnflict in schema

d659e74

added rental model and foreign key relations

d9e2415

added rentals controller, added relations to rentals

625a9a5

Add /zomg route and confirm it works

45fcc91

added model validations to customer and movie

cbc6002

added and passing customer validation tests

41168b6

Build tests for validations on movie model

77e01eb

write some tests

f0987f0

wrote relations tests

22c1bf7

resolved merge conflict in yml files

9f94646

revised relations tests, passing

0f98b40

added in negative test cases for relations

aed9342

add test for if the movie has not been renter

0862448

Merge branch 'movies-tests'

b5e7826

wrote relations tests for rentals model

eac8b4b

wrote customer#index controller tests - added in route and filled in …

4470c5e

…method TDD

added in test for if no customers

6f69770

Merge branch 'get_customers'

56339d6

Add controller tests to movies#index

bd9dc0a

Add before block so not repeating code

826af96

added migrations/updated movie.rb to account for setting avialable in…

7dabaeb

…ventory; edited inventory validation to no longer be greater than zero

added in negative test cases for rental.rb

eeefb89

change order for sort on movies controller test

b5ca545

revised what customers#index removes

53b18ab

All tests passing

4b66bbe

samanthaberk and others added 24 commits May 8, 2018 10:38

Remove Zomg and condense routes

63cc4c3

Merge branch 'master' of https://github.com/samanthaberk/VideoStoreAPI

0b66246

Write initial tests for movie#show

8eebda3

movies#show working with passing tests

5ae0389

Merge branch 'master' of https://github.com/samanthaberk/VideoStoreAPI

a30a031

added in movies#create tests

ff12a1b

added route and method for movies#create - passing all controller tests

019c22b

change status for successful movies post from :created to :success to…

63d446f

… pass postman smoke tests

Add checkout date and due date with default values to Rentals table

8098059

Merge branch 'master' of https://github.com/samanthaberk/VideoStoreAPI

0af344a

Write initial controller tests for rentals#checkout

5148b7f

Write routes for checkout and checkin and code for checkout that pass…

6f78be5

…es controller tests

Change checkout and checkin routes to match smoke test requirements

244e3ca

added in decrement_available_inventory tests; modified the before val…

1c6de97

…idation for available inventory to be only on create

wrote decrement_avail_inv, passing all tests

494b63c

revised rental#create to accomodate for avail_inv; wrote find_rental_…

15408d2

…movie, wrote tests, commented out method as it isn't working

revised rental#create, passing all controller tests; moved helper met…

25264f8

…hod from model to controller

Write tests for increment_available_inventory on Movie class and code…

19dfd05

… that passes said tests (in that order)

Write intital controller tests for checkin

bd13df2

write rentals#create tests and code that passes those tests

d6ba637

added customer model increment and decrement movies checked out count…

d2a3620

…. also wrote tests

fixed failing tests

bbfec9f

removed 2x private methods for finding movie and customer

8670626

passing all smoke tests - added in customer.save and movie.save

06c4359

kariabancroft reviewed May 17, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sam & Hannah -- Octos #17

Sam & Hannah -- Octos #17

samanthaberk commented May 11, 2018

kariabancroft commented May 17, 2018

kariabancroft May 17, 2018

kariabancroft May 17, 2018

kariabancroft May 17, 2018

kariabancroft May 17, 2018

kariabancroft May 17, 2018

		@@ -0,0 +1,27 @@
		class Movie < ApplicationRecord
		before_validation :set_available_inventory_default, on: :create

Sam & Hannah -- Octos #17

Are you sure you want to change the base?

Sam & Hannah -- Octos #17

Conversation

samanthaberk commented May 11, 2018

Video Store API

Comprehension Questions

kariabancroft commented May 17, 2018

Video Store

What We're Looking For

kariabancroft May 17, 2018

Choose a reason for hiding this comment

kariabancroft May 17, 2018

Choose a reason for hiding this comment

kariabancroft May 17, 2018

Choose a reason for hiding this comment

kariabancroft May 17, 2018

Choose a reason for hiding this comment

kariabancroft May 17, 2018

Choose a reason for hiding this comment