Conversation
tgoslee
left a comment
There was a problem hiding this comment.
Great job San! Your code was organized and the use of helper functions made your code readable. I left some comments on some things you could refactor. Let me know if you have any questions.
| from .routes import task_bp | ||
| from .routes import goal_bp |
There was a problem hiding this comment.
you could seperate the routes for tasks and goals into two separate files. So you can have task_routes.py and goal_routes.py
| class Goal(db.Model): | ||
| goal_id = db.Column(db.Integer, primary_key=True) | ||
| title = db.Column(db.String) | ||
| tasks = db.relationship("Task", lazy = True, backref="goal") |
There was a problem hiding this comment.
Because the default value for lazy is True you could technically leave it off. More info here https://docs.sqlalchemy.org/en/14/orm/loading_relationships.html
| title = db.Column(db.String) | ||
| tasks = db.relationship("Task", lazy = True, backref="goal") | ||
|
|
||
| def to_dict(self): |
| task_id = db.Column(db.Integer, primary_key=True) | ||
| title = db.Column(db.String, nullable=False) | ||
| description = db.Column(db.String, nullable=False) | ||
| completed_at = db.Column(db.DateTime, nullable=True) |
There was a problem hiding this comment.
good job adding nullable=True here because completed_at doesn't have to exist
| def dict(self): | ||
| return (dict(task={ | ||
| "id" : self.task_id, | ||
| "title": self.title, | ||
| "description" : self.description, | ||
| "is_complete" : bool(self.completed_at), | ||
| "goal_id" : self.goal_value | ||
|
|
||
| })) | ||
|
|
||
| def to_dict(self): | ||
| return (dict(task={ | ||
| "id" : self.task_id, | ||
| "title": self.title, | ||
| "description" : self.description, | ||
| "is_complete" : bool(self.completed_at) | ||
| })) No newline at end of file |
There was a problem hiding this comment.
you can refactor this to one instance method
response_dict = dict(
id=self.task_id,
title=self.title,
description=self.description,
is_complete= bool(self.completed_at)
)
if self.goal_id:
response_dict["goal_id"] = self.goal_id
return response_dict| else: | ||
| return goal | ||
|
|
||
| #-------------------------- GOAL --------------------------# |
There was a problem hiding this comment.
I do like that you added these comments to organize this file
| new_goal = Goal(title = request_body['title'] | ||
| ) | ||
| except: | ||
| abort(make_response({"details":f"Invalid data"}, 400)) |
There was a problem hiding this comment.
think about how you can add more details to your error message Invalid Data: Title is missing. I think this helps the user understand what's causing the error.
| }},201 | ||
|
|
||
| @goal_bp.route("/<goal_id>/tasks", methods=["POST"]) | ||
| def create_goal_list(goal_id): |
| db.session.commit() | ||
|
|
||
| return { | ||
| "task": { |
There was a problem hiding this comment.
didn't you create an instance method you could use here?
| { | ||
| "id":task.task_id, | ||
| "title":task.title, | ||
| "description":task.description, | ||
| "is_complete": False | ||
| } |
No description provided.