Conversation
CheezItMan
left a comment
CheezItMan
left a comment
There was a problem hiding this comment.
Well done Areeba, you hit the learning goals here. Well done. I made a few minor comments, but this was a very solid submission. Nice work.
| from slack_sdk import WebClient | ||
| from slack_sdk.errors import SlackApiError |
There was a problem hiding this comment.
The Slack SDK is fine, but you can also do this with the built-in request object from Python.
|
|
||
| ################### | ||
| #Helper functions for tasks | ||
| def set_task(request_body): |
|
|
||
| return new_task | ||
|
|
||
| def format_task_dictionary(new_task): |
There was a problem hiding this comment.
This function, as I remember you mentioning, would work better as an instance method of the Task class.
| task = Task.query.get(task_id) | ||
|
|
||
| if task is None: | ||
| return make_response("Not Found", 404) |
There was a problem hiding this comment.
Just for consistency
| return make_response("Not Found", 404) | |
| return make_response(f"Task {task_id} not found", 404) |
| task = Task.query.get(task_id) | ||
| if task is None: | ||
| return make_response(f"Task {task_id} not found", 404) |
There was a problem hiding this comment.
I just want to point out that you are repeating this code segment a lot. It would be nice to be able to DRY this up a bit. In the future that might be something to research.
| def format_goal_dictionary(new_goal): | ||
| goal = {} | ||
| goal['id'] = new_goal.goal_id | ||
| goal['title'] = new_goal.title | ||
|
|
||
| return goal |
There was a problem hiding this comment.
This would also make an excellent model method.
|
|
||
| goal = Goal.query.get(goal_id) #grab the specific goal | ||
| if goal is None: | ||
| return make_response("", 404) |
There was a problem hiding this comment.
It would be best to include in the body some indication of why it failed in addition to the response code.
| response = {"id": goal.goal_id, "title": goal.title, "tasks": []} | ||
|
|
||
| for task in tasks: | ||
| task_dict = format_task_dictionary(task) | ||
| response["tasks"].append(task_dict) |
There was a problem hiding this comment.
This would make a great helper method in the Goal class.
| # def test_update_task_not_found(client): | ||
| # # Act | ||
| # response = client.put("/tasks/1", json={ | ||
| # "title": "Updated Task Title", | ||
| # "description": "Updated Test Description", | ||
| # }) | ||
| # response_body = response.get_json() | ||
|
|
||
| # # Assert | ||
| # assert response.status_code == 404 | ||
| # assert response_body == None |
There was a problem hiding this comment.
| # def test_update_task_not_found(client): | |
| # # Act | |
| # response = client.put("/tasks/1", json={ | |
| # "title": "Updated Task Title", | |
| # "description": "Updated Test Description", | |
| # }) | |
| # response_body = response.get_json() | |
| # # Assert | |
| # assert response.status_code == 404 | |
| # assert response_body == None |
I think you copied this over to use in writing the goal test, but it's not needed now.
| assert response.status_code == 404 | ||
| assert response_body == None | ||
| assert Goal.query.all() == [] | ||
|
|
There was a problem hiding this comment.
It might also be good to have tests for goals that have tasks and goals that do not have tasks.
2 participants
No description provided.