Scissors: Mai Truong #55
Conversation
| __tablename__ = "customer" | ||
| id = db.Column(db.Integer, primary_key=True, autoincrement=True) | ||
| name = db.Column(db.String) | ||
| postal_code = db.Column(db.Integer) |
There was a problem hiding this comment.
This solution is fine but it causes the code to fail 2 smoke tests (there was inconsistency around the postal_code in the tests).
| if customer == None: | ||
| return jsonify({"details": "customer not found"}), 404 |
There was a problem hiding this comment.
This code is the same as lines 30 & 31 - consider pulling them out into a helper function. The Flask abort function might be useful in this case, it makes it possible to return a response directly from a helper function.
| errors = "" | ||
| information = ["name", "postal_code", "phone"] | ||
| for info in information: | ||
| if info not in request_body: | ||
| errors = errors + info | ||
| if errors != "": | ||
| return jsonify({"details": errors}), 400 |
There was a problem hiding this comment.
Nice! This is a great candidate for a helper function, and could be used for error checking on video as well if the list of fields is passed as a variable to the function.
| postal_code = db.Column(db.Integer) | ||
| phone_number = db.Column(db.String) | ||
| registered_at = db.Column(db.DateTime, nullable=True) | ||
| videos_checked_out_count = db.Column(db.Integer, default=0) |
There was a problem hiding this comment.
Could you pull videos_checked_out_count from the rental table rather than storing it directly in the customer table?
| title = db.Column(db.String) | ||
| release_date = db.Column(db.DateTime, nullable=True) | ||
| total_inventory = db.Column(db.Integer) | ||
| available_inventory = db.Column(db.Integer) |
There was a problem hiding this comment.
Could you pull available_inventory from the rental table rather than storing it directly in this table?
| customer_id = request_body.get("customer_id") | ||
| video_id = request_body.get("video_id") | ||
|
|
||
| if type(customer_id) != int or type(video_id) != int: | ||
| return jsonify({"details": "Invalid data"}), 400 | ||
|
|
||
| customer = Customer.query.get(customer_id) | ||
| video = Video.query.get(video_id) | ||
|
|
||
| if customer is None and video is None: | ||
| return jsonify({"details": "id's do not exist"}), 404 | ||
|
|
||
| if video.available_inventory < 1: | ||
| return jsonify({"details": "2Invalid data"}), 400 |
| for rental in customer.video: | ||
| if rental.video_id == video_id: | ||
| customer.videos_checked_out_count -= 1 | ||
| video.available_inventory += 1 | ||
| db.session.delete(rental) | ||
| db.session.commit() |
There was a problem hiding this comment.
This will work, but it is also possible to pull the rental from the database directly. This query Rental.query.filter_by(customer_id = customer_id, video_id=video_id).all() is a place to start.
|
Fantastic work on this project! All of the functionality is solid. The majority of my comments focus on ways you can DRY up your code. This project is green, you hit all of the learning goals. |
No description provided.