Angelica Ceja - Wini Irarrazaval - Octos by winirarrazaval · Pull Request #11 · Ada-C9/VideoStoreAPI

winirarrazaval · 2018-05-11T21:27:09Z

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 made a class for each file , and gave each class the attributes from the seed data. And we wrote the rest of the ERD based on the rest of the requirements.
Describe a set of positive and negative test cases you implemented for a model.	In the rental model test we checked that a rental could not be made if in that date range all movie copies are already rented. If there are enough copies on the available inventory then a rental can be made.
Describe a set of positive and negative test cases you implemented for a controller.	In the rental controller test we test that the checkin action can be made for a valid movie and customer id and that the rentals return date is set to today. The negative test case, the check in action responds with not found for movie ID and customer ID that DNE.
How does your API respond when bad data is sent to it?	It responds with not found when trying to perform an action on a record that does not exist. It responds with bad request when trying to create a new record with data that does not pass the validations.
Describe one of your custom model methods and why you chose to wrap that functionality into a method.	In the movie model we created get_available_inventory method to get the current number of available copies of a movie to be able to create a rental.
Do you have any recommendations on how we could improve this project for the next cohort?	Make the instructions for postman (smoketest) more clear/demo it in class.
Link to Trello	https://trello.com/b/Tu0XuJto/ride-share
Link to ERD	https://www.lucidchart.com/invitations/accept/d16d45a0-3afa-41f9-8e0d-bae5892139c1

… method

…show action

…page

…ts for Wave 2

… modify tests accordingly

droberts-sea · 2018-05-18T00:29:07Z

Video Store

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	yes
Comprehension questions	yes
General
Business Logic in Models	some - Your `RentalsController` has a lot of business logic still.
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	tests for many custom methods are missing, but otherwise looks good.
Controller Tests - URI parameters and data in the request body have positive & negative cases	yes - great work!
Overall	Good work overall!

droberts-sea · 2018-05-18T00:21:54Z

app/controllers/rentals_controller.rb

+
+    if rental.save
+      customer = Customer.find_by(id: customer_id)
+      customer.movies_checked_out_count += 1


All of this work around assigning dates, decreasing movies_checked_out_count, etc. should be encapsulated in one testable model method.

droberts-sea · 2018-05-18T00:22:13Z

app/controllers/rentals_controller.rb

+      if rental.save
+        customer = Customer.find_by(id: rental.customer_id)
+        customer.movies_checked_out_count -= 1
+        customer.save


Similarly, all this work ought to be wrapped up in one model method.

droberts-sea · 2018-05-18T00:22:30Z

app/controllers/rentals_controller.rb

+  def checkin
+    rental = Rental.find_by(movie_id: params[:movie_id], customer_id: params[:customer_id])
+
+    unless rental


What if this rental has already been returned?

droberts-sea · 2018-05-18T00:23:25Z

app/models/movie.rb

+  def get_available_inventory
+    checked_out_count = 0
+    self.rentals.each do |rental|
+      if rental.return_date.nil?


I like that you calculate this on the fly rather than saving it in the DB.

droberts-sea · 2018-05-18T00:24:13Z

app/models/rental.rb

+  def enough_inventory_for_rent
+    if movie
+      count = 0
+      self.movie.rentals.each do |rent|


How does this validation interact with returning movies?

droberts-sea · 2018-05-18T00:26:46Z

test/controllers/rentals_controller_test.rb

+
+    it "must respond with bad_request for a movie with no available inventory" do
+      movie = Movie.first
+      customer = Customer.first


droberts-sea · 2018-05-18T00:27:07Z

test/controllers/rentals_controller_test.rb

+
+    it "must respond with not_found for a rental that DNE" do
+
+      movie_id = Movie.last.id + 1


What if the rental has already been checked in

droberts-sea · 2018-05-18T00:27:49Z

test/models/customer_test.rb

+
+  end
+
+  describe "relationships" do


There are some custom methods you're not testing here.

droberts-sea · 2018-05-18T00:28:31Z

test/models/rental_test.rb

+
+    it "a rental can not be completed if all copies are rented for a date range" do
+      movie = Movie.first
+      customer = Customer.first


Good work getting a test in on this.

winirarrazaval and others added 27 commits May 7, 2018 12:13

added rails for api

c3e5c2f

created models movie, rental and customers

351697a

Added columns and we set up foreign key relationships

1bc1f9a

added something to the seed.rb file

33d1b43

Test customer validations

e3554a2

Added some more validations for the models

6e8df24

Edit Rental foreign key names, validate Rental date range with custom…

28eb4b6

… method

created validations and tests for relationships

f4128f4

added validation for renting with enough inventory

e253ff9

Created controllers; tested and wrote Customers index action

bc53918

added show controller action and tests for it

3653d24

Test and write Customer create action

838066e

Set up Movies Controller index tests

6b8eae5

Added a column in movies

021414e

Fixed test and squema typo

c2f3dc3

Test Movies index action

a166d58

Set up Movies 'show' action and use RABL to display JSON

12d8605

Rewrite index action using RABL; render error if movie not found for …

6692a5a

…show action

added some tests for movies show controller

1ba0d1a

added tests for controller movie create and created create method

99bbc92

Test and write rental controller checkout action

0aee863

Working on rental controller

9db8e6f

added controller actions and tests for checkin

07567bc

Modify enough_inventory_for_rent method in Rentals to use return_date

c4198d3

Write available_inventory method for Movie model to use in show RABL …

5197527

…page

Modify Movies and Customers controllers to meet smoke test requiremen…

5b59aed

…ts for Wave 2

Modify Rentals controller to meet smoke test requirements for Wave 3;…

ee70972

… modify tests accordingly

droberts-sea reviewed May 18, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Angelica Ceja - Wini Irarrazaval - Octos#11

Angelica Ceja - Wini Irarrazaval - Octos#11
winirarrazaval wants to merge 27 commits intoAda-C9:masterfrom
winirarrazaval:master

winirarrazaval commented May 11, 2018

Uh oh!

droberts-sea commented May 18, 2018

Uh oh!

droberts-sea May 18, 2018

Uh oh!

droberts-sea May 18, 2018

Uh oh!

droberts-sea May 18, 2018

Uh oh!

droberts-sea May 18, 2018

Uh oh!

droberts-sea May 18, 2018

Uh oh!

droberts-sea May 18, 2018

Uh oh!

droberts-sea May 18, 2018

Uh oh!

droberts-sea May 18, 2018

Uh oh!

droberts-sea May 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		it "must respond with not_found for a rental that DNE" do

		movie_id = Movie.last.id + 1

Conversation

winirarrazaval commented May 11, 2018

Video Store API

Comprehension Questions

Uh oh!

droberts-sea commented May 18, 2018

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants