Skip to content

Hallie/Brianna#21

Open
idhallie wants to merge 29 commits intoAda-C12:masterfrom
idhallie:master
Open

Hallie/Brianna#21
idhallie wants to merge 29 commits intoAda-C12:masterfrom
idhallie:master

Conversation

@idhallie
Copy link

@idhallie idhallie 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 read through all of the requirements and made note of the needed fields and their data types. Then we talked about the relationships to determine whether it was a "has many" or "belongs to" relationship.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning. Since these endpoints have to iterate through every customer or movie in order to display the requested information, it is O(n) with n being 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. The time complexity of POST /rentals/check-in is O(n) because the first thing the check-in action does is locate the rental. The worst case scenario is that the rental would be the last record in the database meaning that the find function would traverse all records in the database, which is O(n) with n being the number of rentals in the database. All other steps in this action are assigning values to the already found object, which does not worsen the time complexity.
Describe a set of positive and negative test cases you implemented for a model. In the rental model, we test that a rental is valid when all fields are present (positive) and we test that it is NOT valid if any of the required fields are missing (negative).
Describe a set of positive and negative test cases you implemented for a controller. For the movies controller, we test that it will respond with the correct keys for a valid movie (positive) and we test that it will return a "not found" error for an invalid movie (negative).
How does your API respond when bad data is sent to it? In the event of bad data, our API responds with ok: false in the JSON along with an appropriate HTTP status (:bad_request, :not_found).
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We created custom model methods to decrease/increase available inventory. This method actually adjusts available inventory AND the customer's movies_checked_out_count since both adjustments need to occur when the action is called. We chose to put this in a model method because it is making adjustments to the database, which we determined counts as "business logic". It is also a self-contained bit of code that peeled out of the controller keeps the controller nice and tidy.
Do you have any recommendations on how we could improve this project for the next cohort? It would be nice to have a more thorough overview of Postman functionality and how to debug failed tests. We got to nearly the last day of the project before we knew how you could see what the test was requesting.
Link to Trello https://trello.com/b/6B6Ygr4a/video-store
Link to ERD https://www.lucidchart.com/documents/edit/822f2c11-d949-4d74-8954-833146896057/0_0
Summary Huh?

@CheezItMan
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and mostly good commit messages. I would avoid using wave numbers in commit messages and focus on functionality added. Pretty good granular commits.
Comprehension questions Check
General
Business Logic in Models You have business logic in the rental model. See my notes there.
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 Missing Business logic tests Otherwise things look good.
Controller Tests - URI parameters and data in the request body have positive & negative cases Check
Overall Nice work, you hit the learning goals. Excellent job!

KEYS = [:id, :name, :address, :city, :state, :postal_code, :phone, :registered_at, :movies_checked_out_count]

def index
if ["name","registered_at", "postal_code"].include? params[:sort]

Choose a reason for hiding this comment

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

Nice

belongs_to :movie
belongs_to :customer

def decrease_available

Choose a reason for hiding this comment

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

I like that you moved some business logic here.

However it's not a good idea to keep the available movies in a field, but rather calculate from the total inventory and the number of outstanding rentals. Otherwise it's possible for things to get out of sync if other applications use the same database.

@@ -0,0 +1,41 @@
require "test_helper"

describe Rental do

Choose a reason for hiding this comment

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

No tests for your custom methods?

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