Skip to content

Eve & Yitgop#27

Open
rinostar wants to merge 27 commits intoAda-C12:masterfrom
rinostar:master
Open

Eve & Yitgop#27
rinostar wants to merge 27 commits intoAda-C12:masterfrom
rinostar:master

Conversation

@rinostar
Copy link

@rinostar rinostar 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 worked on customer and movie individually first, and once we reached rental, we started to make connections among the three classes. Since the seed data doesn't show direct connection between movie and customer, we decided to establish the relationship through rental.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning. It would be O(n), depends on how many customers/movies we have 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. It takes O(1) to find the customer, 0(1) to find the movie, and O(n) to find the rental, which is O(n) in general.
Describe a set of positive and negative test cases you implemented for a model. For rental validations, we tested for the case where all fields are present and rental will be valid, as well as where rental is invalid without a movie.
Describe a set of positive and negative test cases you implemented for a controller. For rental checkin action, we tested that we can check in for valid rental and we can't check in for rental that doesn't exist
How does your API respond when bad data is sent to it? It response with a bad_request code and render a json for error message
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We overwrote the initialize method for rental to set checkout_date and due_date accordingly, so we don't deal with the business logic in controller
Do you have any recommendations on how we could improve this project for the next cohort? Clarify that which test in wave 3 would be optional
Link to Trello We pair programmed throughout the project, so we didn't create a Trello board
Link to ERD https://imgur.com/a/xdxTDYd
Summary Great cross-class collaboration! :D

rinostar and others added 27 commits November 5, 2019 13:47
@CheezItMan
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and good commit messages, even if some are mispelled rantal :p Both team members contributed.
Comprehension questions Check, glad to hear you pair programmed it all.
General
Business Logic in Models Very little in the models
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 Check
Controller Tests - URI parameters and data in the request body have positive & negative cases Overall well done
Overall Nice work, you covered the essentials here. Some more work could be done in terms of business logic and handling the inventory of movies, but you covered the learning goals. Well done.

if rental.returned == false
rental.returned = true
rental.save
@customer.movies_checked_out_count -= 1

Choose a reason for hiding this comment

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

If other systems interacted with the database, keeping a field to track the number of items checked out would be dangerous because it could get out of sync with the rentals in the system. Better to make checked_out_count a method which uses rentals to determine how many movies the user has checked out.

That would be great business logic method

check_response(expected_type: Hash, expected_status: :ok)
end

it "won't create a rental for invalid input and responds with bad request" do

Choose a reason for hiding this comment

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

What about if there's no inventory available?

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

Comments