-
Notifications
You must be signed in to change notification settings - Fork 146
Cheetahs - Alexis Miller #125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on this project Alexis! This project is green based on the informal LP that we set up on 11/9, reducing the requirements to Waves 1-3.
description = db.Column(db.String(2600)) | ||
completed_at = db.Column(db.DateTime, nullable=True) | ||
|
||
def to_dict(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
||
#Validate | ||
def validate_task(task_id): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great helper!
|
||
if "title" not in request_body or "description" not in request_body: | ||
return jsonify({"details": "Invalid data"}), 400 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
task_response.append(task.to_dict()) | ||
|
||
return jsonify(task_response) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though Flask will add the 200 for you, I recommend being explicit about the code everywhere, and not just in non 200 returns. Consistent formats make code easier to read.
return jsonify(task_response) | |
return jsonify(task_response), 200 |
task = validate_task(task_id) | ||
|
||
return make_response({"task": task.to_dict()}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function make_response isn't incorrect here, but I notice that some functions return jsonify and others return make_response. I recommend picking one format and using it throughout your code - having different formats that do the same thing can make it harder for someone reading your code to realize that the behavior is the same.
task = validate_task(task_id) | ||
|
||
task.completed_at = date.today() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
# ***************************************************************** | ||
# **Complete test with assertion about response body*************** | ||
# ***************************************************************** | ||
|
||
assert response_body == {"message" : "Task ID '1' not found"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
No description provided.