Scissors - Araceli #68
Conversation
| def to_json(self): | ||
| return { | ||
| "id": self.goal_id, | ||
| "title": self.title, | ||
| } |
There was a problem hiding this comment.
This is a great helper function, but I suspect it wasn't working as you expected because the tab level is outside of the class.
| def to_dict(self): | ||
| return { | ||
| "id": self.task_id, | ||
| "title": self.title, | ||
| "description": self.description, | ||
| "is_complete": bool(self.completed_at) | ||
| } | ||
|
|
||
| def to_dict_goal(self): | ||
| return { | ||
| "id": self.task_id, | ||
| "goal_id": self.goal_id, | ||
| "title": self.title, | ||
| "description": self.description, | ||
| "is_complete": bool(self.completed_at) | ||
| } |
There was a problem hiding this comment.
Same issue as Goal, but these are great helper functions!
|
|
||
| return jsonify(response), 400 | ||
|
|
||
| else: |
There was a problem hiding this comment.
It's not necessary to include this else statement. If the if statement passes, the function will return and the code after this point will not run.
| if sort_query == "asc": | ||
| tasks = Task.query.order_by(Task.title.asc()) | ||
|
|
||
| elif sort_query == "desc": | ||
| tasks = Task.query.order_by(Task.title.desc()) | ||
|
|
||
| else: | ||
| tasks = Task.query.all() |
| return jsonify(tasks_response), 200 | ||
|
|
||
|
|
||
| @tasks_bp.route("/<task_id>", methods=["GET", "PUT", "DELETE"], strict_slashes=False) |
There was a problem hiding this comment.
In the "" route above (lines 17 & 37), the methods are split into separate functions, while here they're combined into the same method. Both ways are fine, but for readability, I recommend following the same patterns throughout your code.
|
|
||
| if request.method == "GET": | ||
| if task is None: | ||
| return make_response(f"404 Not Found", 404) |
There was a problem hiding this comment.
Having an error message is great, but I would suggest having a more meaningful error message, something like 'Task with ID X not found'. Also this test is something you are doing for all of the methods - it could be moved up before the checks for the type of method so it only needs to be done once.
| else: | ||
| one_task = to_dict(task) | ||
|
|
||
| return {"task": one_task} |
There was a problem hiding this comment.
For readability I recommend using the same format to create your responses throughout your project. Lines 56, 66 and 71 all use a different format. Flask handles all of these formats gracefully, but having multiple formats can make it seem that different things are happening when in fact they are all more or less the same. Also, I recommend explicitly setting a status code everywhere so that you can accurately predict what your API will do in each situation.
| updated_task = { | ||
| "id": task.task_id, | ||
| "title": task.title, | ||
| "description": task.description, | ||
| "is_complete": bool(task.completed_at) | ||
| } |
There was a problem hiding this comment.
This would be a great place to use use the to_dict helper function.
| return make_response(f"404 Not Found", 404) | ||
|
|
||
| else: | ||
| one_task = to_dict(task) |
There was a problem hiding this comment.
Here is where the last test fails - if the task has a valid goal_id, it should include the goal_id in the json output. One way to handle this is:
if task.goal_id:
one_task = to_dict_goal(task)
else:
one_task = to_dict(task)
| # for task in tasks: | ||
| # tasks_list.append(to_dict_goal(task)) | ||
|
|
||
| task_list = [to_dict_goal(task) for task in tasks] |
|
Great work on this project, you hit all the learning goals. Nice work with your to_json & to_dict helper functions! |
No description provided.