Maple - Detroit and Monica #55
Conversation
…. Started GET responses.
…ment for test_get_invalid_video_id.
corrected to registered_at
… with Rental), started rental endpoints.
… Finished GET request for customer_bp.
…rom master branch
| phone = db.Column(db.String) | ||
| registered_at = db.Column(db.DateTime, nullable=True) | ||
| # videos_checked_out_count = db.Column(db.Integer, default=0, nullable=False) |
There was a problem hiding this comment.
I would actually add this column here! You could use it to calculate inventory after someone checks in and checks out a video
|
|
||
| def get_videos_checked_out_count(self): | ||
| from app.models.rental import Rental |
There was a problem hiding this comment.
best practice to have imports at the top of your file. I would move it there
| customers = db.relationship('Customer', secondary='rentals', backref='video') | ||
|
|
||
| def calculate_available_inventory(self): |
There was a problem hiding this comment.
If you add videos_checkout_count to your customer model then you could do something like
customer.videos_checkout_count += 1
self.total_inventory -= 1
| if 'title' not in request_body.keys(): | ||
| return make_response({"details": "Request body must include title." }, 400) | ||
| elif 'release_date' not in request_body.keys(): | ||
| return make_response({"details":"Request body must include release_date."}, 400) | ||
| elif 'total_inventory' not in request_body.keys(): | ||
| return make_response({"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))
| # The try/except clause coerces the typed argument to be an int. If it is a non-numeric value, then a ValueError will be raised. | ||
| # Alternatively, can also use isnumeric(). This will return a boolean, so no need for try/except. An if/else clause will suffice. | ||
| try: | ||
| id = int(video_id) | ||
| except ValueError: | ||
| return jsonify({"details": "Invalid data"}), 400 |
| return make_response({"id": video.id, | ||
| "title": video.title, | ||
| "release_date": video.release_date, | ||
| "total_inventory":video.total_inventory}) |
There was a problem hiding this comment.
this repeated code could be moved to your Video Model as an instance method or moved to a helper function. It could look something like after the instance method is created
| return make_response({"id": video.id, | |
| "title": video.title, | |
| "release_date": video.release_date, | |
| "total_inventory":video.total_inventory}) | |
| return make_response({ | |
| video.video_to_dict() | |
| }) |
| # elif 'registered_at' not in request_body.keys(): | ||
| # return make_response({"details": "Request body must include registered_at."}, 400) |
| # new_rental = Rental(video_id=request_body["video_id"], | ||
| # customer_id=request_body["customer_id"]) | ||
| # updated_videos_checked_out = Customer.query.filter_by(id=request_body['customer_id']).first().videos_checked_out_count +1 | ||
| # Customer.query.filter_by(id=request_body['customer_id']).first().videos_checked_out_count | ||
|
|
||
|
|
||
|
|
||
| # return make_response({"customer_id": request_body["customer_id"] , | ||
| # "video_id": request_body["video_id"]}) | ||
|
|
There was a problem hiding this comment.
remove unused code when pushing final code. If you want to keep this code, I would move it to a readME file.
|
Great Job Monica and Detroit! Your code was readable and passes all tests. I added some comments on refactoring to use helper functions and/or instance methods to your models. Also, it's best practice to remove unused code and to put your import statements at the top of your file. Let me know if you have any questions. |
No description provided.