Skip to content

Leaves - Mariya, Cloudy#32

Open
M-Burr wants to merge 20 commits intoAda-C12:masterfrom
M-Burr:master
Open

Leaves - Mariya, Cloudy#32
M-Burr wants to merge 20 commits intoAda-C12:masterfrom
M-Burr:master

Conversation

@M-Burr
Copy link

@M-Burr M-Burr 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. After reading the requirements and creating our ERD, we hoped to have rentals be a joined table without a model, but after working through wave 1, we decided rentals should be its own model so that we could add business logic to rentals. In summary, our original ERD was wrong and we adapted as we progressed through the project.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning. The O time complexity for customers and movies is: O(n), where "n" represents the total number of customer objects or movie objects (i.e., the hashes in the array in json). The amount of time the function takes is proportional to n items (i.e., it is a linear relationship).
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(1) bc it always display an error (to us, not postman). This result is constant, regardless of the size of the input, therefore it is O(1) (constant).
Describe a set of positive and negative test cases you implemented for a model. We tested model validations to see if requirements were present.
Describe a set of positive and negative test cases you implemented for a controller. We tested to see if a rental checkout was successful and confirm that a rental would fail with invalid data.
How does your API respond when bad data is sent to it? returns a hash where the key("ok") has a value of false and they key("errors") has a description of what errors are present
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We did not use custom model methods.
Do you have any recommendations on how we could improve this project for the next cohort? n/a - great project!
Link to Trello NA
Link to ERD We did it on paper
Summary It was a fun project and we learned a lot. Thanks!

end
end

def checkin
Copy link

@beccaelenzil beccaelenzil Nov 12, 2019

Choose a reason for hiding this comment

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

Consider implementing Checkin so that it does not create new rental but checks in an existing rental. You'd need to first find the rental and then checkin. Some of this logic would go in a custom model method.

Choose a reason for hiding this comment

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

In addition, checkin and checkout should affect available movie inventory -- this will likely be a custom method in the movie model.

Choose a reason for hiding this comment

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

I realize now that movie inventory is optional -- I talked about this with Cloudy

render json: movie.as_json(only: [:id, :title, :overview, :release_date, :inventory]), status: :ok
return
else
render json: { ok: false, "errors" => ["Not Found"]}, status: :not_found

Choose a reason for hiding this comment

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

Instead of "error" --> ["Not Found"] you should report any errors in the movie model created by validations by accessing movie.errors.messages.

avengers = movies(:avengers)
zootopia = movies(:zootopia)

#Act

Choose a reason for hiding this comment

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

It seems you don't use the variable rental. As such, you don't need this assignment.


# Assert
expect(mariya.movies.count).must_equal 1
end

Choose a reason for hiding this comment

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

Consider the case where a movie has no rentals or customers.

end
end

describe "validations" do

Choose a reason for hiding this comment

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

thorough validation tests. nice work!


it "has rentals" do
expect(@up.rentals.count).must_equal 4
end

Choose a reason for hiding this comment

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

Also consider the edge cases of having no rentals or customers.

@beccaelenzil
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes
General
Business Logic in Models business logic to update available inventory is missing (from model or controller)
All required endpoints return expected JSON data yes
Requests respond with appropriate HTTP Status Code yes
Errors are reported see comment
Testing
Passes all Smoke Tests wave 2- all passed, wave 3- 3 failures (1) Customer's movie check out count increased
Model Tests - all relations, validations, and custom functions test positive & negative cases consider a few additional edge cases for relationships
Controller Tests - URI parameters and data in the request body have positive & negative cases yes
Overall Good work creating a web API with model relationships and validations. I am impressed by the thoroughness of your controller tests. You are missing the key feature of updating the available inventory with the checkin and checkout actions. Take a look at my inline comments, and the requirements and smoke tests with regards to this feature, and let me know what questions you have.

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.

2 participants