Adahs branch #58
Adahs branch #58
Conversation
| { | ||
| "id": customer.id, | ||
| "name": customer.name, | ||
| "postal_code": customer.postal_code, | ||
| "phone": customer.phone, | ||
| } | ||
| ) |
There was a problem hiding this comment.
this repeated code could be moved to your Customer Model as an instance method or moved to a helper function. It could look something like after the instance method is created
| { | |
| "id": customer.id, | |
| "name": customer.name, | |
| "postal_code": customer.postal_code, | |
| "phone": customer.phone, | |
| } | |
| ) | |
| customer.to_customer_dict() | |
| ) |
|
|
||
|
|
||
| @customer_bp.route("", methods=["POST"]) | ||
| def put_customer(): |
There was a problem hiding this comment.
i would rename this to something like post_customer() because this a post method
| # @customer_bp.route("/customers/<customer_id>", methods=["DELETE"]) | ||
| # def delete_single_customer(customer_id): | ||
| # print("hiya") | ||
| # try: | ||
| # customer = Customer.query.get(customer_id) | ||
| # except: | ||
| # return jsonify(message=f"Customer {customer_id} was not found"), 404 | ||
|
|
||
| # db.session.delete(customer) | ||
| # db.session.commit() | ||
|
|
||
| # return jsonify(id=customer.id), 200 |
There was a problem hiding this comment.
when pushing the final code remove any unused code.
| if "name" not in request_body: | ||
| return jsonify(None), 400 | ||
| if "postal_code" not in request_body: | ||
| return jsonify(None), 400 |
There was a problem hiding this comment.
you could create a function called something like invalid_customer_info to handle the guard clauses here. You can use the same function in the Post method for customers
|
|
||
| # return jsonify(id=customer.id), 200 | ||
| @customer_bp.route("/<customer_id>", methods=["DELETE"]) | ||
| def delete_single_customer(customer_id): |
| if customer == None: | ||
| return jsonify(message=f"Customer {customer_id} was not found"), 404 | ||
|
|
||
| rental_list = Rental.query.filter_by(customer_id=customer.id, checked_out=True) |
There was a problem hiding this comment.
you already created a customer variable so you can use it here for line 137 like rental_list = customer.rentals and have a relationship in the customer model like rentals = db.relationship('Rental', back_populates='customer', lazy=True)
| phone = db.Column(db.String) | ||
| registered_at = db.Column(db.DateTime, default=date.today()) | ||
| # videos = db.relationship("Video", secondary="rental", backref="customers") |
There was a problem hiding this comment.
Like my previous comment I would suggest putting rentals = db.relationship('Rental', back_populates='customer', lazy=True) here
| # videos = db.relationship("Video", secondary="rental", backref="customers") | ||
|
|
||
| def customer_information(self): |
There was a problem hiding this comment.
wait you already had an instance method!! There were other places you could've used it 👀
| @@ -1,4 +1,16 @@ | |||
| from app import db | |||
| from datetime import timedelta, date | |||
| from datetime import datetime, timedelta | |||
|
|
|||
|
|
|||
| class Rental(db.Model): | |||
| from app import db | ||
|
|
||
|
|
||
| class Video(db.Model): |
| count_of_rentals = len(video.video_rentals) | ||
|
|
||
| video_avialable_inventory = video.total_inventory - count_of_rentals |
There was a problem hiding this comment.
another suggestion would be to add video_checkout_count = db.Column(db.Integer, default=0 to you Customer model then add to the count and subtract from the inventory count. for videos
| count_of_rentals = len(video.video_rentals) | |
| video_avialable_inventory = video.total_inventory - count_of_rentals | |
| customer.videos_checkout_count += 1 | |
| video.total_inventory -= 1 |
| today = datetime.today() | ||
| due_date = today + timedelta(days=7) |
There was a problem hiding this comment.
make one line like due_date = date.today() + timedelta(days=7)
| db.session.commit() | ||
|
|
||
| videos_customer_checked_out = Rental.query.filter_by(customer_id=customer.id, checked_out=True).count() | ||
| video_avialable_inventory = video.total_inventory - count_of_rentals |
There was a problem hiding this comment.
same suggestion from above you could add videos_checkout_count to your model then subtract from the count by doing customer.videos_checkout_count -= 1
| def get_all_videos(): | ||
| if request.method == "GET": | ||
| video = Video.query.all() | ||
| video_response = [video.video_information() for video in video] |
There was a problem hiding this comment.
okay, list comprehension! Think about other places you could implement this
| if "title" not in request_body: | ||
| return jsonify(details="Request body must include title."), 400 | ||
|
|
||
| if "release_date" not in request_body: | ||
| return jsonify(details="Request body must include release_date."), 400 | ||
|
|
||
| if "total_inventory" not in request_body: | ||
| return jsonify(details="Request body must include total_inventory."), 400 |
There was a problem hiding this comment.
you could create a function to handle these guard clauses dynamically and then use that function for customers and videos it could look something like
def validate_parameters(request_body, parameters):
for attribute in parameters:
if attribute not in request_body:
abort(make_response({"details": f"Request body must include {attribute}."}, 400))
| return jsonify(new_video.video_information()), 201 | ||
|
|
||
|
|
||
| @videos_bp.route("/<video_id>", methods=["GET"]) |
| return jsonify(video.video_information()), 200 | ||
|
|
||
|
|
||
| @videos_bp.route("/<video_id>", methods=["PUT"]) |
| return jsonify(video.video_information()), 200 | ||
|
|
||
|
|
||
| @videos_bp.route("/<video_id>", methods=["DELETE"]) |
| if video == None: | ||
| return jsonify(message=f"Video {video_id} was not found"), 404 | ||
|
|
||
| rental_list = Rental.query.filter_by(video_id=video.id, checked_out=True) |
There was a problem hiding this comment.
this could be videos.rentals if you add rentals = db.relationship('Rental', back_populates='video', lazy=True) to your video model
|
Great job Adah and Juliana! I liked how your files were organized by routes and models! I saw that you were using helper functions in some places. I added some comments on refactoring and using some more helper functions. Let me know if you have questions. |
No description provided.