Skip to content

Kristina & Amal#23

Open
krismosk wants to merge 54 commits intoAda-C12:masterfrom
krismosk:master
Open

Kristina & Amal#23
krismosk wants to merge 54 commits intoAda-C12:masterfrom
krismosk:master

Conversation

@krismosk
Copy link

@krismosk krismosk commented Nov 8, 2019

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 used the seed data to make a decision about what columns each table needed and the relationships between them. We noticed that the only thing that ties a customer and movie together is a rental so that Rental model would have a foreign key for Movie and Customer.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning. The show controller action for Movies and Customers would be O(n) because you'd have to iterate through the database for all records of that type and return the one that matches the unique ID passed in.
What is the Big-O time complexity of the POST /rentals/check-in endpoint? What does the time complexity depend on? Explain your reasoning. The show controller action for Rental Checkin would be O(n) because you'd have to iterate through the database for all records of that type and return the one that matches the unique ID passed in.
Describe a set of positive and negative test cases you implemented for a model. checkout_movie in the Rental model has a positive test that it will reduce the inventory for a movie by 1 and increase the count for a customers checked out movies count by 1. The negative case is if movie isn't available won't reduce the inventory for a movie and won't increase the count for a customers checked out movies.
Describe a set of positive and negative test cases you implemented for a controller. In the rentals controller we wrote a custom action called "checkin". The positive test case is that will return the checked in rental's id and status of ok. The negative test case will return a status of not_found if given invalid rental id.
How does your API respond when bad data is sent to it? Depending on what happened ,it would respond with a status of not_found, or bad_request. We also have the functionality to return an error message if a model validation is violated.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We chose to make checkout_movie a model method in the rental model because we didn't want to implement complex business logic in the rentals controller and since our method modifies data in the database, it made sense to keep it in the model.
Do you have any recommendations on how we could improve this project for the next cohort? The smoke tests were failing when they should not have been, for example there was a failure for Create Movie where it said we weren't returning a status of 200 or an id, which we were and our unit tests and integration tests are passing.
Link to Trello https://trello.com/b/YavNjs5O/video-store-api
Link to ERD Can send later upon request.
Summary

krismosk and others added 30 commits November 5, 2019 12:57
krismosk and others added 24 commits November 6, 2019 12:08
…ttributes, added a test expectation to test that body returns an id whenever a new movie or rental is created
…ventory by 1 and increments a customers rental list by 1, corresponding test passing
@CheezItMan
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Granular commits and good commit messages, both team members contributing.
Comprehension questions Check
General
Business Logic in Models Check, well done
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 Once I corrected your routes, yes
Model Tests - all relations, validations, and custom functions test positive & negative cases Check
Controller Tests - URI parameters and data in the request body have positive & negative cases Check
Overall This is really well done. You had some incorrect routes for check-in and check-out, but the rest is very well constructed. Nice work!

Rails.application.routes.draw do
resources :customers, only: [:index, :show]
resources :movies, only: [:index, :show, :create]
post '/rentals/checkout', to: 'rentals#checkout', as: 'rental_checkout'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be:

Suggested change
post '/rentals/checkout', to: 'rentals#checkout', as: 'rental_checkout'
post '/rentals/check-out', to: 'rentals#checkout', as: 'rental_checkout'

resources :customers, only: [:index, :show]
resources :movies, only: [:index, :show, :create]
post '/rentals/checkout', to: 'rentals#checkout', as: 'rental_checkout'
post '/rentals/checkin', to: 'rentals#checkin', as: 'rental_checkin'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be:

Suggested change
post '/rentals/checkin', to: 'rentals#checkin', as: 'rental_checkin'
post '/rentals/check-in', to: 'rentals#checkin', as: 'rental_checkin'

expect(body["errors"].keys).must_include "movie"
end

it "won't create a rental for a movie with 0 inventory" do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 🏆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants