trees: Mira & Natalie by NatalieTapias · Pull Request #18 · Ada-C12/video-store-api

NatalieTapias · 2019-11-08T22:16:18Z

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 decided that a rental consisted of a customer_id, movie_id, checkout date, and due date based on the user stories that we received. We also managed to perform a few migrations to track data that the smoke tests expected to be present (movies_checked_out_count for customer and currently_available for movie). We decided that a customer may have many rentals. A movie may be rented many times. A rental only has one customer and one movie.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning.	O(1) I believe it is a constant time complexity because we are returning the entire table. We are not performing a search or a lookup for customers and movies endpoints.
What is the Big-O time complexity of the `POST /rentals/check-in` endpoint? What does the time complexity depend on? Explain your reasoning.	This is a database lookup using two parameters. The time complexity is O(n).
Describe a set of positive and negative test cases you implemented for a model.	Positive tests for the model: when we looked at checkout, we expected the customer’s movies_checked_out would increase upon checkout. Negative test cases: when there is no more available movie inventory the model method “checkout” returns false.
Describe a set of positive and negative test cases you implemented for a controller.	Positive test case for rental controller: when we checked out a movie that was available with a valid customer, the db will store a rental, responds with json, and status is ok. A negative case for the rental controller is that when a customer doesn’t exist, the response is bad_request and the database shouldn’t change.
How does your API respond when bad data is sent to it?	We respond with a json that contains the error messages and bad_request status.
Describe one of your custom model methods and why you chose to wrap that functionality into a method.	Checkin is one of our custom model methods. It handily increments the movie available_inventory and decrements the customer’s movie_checked_out_count. That allowed us to test that functionality separately from the controller logic (whether or not the request data matches a rental in the database).
Do you have any recommendations on how we could improve this project for the next cohort?	It would be nice if the smoke tests matched the readme requirements. Specifically where incrementing/decrementing customer and movie data upon checkout. Also, have the smoke tests match the fields listed in the readme to return when calling show or index.
Link to Trello	https://trello.com/b/g5hOOC9d/videostoreapi
Link to ERD	https://www.lucidchart.com/documents/edit/8e8ee0ef-5d9f-48d1-93ff-5921c1df4d40/0_0
Summary

Models

Customer

Movies

Migration, Smoke Tests Pass

Rental

…equal inventory

Rental

CheezItMan · 2019-11-15T19:41:27Z

Video Store

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	Granular commits, good commit messages and both members contributing
Comprehension questions	Check, I would suggest your index actions are O(n) since you need to access each item.
General
Business Logic in Models	You've added the check-in and check-out functionality here, well done.
All required endpoints return expected JSON data	Check
Requests respond with appropriate HTTP Status Code	Mostly yes
Errors are reported	Check
Testing
Passes all Smoke Tests	Check
Model Tests - all relations, validations, and custom functions test positive & negative cases	Some missing tests, and some assert-style tests. Some improvements would be good here.
Controller Tests - URI parameters and data in the request body have positive & negative cases	Some improvements here, but overall not bad.
Overall	Nice work, you hit the learning goals here. You do have some improvements needed in testing, but otherwise this is well done. Congrats, you're done with Rails!

CheezItMan · 2019-11-15T19:29:17Z

app/models/rental.rb

+    end
+  end
+
+  def checkout 


CheezItMan · 2019-11-15T19:30:22Z

app/controllers/movies_controller.rb

+    if movie
+      render json: movie.as_json(only: [:id, :release_date, :title, :overview, :inventory, :available_inventory]), status: :ok
+    else
+      render json: {"errors" => {"id" => ["Movie with id #{params[:id]} not found."]}}


status code?

CheezItMan · 2019-11-15T19:34:35Z

test/models/customer_test.rb

@@ -0,0 +1,11 @@
+require "test_helper"
+
+describe Customer do


No validation tests?

Only testing having many rentals?

CheezItMan · 2019-11-15T19:35:53Z

test/models/movie_test.rb

+    end
+
+    it 'can make a movie with a title' do
+      assert @movie.valid?


These kinds of assertions work, but they're not in the style we're doing at Ada. So I'd suggest conforming to the spec-style testing.

Suggested change

assert @movie.valid?

expect(@movie.valid?).must_equal true

CheezItMan · 2019-11-15T19:37:02Z

test/models/rental_test.rb

+
+      end
+
+      it 'is invalid and returns errors when movie has no availability' do


Just noting the incomplete test. It's good you were thinking about this.

CheezItMan · 2019-11-15T19:38:06Z

test/controllers/customers_controller_test.rb

+      must_respond_with :ok
+    end
+
+    it 'responds with an array of customer hashes' do


What about when there are no customers?

CheezItMan · 2019-11-15T19:38:32Z

test/controllers/movies_controller_test.rb

+      must_respond_with :ok
+    end
+
+    it 'responds with an array of movie hashes' do


What about when there are no movies?

CheezItMan · 2019-11-15T19:39:32Z

test/controllers/rentals_controller_test.rb

+require "test_helper"
+
+describe RentalsController do
+  describe "checkout" do 


What about when there are no movies available for rental.

CheezItMan · 2019-11-15T19:40:09Z

test/controllers/rentals_controller_test.rb

+  end
+
+  describe "checkin" do
+    it "wont_change the db and responds ok for an existing customer and movie" do


Suggested change

it "wont_change the db and responds ok for an existing customer and movie" do

it "wont_change the Rental count in the db and responds ok for an existing customer and movie" do

CheezItMan · 2019-11-15T19:40:36Z

test/controllers/rentals_controller_test.rb

+      expect {
+        post checkin_path, params: checkin_params
+    }.wont_change "Rental.count"
+


You should also check to verify if the movie is updated properly.

NatalieTapias and others added 30 commits November 5, 2019 12:06

new rails api

dbb4a6f

Movie and Customer Model

8d8dc21

generates movies controller

6071afe

generates customers controller

5fb5617

Merge pull request #1 from calopter/models

5fd5a9f

Models

adds minitest

edfaf18

some customer fixtures

3b5fc6e

index for customers

ab336c1

Merge pull request #2 from calopter/customer

baaa2f8

Customer

Movies#Index action

b444301

Movies#show action

ab04481

Merge pull request #3 from calopter/movies

c581d01

Movies

validations for movie

19c0379

create for movie

424db42

Merge pull request #4 from calopter/movies

f3acb4b

Movies

Migration, Smoke Tests Pass

05eeaf0

fixed failing tests

639abf3

Merge pull request #5 from calopter/smoking

7c344a2

Migration, Smoke Tests Pass

Migrations adding relations

2cafa66

Rental Model and Test

03789aa

Relations to rentals for Customer and Movie

35c1fca

Belongs_to relations for Rental

4d8046d

Merge pull request #6 from calopter/rental

54332f2

Rental

routes for movies#checkin and movies#checkout

b0c6ef8

routes update

9b1ff23

rental model tests

1523b15

saving work

cf33a33

we now have a handy private method which sets available_inventory to …

72cd1c7

…equal inventory

rentals validations

5816f71

checkout date and due date tests in rental model

af5c146

NatalieTapias and others added 8 commits November 7, 2019 12:44

checkout valid validations

c86d5c1

checkout action

e7e5460

clean up comments and formatting

d27bccf

a few changes to start

d95b54d

merged

9b195c3

checkin

53b6ea2

checkin for rental

a5bf4f4

Merge pull request #7 from calopter/rental

9489785

Rental

CheezItMan reviewed Nov 15, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

trees: Mira & Natalie#18

trees: Mira & Natalie#18
NatalieTapias wants to merge 38 commits intoAda-C12:masterfrom
calopter:master

NatalieTapias commented Nov 8, 2019

Uh oh!

CheezItMan commented Nov 15, 2019

Uh oh!

CheezItMan Nov 15, 2019

Uh oh!

CheezItMan Nov 15, 2019

Uh oh!

CheezItMan Nov 15, 2019

Uh oh!

CheezItMan Nov 15, 2019

Uh oh!

CheezItMan Nov 15, 2019

Uh oh!

CheezItMan Nov 15, 2019

Uh oh!

CheezItMan Nov 15, 2019

Uh oh!

CheezItMan Nov 15, 2019

Uh oh!

CheezItMan Nov 15, 2019

Uh oh!

CheezItMan Nov 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		end

		it 'is invalid and returns errors when movie has no availability' do

	it "wont_change the db and responds ok for an existing customer and movie" do
	it "wont_change the Rental count in the db and responds ok for an existing customer and movie" do

Conversation

NatalieTapias commented Nov 8, 2019

Video Store API

Comprehension Questions

Uh oh!

CheezItMan commented Nov 15, 2019

Video Store

What We're Looking For

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants