Tatiana and Amy W by amythetester · Pull Request #25 · Ada-C11/VideoStoreAPI

amythetester · 2019-05-17T22:00:48Z

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.	Based on the JSON files we were able to create models for customer and movie. When we read the requirements we determined that Rentals were comprised of one movie and one customer. This led us to create a belongs_to relationship where rentals belong to movies and customers.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning.	We believe the time complexity is O(n) where n is the number of records in the database for that model. This is because as records are added to the database the iterations the method has to go through increases linearly.
What is the Big-O time complexity of the `POST /rentals/check-in` endpoint? What does the time complexity depend on? Explain your reasoning.	We believe the time complexity is O(n) where n is the number of records in the rentals database for the Rental model. This is because the rental you are looking for could be at the end of the dataset.
Describe a set of positive and negative test cases you implemented for a model.	For the movie model, we had a positive test where we tested that the available inventory increased and we had a negative test where available inventory was not increased because all movies were already checked in.
Describe a set of positive and negative test cases you implemented for a controller.	For the rental controller, our positive test was testing that a new rental was created and our negative test was to ensure a rental was not created when an invalid movie id was provided.
How does your API respond when bad data is sent to it?	Most of the time it returns a bad request and an error message in JSON.
Describe one of your custom model methods and why you chose to wrap that functionality into a method.	Reduce available inventory was one of our custom model methods for movie. We chose to wrap this functionality into a method because this keeps the business logic out of the controller, but allows the method to be called from the controller.
Do you have any recommendations on how we could improve this project for the next cohort?	This note in Wave 2 of the instructions was confusing because the smoke tests include a test that required optionals to be complete: "movies_checked_out_count: This will be 0 unless you've completed optional requirements"
Link to Trello	https://trello.com/b/AWPyA1yo/videostoreapi
Link to ERD	https://drive.google.com/file/d/1275Oly6fSqGLj-yq_7e-UMTvUAvGDkL6/view?usp=sharing
Summary	We worked very intentionally to implement TDD red/green refactor approach and we also did a PR/Merge workflow as well.

describe block

Customer model tests (can delete when merged)

Movie model test

…not passing

Customer controller tests and CRUD

Movie controller

updated for fields that only need to be returned

Rentals Checkin

…s checked in

Rental tq

added inventory check to controller tests for checkin

Rental tq

CheezItMan · 2019-05-25T23:50:30Z

Video Store

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	Good number of commits and good commit messages
Comprehension questions	Check, I'm glad you were able to practice TDD in this exercise
General
Business Logic in Models	Check
All required endpoints return expected JSON data	Check
Requests respond with appropriate HTTP Status Code	Check
Errors are reported	Check
Testing
Passes all Smoke Tests	Check
Model Tests - all relations, validations, and custom functions test positive & negative cases	Nope skimpy testing
Controller Tests - URI parameters and data in the request body have positive & negative cases	Nope Very skimpy testing here as well, mostly just copy-paste of the instructor tests in class.
Overall	You have a nice working API. However your testing was very weak. I suggest when looking at testing taking a step back and try to play gremlin with the situation. Look at the check-in process and the variables involved, movie_id and customer_id and see what could be passed in wrongly.

CheezItMan · 2019-05-25T23:31:51Z

app/models/movie.rb

@@ -0,0 +1,24 @@
+class Movie < ApplicationRecord
+  validates :title, :inventory, presence: true
+  validates :available_inventory, numericality: { greater_than: -1 }


This is causing the seeds file to fail!

CheezItMan · 2019-05-25T23:42:34Z

test/models/rental_test.rb

+require "test_helper"
+
+describe Rental do
+  let(:rental) { rentals(:one) }


No tests here?

Also you should try to see what happens if you create a rental with no available inventory for the movie.

CheezItMan · 2019-05-25T23:42:51Z

app/models/customer.rb

@@ -0,0 +1,9 @@
+class Customer < ApplicationRecord


No has_many?

CheezItMan · 2019-05-25T23:43:10Z

app/models/movie.rb

@@ -0,0 +1,24 @@
+class Movie < ApplicationRecord


No has many rentals?

CheezItMan · 2019-05-25T23:43:47Z

test/models/customer_test.rb

+    end
+  end
+
+  describe "relationships" do


Missing tests!!!

CheezItMan · 2019-05-25T23:46:44Z

test/models/movie_test.rb

+  end
+
+  describe "custom methods" do
+    describe "checkin" do


What about a customer trying to return a movie that wasn't checked out? Or a movie that wasn't checked out by them?

CheezItMan · 2019-05-25T23:48:21Z

app/controllers/movies_controller.rb

+      render json: movie.as_json(only: [:id]),
+             status: :ok
+    else
+      render json: { errors: ["Movie could not be saved"] },


No validation errors reported?

amythetester and others added 30 commits May 14, 2019 11:39

intial rails app created

ed6a390

generated movie and customer models

ea471d6

routes added

521eed1

merged conflicts

b9c7bbd

added some fixtures to movie

9d424f6

added model validation tests for movie

742642b

validation tests added, but failing

1bf1b08

added customer model validations and tests pass

c7b295a

added words to

4199ed6

describe block

Merge pull request #1 from tatsqui/customer-model-tests

47ca5df

Customer model tests (can delete when merged)

added more tests to movie_test

3722815

Merge pull request #2 from tatsqui/movie-model-test

25de2a0

Movie model test

generated controller movies

88242d2

customer controller tests written and CRUD action wrritten, one test …

9abc444

…not passing

all controller tests passing

60f1760

removed unrelated references

d187cf1

added basic tests to movies_controller_tests for index action

880fcd1

made movie params private

f2f45c6

added params

b6d69ce

changed order

fccb4ea

Merge pull request #3 from tatsqui/customer-controller

64563f3

Customer controller tests and CRUD

Merge pull request #4 from tatsqui/movie-controller

cc1d0fb

Movie controller

Merge branch 'master' into movie-controller

1fd9fc9

added id to render movies json

401070e

updated for fields that only need to be returned

904b277

add id to test line 35

1e67b8a

Merge pull request #5 from tatsqui/customer-controller-fix

5276332

updated for fields that only need to be returned

added movies checked out count column with a default of 0

3ecdeed

added available_inventory to Movie

79a6fd3

updated what is returned in JSON to include movie count

6cbfd75

amythetester and others added 28 commits May 15, 2019 13:44

removed movie key from test

a754d29

DRY up line 58

07f1078

Merge branch 'master' of github.com:tatsqui/VideoStoreAPI

fae4b5e

fixed rental model test

9d8a1ea

test stubs for check out

fd094e1

added rental_params

a8f6e3f

contoller test shells

72e8209

merged conflict

fd41e71

added checkout method stub

c82c903

test001 green

8599a88

wrote test for nominal failure case

e1a3de5

customer model method and test

60ca72f

wrote tests for bad movie_id and customer_id

6432e84

negative test for custom method

5765bd5

checkin tests and code

d6111fb

passing controller edge case for checkin

c3ce7c5

all tests for checkin passing

03ab29b

Merge pull request #11 from tatsqui/rentals-amy

6f89553

Rentals Checkin

working on edge success test for checking out on same day something i…

1a11ef0

…s checked in

all tests green

98a8c88

added inventory check to controller tests for checkin

6e2c452

added model custom method tests

2a27520

Merge pull request #12 from tatsqui/rental-tq

8fe4d81

Rental tq

Merge branch 'master' into rentals-amy

81f51fd

Merge pull request #13 from tatsqui/rentals-amy

ecf2b37

added inventory check to controller tests for checkin

cleaned up some controller tests

03f7e61

Merge branch 'master' into rental-tq

8f24947

Merge pull request #14 from tatsqui/rental-tq

379ef9d

Rental tq

CheezItMan reviewed May 25, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tatiana and Amy W#25

Tatiana and Amy W#25
amythetester wants to merge 79 commits intoAda-C11:masterfrom
tatsqui:master

amythetester commented May 17, 2019 •

edited

Loading

Uh oh!

CheezItMan commented May 25, 2019

Uh oh!

CheezItMan May 25, 2019

Uh oh!

CheezItMan May 25, 2019

Uh oh!

CheezItMan May 25, 2019

Uh oh!

CheezItMan May 25, 2019

Uh oh!

CheezItMan May 25, 2019

Uh oh!

CheezItMan May 25, 2019

Uh oh!

CheezItMan May 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

amythetester commented May 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Video Store API

Comprehension Questions

Uh oh!

CheezItMan commented May 25, 2019

Video Store

What We're Looking For

Uh oh!

CheezItMan May 25, 2019

Choose a reason for hiding this comment

Uh oh!

CheezItMan May 25, 2019

Choose a reason for hiding this comment

Uh oh!

CheezItMan May 25, 2019

Choose a reason for hiding this comment

Uh oh!

CheezItMan May 25, 2019

Choose a reason for hiding this comment

Uh oh!

CheezItMan May 25, 2019

Choose a reason for hiding this comment

Uh oh!

CheezItMan May 25, 2019

Choose a reason for hiding this comment

Uh oh!

CheezItMan May 25, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

amythetester commented May 17, 2019 •

edited

Loading