Skip to content

Conversation

@emilyvomacka
Copy link

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 got column names for customers and movies from the seed data, and then reasoned about what rentals should hold based on the functionality the spec described.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning. Index endpoints are O(n), where n is the number of items in the db. Show is O(1), because we think that lookup is direct using id/the primary key.
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(n), because we're querying each item in the rental db to see if it has the correct combo of movie_id and customer_id. (We could probably make this more efficient, in retrospect)
Describe a set of positive and negative test cases you implemented for a model. Movie can have many rentals or no rentals.
Describe a set of positive and negative test cases you implemented for a controller. Movie can't be rented with 0 available copies, movie can be rented with >0 available copies.
How does your API respond when bad data is sent to it? It responds with an error json, and (in instances where smoke test didn't determine result) we chose to respond bad_request.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We have a customer.movies_checked_out_update method, which allows the model to update its movies_checked_out_count so this business logic is not handled by the controller.
Do you have any recommendations on how we could improve this project for the next cohort? We thought the ambiguity was good to think through. We would edit smoke tests to not reflect optional objectives.
Link to Trello We used slack frequently
Link to ERD We used paper and pencil!
Summary

stupendousC and others added 30 commits November 5, 2019 12:57
set up models, controllers, and schema
emilyvomacka and others added 28 commits November 7, 2019 22:53
@CheezItMan
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good granular commits and good commit messages
Comprehension questions Check
General
Business Logic in Models Not much here, at all
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 Check, although you are missing some bonus content tests
Overall Really nice work, you hit the learning goals here. You even got a lot of the optionals. Well done.

@@ -0,0 +1,93 @@
class ApplicationController < ActionController::API

def validate_query_params(acceptable_keys:)

Choose a reason for hiding this comment

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

interesting method!

Comment on lines +86 to +92
def get_active_rentals_from(instance:)
return instance.rentals.select { |rental| !rental.returned }
end

def get_past_rentals_from(instance:)
return instance.rentals.select { |rental| rental.returned }
end

Choose a reason for hiding this comment

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

These seem like they should be model methods.

@@ -0,0 +1,73 @@
class RentalsController < ApplicationController

Choose a reason for hiding this comment

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

It might be useful to move some of the business logic from this controller into model 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