Cedar - Ana S. & Maria O. #37
Conversation
…tered_at, and to_dict method
Create video Model and /videos endpoints
create routes folder
er' of https://github.com/mob regong/retro-video-store Co-authored-by: mobregong <[email protected]>
…get_video_history
…routes, Implement routes and pass the tests
There was a problem hiding this comment.
Great work implementing this complex Flask project. You made great use of helper functions and stretched yourself to write tests for optional features. I've left some in-line comments focused on ways that you could use instance methods and class methods to DRY up your code and simplify the logic in the route functions. Please let me know if you have any questions. Nice work!
| videos_checked_out_count = db.Column(db.Integer, nullable=True) |
There was a problem hiding this comment.
Consider calculating available inventory based of the number of rentals a video currently has. For instance something like, for the video model: available_inventory = len([rental for rental in self.rentals if not rental.check_in_date])
This method requires that with have a rentals relationships column on our video: rentals = db.relationship('Rental', back_populates='customer', lazy=True)
This method of dynamically computing available_inventory rather than storing it in an instance variable makes it so there isn’t information stored in multiple places that could potentially conflict. Also something to note is that available_inventory is more of an attribute of a video rather than rental
| due_date = db.Column(db.DateTime(timezone=True),nullable=True) | ||
| available_inventory = db.Column(db.Integer, nullable=False) | ||
| videos_checked_out_count = db.Column(db.Integer, nullable=True) |
There was a problem hiding this comment.
Similar to available_inventory, consider how you can dynamically calculate videos_checked_out_count for each customer by checking the length of the rentals attribute for a customer instance.
| total_inventory = db.Column(db.Integer(), nullable=False) | ||
| # relationship | ||
| customers = db.relationship("Customer", secondary="rental", backref="video",lazy=True) |
There was a problem hiding this comment.
Nice work creating this secondary relationship column!
| response_list = [] | ||
| for customer in customer_objects: | ||
| response_list.append(customer.to_dict()) |
There was a problem hiding this comment.
This could be a nice place to use a list comprehension
| if "name" in request_body: | ||
| selected_customer.name = request_body["name"] | ||
| else: | ||
| abort(make_response({"error": "Invalid input"},400)) | ||
| if "phone" in request_body: | ||
| selected_customer.phone = request_body["phone"] | ||
| else: | ||
| abort(make_response({"error": "Invalid input"},400)) | ||
| if "postal_code" in request_body: | ||
| selected_customer.postal_code = request_body["postal_code"] | ||
| else: | ||
| abort(make_response({"error": "Invalid input"},400)) |
There was a problem hiding this comment.
Note the similarities between this code and the code on lines 30-38. Consider how you could use a helper function to DFRY up your code.
| response_body = rentals.to_dict() | ||
| return make_response(response_body, 200) | ||
|
|
||
| @rentals_bp.route("/overdue", methods=["GET"], strict_slashes=False) |
There was a problem hiding this comment.
Great work implementing this route!
| @video_bp.route("/<video_id>", methods=["DELETE"]) | ||
| def delete_video(video_id): | ||
| video = get_video_by_id(video_id) | ||
| rentals = Rental.query.filter_by(video_id=video_id).all() |
There was a problem hiding this comment.
In a previous comment, I mentioned you could make a rentals relationship column for the video. This is a place it could be helpful.
| response_body.append({"due_date": rental.due_date, | ||
| "name": customer.name, | ||
| "phone": customer.phone, | ||
| "postal_code":customer.postal_code}) |
There was a problem hiding this comment.
Consider using an instance method to create this dictionary.
| @@ -0,0 +1,102 @@ | |||
| from app.models.video import Video | |||
There was a problem hiding this comment.
Great work implementing these tests. You mentioned that they're not all passing yet. It is a great practice to write tests before you implement the feature. Bravo!
Co-authored-by: beccaelenzil-teach <[email protected]>
Co-authored-by: beccaelenzil-teach <[email protected]>
We are not passing all of the tests on wave 3 because we did not have time to implement all of the optional enhancements.