Conversation
… modified task model logic for calculating is_complete
…lf-written tests)
…e tasks of one goal, Wave 06 passing except for last test
…_task so that goal_id is included in response if the task is related to a goal, passing last test on Wave 06
beccaelenzil
left a comment
beccaelenzil
left a comment
There was a problem hiding this comment.
Great work on this complex Flask project! You've met the learning goals around writing tests and creating relationships with SQLAlchemy. Nice work implementing instance methods. I've left a few in-line comments on ways you may consider refactoring. Please let me know if you have any questions. Nice work!
| def to_dict(self): | ||
| return { | ||
| "id": self.task_id, | ||
| "title": self.title, | ||
| "description": self.description, | ||
| # "is_complete": True if self.completed_at == True else False | ||
| "is_complete": bool(self.completed_at) | ||
| } |
There was a problem hiding this comment.
Consider how you could refactor this to include the goal_id when self.goal_id is not null (None), then look below for one possible solution.
dict = {
"id": self.task_id,
"title": self.title,
"description": self.description,
# "is_complete": True if self.completed_at == True else False
"is_complete": bool(self.completed_at)
}
if self.goal_id:
dict["goal_id"] = self.goal_id
return dict
| "is_complete": bool(self.completed_at) | ||
| } | ||
|
|
||
| def is_related_to_goal(self): |
There was a problem hiding this comment.
This is related to goal! Note how the suggested refactor above negates the need for this check.
| # relationship | ||
| tasks = db.relationship("Task", backref="goal", lazy=True) | ||
|
|
||
| def to_dict(self): |
There was a problem hiding this comment.
Good work making these instance methods
| goals_bp = Blueprint('goals', __name__, url_prefix='/goals') | ||
|
|
||
| # # Helper functions | ||
| def valid_int(id): |
There was a problem hiding this comment.
Note that you have this function in goal_routes and task_routes. Consider making a separate file with shared helper functions and importing them where you need them to DRY up your code.
| @goals_bp.route("", methods=["POST"], strict_slashes=False) | ||
| def create_goal(): | ||
| request_body = request.get_json() | ||
| valid_goal(request_body) |
There was a problem hiding this comment.
Great use of a helper function. You might consider also including the creation of the goal in valid_goal such that the function either aborts because of Invalid data or returns the goal instance.
| for goal in goal_objects: | ||
| response_list.append(goal.to_dict()) |
There was a problem hiding this comment.
This could be a nice place to use a list comprehension.
| task_ids_list = request_body["task_ids"] | ||
|
|
||
| for task_id in task_ids_list: | ||
| task = Task.query.get(task_id) |
There was a problem hiding this comment.
We should probably double check that the task ids in the request body actually correspond to existing tasks. We can do that with a get_or_404 or a check on whether the task is None.
Also note that add_tasks_to_goal_dict does the work of creating a list of task ids. That list has already been handed in as part of the request body, so this extra work is not necessary.
| @tasks_bp.route("", methods=["POST"], strict_slashes=False) | ||
| def create_task(): | ||
| request_body = request.get_json() | ||
| valid_task(request_body) |
There was a problem hiding this comment.
Just like with goal, consider using valid_task to return a task instance.
| "title": "Updated Goal Title" | ||
| } | ||
| } | ||
| goal = Goal.query.get(1) |
There was a problem hiding this comment.
Nice work querying the database to make sure the change was persisted there.
2 participants
No description provided.