-
Notifications
You must be signed in to change notification settings - Fork 146
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
Cheetahs - Ev #129
base: master
Are you sure you want to change the base?
Cheetahs - Ev #129
Conversation
…ute to pass tests
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.
Excellent job! Your code overall is great! There are a few comments below on ways to make the code easier to read, but otherwise you did a great job!
@@ -2,4 +2,29 @@ | |||
|
|||
|
|||
class Task(db.Model): |
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.
Excellent!
tasks_bp = Blueprint("tasks", __name__, url_prefix="/tasks") | ||
|
||
@tasks_bp.route("", methods=["POST", "GET"]) |
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.
It would be a good idea to separate "POST" and "GET". Putting them together makes the function really long and harder to read since everything is indented an extra level because of the if statement to distinguish between POST and GET.
sort_by_query = request.args.get('sort') | ||
|
||
tasks = Task.query.all() |
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.
It would be a good idea to have this as the else
that comes at the end of the if
statement that starts on line 33. The way it's done now, all of the tasks are pulled twice if there's a sort to do.
|
||
try: | ||
task = Task(title = request_body['title'], description = request_body['description']) | ||
except KeyError: | ||
invalid_dict = {"details": "Invalid data"} | ||
return make_response(jsonify(invalid_dict),400) | ||
|
||
db.session.add(task) | ||
db.session.commit() | ||
|
||
response_body = {"task": task.to_dict()} | ||
|
||
return make_response(jsonify(response_body), 201) |
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.
Excellent!
return make_response({"details": f'Task {task.task_id} ' f'"{task.title}"' ' successfully deleted'}, 200) | ||
|
||
def validate_model_id(cls,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.
Yay, helper function!
def test_get_task_not_found(client): | ||
# Act | ||
response = client.get("/tasks/1") | ||
response_body = response.get_json() | ||
|
||
# Assert | ||
assert response.status_code == 404 | ||
|
||
raise Exception("Complete test with assertion about response body") | ||
assert response_body == {"message":"Task 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.
Excellent!
@@ -131,13 +130,13 @@ def test_update_task_not_found(client): | |||
# Assert | |||
assert response.status_code == 404 | |||
|
|||
raise Exception("Complete test with assertion about response body") | |||
# ***************************************************************** | |||
assert response_body == {"message":"Task 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.
Excellent!
@@ -161,15 +160,15 @@ def test_delete_task_not_found(client): | |||
# Assert | |||
assert response.status_code == 404 | |||
|
|||
raise Exception("Complete test with assertion about response body") | |||
assert response_body == {"message":"Task 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.
Excellent!
@@ -128,13 +128,13 @@ def test_mark_complete_missing_task(client): | |||
# Assert | |||
assert response.status_code == 404 | |||
|
|||
raise Exception("Complete test with assertion about response body") | |||
assert response_body == {"message": "Task 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.
Excellent!
@@ -143,7 +143,7 @@ def test_mark_incomplete_missing_task(client): | |||
# Assert | |||
assert response.status_code == 404 | |||
|
|||
raise Exception("Complete test with assertion about response body") | |||
assert response_body == {"message": "Task 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.
Excellent!
No description provided.