Skip to content

Kayla & Marj#29

Open
Schmarj3 wants to merge 43 commits intoAda-C14:masterfrom
schmarj3-learning-projects:master
Open

Kayla & Marj#29
Schmarj3 wants to merge 43 commits intoAda-C14:masterfrom
schmarj3-learning-projects:master

Conversation

@Schmarj3
Copy link

@Schmarj3 Schmarj3 commented Dec 4, 2020

Assignment Submission: Video Store API

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

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.

Reflection

Prompt Response
Explain how you came up with the initial design of your ERD, based on the seed data and reading through the API endpoints We saw that there were customers and videos that needed to connect through rentals.
What would be the Big-O time complexity of your /customers & /videos endpoints? What does the time complexity depend on? Explain your reasoning. We think it will take O(n) to loop through all customers or videos then O(n) to print them all out as json. When the endpoint requires specific information like an id, we think it might be O(1) because the address needed known.
What is the Big-O time complexity of the POST /rentals/check-in endpoint? What does the time complexity depend on? Explain your reasoning. Since both checkin and checkout require the customer_id and video_id, it's easier to look the information up quickly, so O(1). Then updating the database should also take just O(1).
Describe a specific set of positive and negative test cases you implemented for a model. If did, would would have created a validation test for the video model to make sure there was at least a title present.
Describe a specific set of positive and negative test cases you implemented for a controller. "increase the customer videos_checked_out_count by one" as the positive test case, and "return back detailed errors and a status 404: Not Found if the customer does not exist" as the negative test case.
Broadly, describe how an API should respond when it handles a request with invalid/erroneous parameters. should respond with a 404 or invalid request error message and status ok is false.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We tried to create a custom method for the checkout due date, but kept receiving a no method error.

Schmarj3 and others added 30 commits December 1, 2020 18:04
initial commit, generated models and controllers, routes
Created migrations for video and customer tables to change datatypes
changed and fixed schema columns in order to seed data
modified returned json data for customer
Copy link

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

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

Video Store API

Major Learning Goals/Code Review

Criteria yes/no, and optionally any details/lines of code to reference
Practices git with at least 10 small commits and meaningful commit messages ✔️
Understands the importance of how APIs handle invalid/erroneous data in their reflection questions ✔️
Practices Rails best practices and well-designed separation, and encapsulates business logic around check-out and check-in in models ⚠️ No business logic methods
Uses controller tests to ensure quality code for every route, and checks for response status and content, with positive cases and negative cases ⚠️ Looks like you had ideas on what to write for wave 2, you got wave 1 in.
Uses controller tests to ensure correctness for check out and check in requirement, and that checked out counts and inventories appropriately change ⚠️ You got started on this, but it didn't look like it was finished.

Functional Requirements

Functional Requirement yes/no
All provided smoke tests for Wave 1 pass ✔️, Except for the error reporting because you're missing validations.
All provided smoke tests for Wave 2 pass ⚠️ NA

Overall Feedback

Overall Feedback Criteria yes/no
Green (Meets/Exceeds Standards) 3+ in Code Review && 2 in Functional Requirements 💚

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality Yes?
Perfect Indentation
Descriptive/Readable
Logical/Organized

Summary

This is probably a yellowish green submission due to the failing tests for creating a video. Take a look at my comments, but the main issues are missing validations, and limited tests for the additional parts you wrote.

That said you demonstrated you know how to recieve an API request and respond with JSON and the proper status code.

resources :customers, only: [:index, :show]
resources :videos, only: [:index, :show, :create]
# resources :rentals, only: [:index, :show]
post '/rentals/checkout', to: "rentals#checkout", as: 'checkout'

Choose a reason for hiding this comment

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

No route for check-in?

Comment on lines +2 to +3
resources :customers, only: [:index, :show]
resources :videos, only: [:index, :show, :create]

Choose a reason for hiding this comment

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

👍

@@ -0,0 +1,4 @@
class Customer < ApplicationRecord

Choose a reason for hiding this comment

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

No validations?

Comment on lines +6 to +9
def calculate_due_date(checkout_date, days_due)
date = checkout_date + (60 * 60 * 24 * days_due)
return date.strftime('%Y-%m-%d')
end

Choose a reason for hiding this comment

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

👍

has_many :rentals
has_many :customers, through: :rentals

validates :title, presence: true

Choose a reason for hiding this comment

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

Only this validation?

@@ -0,0 +1,7 @@
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.

Noting no tests

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

describe Video do

Choose a reason for hiding this comment

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

Noting no tests

Comment on lines +25 to +31
it "decrease the video's available_inventory by one" do

end

it 'should create a due date, 7 days from checkout date' do

end

Choose a reason for hiding this comment

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

Missing some tests

Comment on lines +71 to +73
it 'return back detailed errors and a status 400: Bad Request if the video does not have any available inventory before check out ' do

end

Choose a reason for hiding this comment

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

Good that you knew you needed this test.

end
end

describe "rental check-in" do

Choose a reason for hiding this comment

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

These would be the things you need to test.

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