Ports & Sockets - Jillianne & Shirley #24

jillirami · 2019-05-17T21:51:28Z

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.	The seed data, as well as the description of the project indicated to us that the model that unites customers and movies is the creation of a rental, and so we created our ERD accordingly.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning.	Customer index is O(n) and Movie index is O(n). And, this is dependent on the size of movies or customers in the database.
What is the Big-O time complexity of the `POST /rentals/check-in` endpoint? What does the time complexity depend on? Explain your reasoning.	Rental checkin endpoint has an O(n) time complexity as we are still finding the rental to checkin through all rentals available.
Describe a set of positive and negative test cases you implemented for a model.	For the rental model, we tested that it does have a relationship with a customer and a movie, and we tested that false is returned if a movie is unavailable.
Describe a set of positive and negative test cases you implemented for a controller.	In the Rental controller, we checked for the return of an error if the movie is checkout already. We also determined what values are being output when input is valid.
How does your API respond when bad data is sent to it?	It has an errors key and will return all errors applicable when bad data is passed in.
Describe one of your custom model methods and why you chose to wrap that functionality into a method.	We chose to make custom methods our of reducing inventory of a movie and increasing it, so that we could parse out certain functionality (business logic) from our controller.
Do you have any recommendations on how we could improve this project for the next cohort?	Remind students that if Postman does not work, to attempt a rails db:reset.
Link to Trello	None utilized.
Link to ERD	https://www.lucidchart.com/documents/view/99dca357-9b79-4ab1-8717-0f9a14a737b4/0
Summary	We enjoyed working with another classmate in another class! Great project!

movie model testing

Jrcheckintestingcontinued

droberts-sea · 2019-05-23T20:41:57Z

Video Store

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	yes
Comprehension questions	yes
General
Business Logic in Models	some - see inline
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
Controller Tests - URI parameters and data in the request body have positive & negative cases	yes
Overall	Great job overall. I'm particularly happy with your test coverage. I do see some work for growth around identifying and isolating model methods. This won't be quite as straightforward to practice in JavaScript, since we'll be so focused on display logic, but there will definitely be opportunities to isolate business logic. Other than that I like what I see - keep up the hard work.

droberts-sea · 2019-05-23T17:27:08Z

app/controllers/customers_controller.rb

+    def index
+        customers = Customer.all 
+        render status: :ok, json: customers.as_json(only: [:name, :phone, :postal_code, :registered_at, :id], methods: [:movies_checked_out_count])
+    end 


I would probably split line 4 across multiple lines.

droberts-sea · 2019-05-23T17:29:06Z

app/controllers/movies_controller.rb

+
+    if params[:release_date]
+      movie.release_date = Date.parse(params[:release_date]).strftime('%Y-%m-%d')
+    end


Since release_date is a column of type date, postgres should handle this parsing automatically.

droberts-sea · 2019-05-23T17:30:54Z

app/controllers/rentals_controller.rb

+  def checkout
+    rental = Rental.new(rental_params)
+    rental.checkout_date = Date.today
+    rental.due_date = Date.today + 1.week


This controller action is doing a ton of work! Would it be possible to extract this to a model method, maybe something like Rental.check_out(customer_id, movie_id)? This would make testing easier too.

droberts-sea · 2019-05-23T17:31:51Z

app/controllers/rentals_controller.rb

+    movie = Movie.find_by(id: rental_params[:movie_id])
+
+    unless movie 
+      render json: { ok: false, errors: rental.errors.messages },


Since Rental has a belongs_to relationship with Movie, this check should be made automatically.

Also, on line 12 you haven't called rental.save or rental.valid? yet, so rental.errors will be empty.

droberts-sea · 2019-05-23T17:32:11Z

app/controllers/rentals_controller.rb

+    if rental
+      if rental.update(checkin_date: Date.today)
+        movie = Movie.find(rental.movie_id)
+        movie.increase_inventory


Again, it would probably make sense to move this into a model method.

droberts-sea · 2019-05-23T20:17:40Z

app/models/customer.rb

+  def movies_checked_out_count
+    checkouts = []
+
+    self.rentals.each do |rental|


I love that you've made this a method, instead of storing it in the database - that means there's no way to accidentally forget to update it.

You could simplify this method a bit as:

def movies_checked_out_count return self.rentals.where(checkin_date: nil).length end

droberts-sea · 2019-05-23T20:18:43Z

app/models/movie.rb

+
+  def available_inventory
+    return self.inventory
+  end


I would probably have chosen to calculate this on the fly instead of storing it, similar to movies_checked_out_count

droberts-sea · 2019-05-23T20:29:51Z

test/controllers/movies_controller_test.rb

+    it "returns JSON with bogus data" do
+      get movie_path(-1)
+      expect(response.header["Content-Type"]).must_include "json"
+    end


Here you might also check the structure of the JSON itself - you would expect it to contain at least:

{ ok: false errors: // ... something ... }

droberts-sea · 2019-05-23T20:38:34Z

test/controllers/movies_controller_test.rb

+    # failure case, responds with 404
+    it 'responds with 404 for movie not found' do
+      id = -1
+      get movie_path(id)


You switch back and forth between success and failure cases in this describe block - if you were to organize it differently, it might be easier to read.

shirley1chu and others added 30 commits May 14, 2019 11:28

rails app created, zomg route initiated but no action taken yet

f22f4d2

model and controllers created for movie and customer, routes updated

b2e58e5

validations ammended

1a0a233

customer model validation tests written and passed

2cc94db

movie model testing

98ba325

Merge pull request #1 from jillirami/jrmoviemodeltests

635d700

movie model testing

zomg works! also added customer controller tests

971bf23

customer index controller tests passed

3dc1b03

edge case tested

2ab3df4

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

d5dcc5c

movie controller and controller tests added

221337a

date parsed from string input, movie controller

1c60093

updated for available inventory requirement

a8c83bc

removed required param from movie controller

2748c19

got rid of require in customer controller

69a1d9d

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

8dae7b5

rental model and controller created, foreign keys added to schema

2c8613a

movie model and controller tests refactored for validation flexibility

d313caf

customer model and controller tests refactored

581df1a

rental schema updated, and controller actions created

297083e

corrected checkin action

90ebc49

checkout tests initiated, invalid rental data test has errors

aa3321d

refactor of movie checkout method in customer model

4e98c48

removed commented out code in customer controller test

bf8c422

Merge pull request #2 from jillirami/jrcheckintestingcontinued

b19cc42

Jrcheckintestingcontinued

added rental availiability custom validation, checkout tests passed

c89b5cd

all rental model validation tests written and passed

31ed619

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

6660c87

rental model tests merged in, all tests passing

45f2545

testing for checkin function complete

c8367c7

droberts-sea reviewed May 23, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Ports & Sockets - Jillianne & Shirley #24

Ports & Sockets - Jillianne & Shirley #24

Uh oh!

jillirami commented May 17, 2019

Uh oh!

droberts-sea commented May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

droberts-sea May 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Ports & Sockets - Jillianne & Shirley #24

Are you sure you want to change the base?

Ports & Sockets - Jillianne & Shirley #24

Uh oh!

Conversation

jillirami commented May 17, 2019

Video Store API

Comprehension Questions

Uh oh!

droberts-sea commented May 23, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants