Amy^2 by amyesh · Pull Request #16 · Ada-C11/VideoStoreAPI

amyesh · 2019-05-16T19:35:35Z

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 and customers were disparate entities, and that in order to satisfy a many to many relationship, there needed to be a join table, stage left: rentals.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning.	O(n) linear time complexity where n represents the number of movies or customers.
What is the Big-O time complexity of the `POST /rentals/check-in` endpoint? What does the time complexity depend on? Explain your reasoning.	O(n) linear time complexity, where n represents the number of elements in the rentals collection.
Describe a set of positive and negative test cases you implemented for a model.	That due date can be set given valid rental data, and that it can't be set if rental data is invalid.
Describe a set of positive and negative test cases you implemented for a controller.	We tested that a rental can't be created without a valid movie or customer, and tested that it can be created with valid data.
How does your API respond when bad data is sent to it?	It displays a list of errors.
Describe one of your custom model methods and why you chose to wrap that functionality into a method.	We wrapped the set_available_inventory data in a model method. This method sets the available_inventory to the given inventory unless the user specifies a different availability in the HTTP request. We chose to do this in the model because the value of available_inventory relies upon the value given as the initial inventory.
Do you have any recommendations on how we could improve this project for the next cohort?	It would be helpful to have more context about how this particular API is intended to be used - user stories would have been super helpful. The smoke tests felt less like tests and more prescriptive in terms of how the data was structured, whereas the assignment felt like you had more control over how your API functioned. Also, the smoke tests for Wave 3 included optional requirements.
Link to Trello	https://trello.com/b/JSDQhqTj/scarecrow-20
Link to ERD	(included in trello board)
Summary	Thanks for pairing us together - y'all are the best <3

add movies and customer routes

finished tests and methods for customer controller

Ap/movies Index, Show & Create actions and tests

removed show and create methods from customers controller

…inventory

added custom method/tests to movies model, and updated controller method and tests

Ap/rentals model and tests

Checkin

remove due date validation on rental model

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

Video Store

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	Good number of commits and good commit messages
Comprehension questions	Check, you're welcome
General
Business Logic in Models	Somewhat, it would make sense to have a method to check out and check in a movie.
All required endpoints return expected JSON data	Required stuff done
Requests respond with appropriate HTTP Status Code	Check
Errors are reported	Check
Testing
Passes all Smoke Tests	All required tests pass
Model Tests - all relations, validations, and custom functions test positive & negative cases	Check, but you should have more model methods. What happens if you check in a movie that's not checked out, or check out a movie with not enough inventory.
Controller Tests - URI parameters and data in the request body have positive & negative cases	Check, you should also have some tests in the effects of checking in and checking out.
Overall	I'm not sure how to pluralize Amy (Amies?). You both hit the major learning goals. You did everything required. See my inline comments and let me know if you have any questions.

CheezItMan · 2019-05-23T23:12:03Z

app/controllers/rentals_controller.rb

+class RentalsController < ApplicationController
+
+  def checkin
+    rental = Rental.find_by(customer_id: rental_params[:customer_id], movie_id: rental_params[:movie_id])


How is this changing the Rental status? There's no way to tell in the DB if the rental was checked in!

CheezItMan · 2019-05-23T23:13:00Z

app/models/customer.rb

@@ -0,0 +1,7 @@
+class Customer < ApplicationRecord
+  has_many :rentals
+


So no method to tell how many movies they have checked out.

CheezItMan · 2019-05-23T23:13:30Z

app/models/movie.rb

+  validates :overview, presence: true
+  validates :release_date, presence: true
+
+  def set_available_inventory


There are ways in Rails to invoke this method when a Movie is created, saved or updated.

CheezItMan · 2019-05-23T23:15:28Z

app/controllers/rentals_controller.rb

+    if rental
+      render json: {id: rental.id, movie_id: rental.movie.id, customer_id: rental.customer.id}, status: :ok
+    else
+      render json: [{errors: {rental: ["Rental not found for customer #{rental_params[:customer_id]}, and movie #{rental_params[:movie_id]}"]}}],


This line is a bit long, feel free to put in line breaks to make this more readable.

CheezItMan · 2019-05-23T23:15:37Z

app/controllers/rentals_controller.rb

+
+    if rental.save
+      rental.set_due_date
+      render json: [{ id: rental.id, movie_id: rental.movie.id, customer_id: rental.customer.id, due_date: rental.due_date }], status: :ok   


This line is also long. You also need to have some impact on the available inventories.

aphunk and others added 30 commits May 14, 2019 12:17

initial commit

4e6c4e9

generate customers, movies, rentals model

1cbf971

seed db

a369822

added controllers and validations

72b4a0d

added validation tests for all models

da5ac64

added foreign keys to rental

0e7087f

added foreign keys again

6327dcd

fix relationship tests

9f3d9bb

finish wave 1

8d697ba

add movies and customer routes

0855cfb

Merge pull request #1 from aphunk/ap/movies_CRUD_actions

7ddf95d

add movies and customer routes

movies CRUD tests

3e3aab0

complete Movies controller tests

80a8be0

add a lil' more to Movies index test

b308f67

add whitespace

f558664

update movie index return fields to match project requirements

be716b9

update test to reflect returned fields requirement

66fad3e

renders Movie ID when a new movie is created

a776f56

finished tests and methods to customer controller

19608e6

test that a BAD REQUEST is still returned as JSON

8bf2d98

putzin' around in show tests

208d9b1

Merge pull request #3 from aphunk/am_customercrud

744d482

finished tests and methods for customer controller

removed show and create methods from customers controller

a910a60

Merge pull request #2 from aphunk/ap/movies_CRUD_actions

8bc48b8

Ap/movies Index, Show & Create actions and tests

Merge pull request #4 from aphunk/am_customercrud

4f9d657

removed show and create methods from customers controller

add rental relationship test

9a6992b

added custom method and controller method and tests to set available …

502da99

…inventory

added tests to set available inventory

7c454ea

Merge pull request #5 from aphunk/setting_avail_inventory

5ea2873

added custom method/tests to movies model, and updated controller method and tests

added tests for rental checkin

efcea08

aphunk and others added 14 commits May 15, 2019 16:26

add rentals controller tests

eb5a538

complete checkout tests

f413c21

Due date test in rental model

f890283

Merge branch 'master' into ap/rentals_model_and_tests

80fea82

wrote rental checkin method

7d88f98

Merge pull request #6 from aphunk/ap/rentals_model_and_tests

c5026d3

Ap/rentals model and tests

Merge branch 'master' into checkin

c10be16

Merge pull request #7 from aphunk/checkin

093812b

Checkin

added end

2b7a69f

remove due date validation on rental model

1a98370

Merge pull request #8 from aphunk/ap/rentals_model_and_tests

f3ee8e3

remove due date validation on rental model

mollified postMAN

9b01231

refactor set_due_date method to pass smoke tests

c3bf32d

fix due date model test

ce78c25

CheezItMan reviewed May 24, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Amy^2#16

Amy^2#16
amyesh wants to merge 44 commits intoAda-C11:masterfrom
aphunk:master

amyesh commented May 16, 2019

Uh oh!

CheezItMan commented May 23, 2019

Uh oh!

CheezItMan May 23, 2019

Uh oh!

CheezItMan May 23, 2019

Uh oh!

CheezItMan May 23, 2019

Uh oh!

CheezItMan May 23, 2019

Uh oh!

CheezItMan May 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,7 @@
		class Customer < ApplicationRecord
		has_many :rentals

Conversation

amyesh commented May 16, 2019

Video Store API

Comprehension Questions

Uh oh!

CheezItMan commented May 23, 2019

Video Store

What We're Looking For

Uh oh!

CheezItMan May 23, 2019

Choose a reason for hiding this comment

Uh oh!

CheezItMan May 23, 2019

Choose a reason for hiding this comment

Uh oh!

CheezItMan May 23, 2019

Choose a reason for hiding this comment

Uh oh!

CheezItMan May 23, 2019

Choose a reason for hiding this comment

Uh oh!

CheezItMan May 23, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants