Snow Leopards | Anika SW #142
Conversation
….py, adds to_dict method to Task model
…by task ID and appropriate HTTP status codes
…tes to_dict function so it will return properly
| from app.models.goal import Goal | ||
| from app.models.task import Task | ||
|
|
||
| goals_bp = Blueprint("goals", __name__, url_prefix="/goals") |
There was a problem hiding this comment.
This can just be called bp since you've only got one blueprint in this file and you have routes for each model in their own file.
|
|
||
| goals_bp = Blueprint("goals", __name__, url_prefix="/goals") | ||
|
|
||
| def validate_complete_request(request_body): |
There was a problem hiding this comment.
Nice error handling here, we can't expect users to always give us valid data and need to handle those scenarios afccordingly
| new_goal = Goal( | ||
| title=goal_data["title"], | ||
| ) | ||
| return new_goal |
There was a problem hiding this comment.
2 notes:
- You can use the cls keywork here instead of Goal
- You can also directly return a dictionary literal without creating the new_goal variable:
return cls(
title = dict_data["title"],
)| "title": self.title, | ||
| "description": self.description, | ||
| "is_complete": True | ||
| } |
There was a problem hiding this comment.
Notice in to_dict_get_patch, you have several lines of code repeated in each dictionary literal you're returning. What if you used a variable and built up a dictionary using conditional logic and return the dictionary at the end of the function to avoid repetition?
This example flips the logic a little bit but you can see how we build up updated_task so we don't need to repeat a bunch of dictionary literals.
updated_task = {
"id": self.id,
"title": self.title,
"description": self.description
}
if not self.completed_at:
updated_task["is_complete"]: False
if self.goal_id:
updated_task["goal_id"]: self.goal_id
# more logic here to add whatever key/value pairs are needed for updated_task
return updated_task| if title_query: | ||
| goal_query = Goal.query.filter_by(title=title_query) |
|
|
||
| tasks = [] | ||
| for task in goal.tasks: | ||
| tasks.append( |
There was a problem hiding this comment.
Consider validating task before appending it to tasks, like:
for task in goal.tasks:
task = validate_model_id(Task, task.id)
tasks.append(task)then you don't need to write out the dictionary literal that represents a task because validate_model_id returns an instance of task.
| "goal_id": task.goal_id, | ||
| "title": task.title, | ||
| "description": task.description, | ||
| "is_complete": False |
There was a problem hiding this comment.
If we're getting all tasks associated with an goal, do we need to hard code is_complete to False? Could a goal have a list of tasks where some are completed and some are not?
| for task in goal.tasks: | ||
| task_response.append( |
There was a problem hiding this comment.
If we just want to get a specific task by id, then we don't want to go through all the tasks in goal.tasks because then we'd end up with a list of all the tasks associated with the goals.
This route repeats the logic in task_routes.py on lines 72-79 where you get a task by id. Here you nest the route under goals. It's up to the developer to decide if you want to have the request to get a single task by ID nested under goals/goal_id/tasks/task_id or if you can just a route that is /tasks/task_id
|
|
||
| @tasks_bp.route("", methods=["GET"]) | ||
| def get_all_tasks_sort_asc(): | ||
| task_query = Task.query.all() |
There was a problem hiding this comment.
Here, we should have task_query = Task.query without all() because we dont actually want to go query everything yet. We want to see which request params were provided to the request and then filter or order accordingly. Finally if no request params were provided then we'd want to query for all tasks near the end of this method.
| if sort_query == "desc": | ||
| task_query = Task.query.order_by(Task.title.desc()) | ||
|
|
||
| tasks = task_query |
There was a problem hiding this comment.
Here is where we'd want to query for all tasks if not query params were provided so line 64 should be
tasks = task_query.all()
# rest of logic 
No description provided.