Skip to content

Cheetahs - Ev#129

Open
evelynburrito wants to merge 8 commits intoAda-C18:masterfrom
evelynburrito:master
Open

Cheetahs - Ev#129
evelynburrito wants to merge 8 commits intoAda-C18:masterfrom
evelynburrito:master

Conversation

@evelynburrito
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@nancy-harris nancy-harris left a 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!

Comment thread app/models/task.py
@@ -2,4 +2,29 @@


class Task(db.Model):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

Comment thread app/routes.py

tasks_bp = Blueprint("tasks", __name__, url_prefix="/tasks")

@tasks_bp.route("", methods=["POST", "GET"])
Copy link
Copy Markdown

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.

Comment thread app/routes.py
elif request.method == "GET":
sort_by_query = request.args.get('sort')

tasks = Task.query.all()
Copy link
Copy Markdown

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.

Comment thread app/routes.py
Comment on lines +12 to +25
request_body = request.get_json()

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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

Comment thread app/routes.py

return make_response({"details": f'Task {task.task_id} ' f'"{task.title}"' ' successfully deleted'}, 200)

def validate_model_id(cls,task_id):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay, helper function!

Comment thread tests/test_wave_01.py
assert response.status_code == 404

raise Exception("Complete test with assertion about response body")
assert response_body == {"message":"Task 1 not found"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

Comment thread tests/test_wave_01.py

raise Exception("Complete test with assertion about response body")
# *****************************************************************
assert response_body == {"message":"Task 1 not found"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

Comment thread tests/test_wave_01.py
assert response.status_code == 404

raise Exception("Complete test with assertion about response body")
assert response_body == {"message":"Task 1 not found"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

Comment thread tests/test_wave_03.py
assert response.status_code == 404

raise Exception("Complete test with assertion about response body")
assert response_body == {"message": "Task 1 not found"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

Comment thread tests/test_wave_03.py
assert response.status_code == 404

raise Exception("Complete test with assertion about response body")
assert response_body == {"message": "Task 1 not found"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants