Conversation
CheezItMan
left a comment
CheezItMan
left a comment
There was a problem hiding this comment.
I strongly suggest that you describe functionality added when you make commit messages. Your commit messages mostly describe tests passed etc. Just describe what the code you wrote does. Also try to make small granular commits, maybe do one function and then add and commit.
Otherwise this is quite well done. I left some minor feedback. Let me know if you have any questions.
| __tablename__ = 'goal' | ||
| id = db.Column(db.Integer, primary_key=True, autoincrement=True) | ||
| title = db.Column(db.String) | ||
| #these are not just IDs, like a list of numbers...they are objects of type Task |
| def to_dict(self): | ||
| return { | ||
| "id": self.id, | ||
| "title": self.title, | ||
| } | ||
|
|
||
|
|
||
| def goal_tasks_to_dict(self): | ||
| mytasks = Task.query.all(self[0].tasks) | ||
| mylist = [] | ||
| for thing in mytasks: | ||
| mylist.append(thing.id) | ||
|
|
||
| return { | ||
| "id": self.id, | ||
| "task_ids": mylist | ||
| } No newline at end of file |
There was a problem hiding this comment.
I feel like these could be combined.
| completed_at = db.Column(db.DateTime, nullable=True) | ||
| goal_id = db.Column(db.Integer, db.ForeignKey("goal.id"), nullable=True) | ||
|
|
||
| def to_dict(self): |
| from dotenv import load_dotenv | ||
| import os | ||
|
|
||
| load_dotenv() |
There was a problem hiding this comment.
I think the load_dotenv() function has been run in the app/__init__.py file
| if request_body['completed_at'] == None: | ||
| return make_response({"task":new_task.to_dict()}), 201 | ||
| else: | ||
| return make_response({"task":new_task.to_dict()}), 201 |
There was a problem hiding this comment.
I'm not sure what the point of this if-else block is?
| my_task = Task.query.get(task_id) | ||
| if request.method == "GET": | ||
| if my_task is None: | ||
| return "", 404 |
There was a problem hiding this comment.
This is mostly a chris-ism, but I prefer having a meaningful message in the response body in addition to responding with the response code.
| else: | ||
| if completed_status == "mark_complete": | ||
| my_task.completed_at = datetime.now() | ||
| send_slack_notice(my_task.title) |
There was a problem hiding this comment.
I love the fact that you use a helper function here.
| if request.method == "POST": | ||
| request_body = request.get_json() | ||
| if "title" not in request_body: | ||
| return {"details": "Invalid data"}, 400 |
There was a problem hiding this comment.
Maybe respond with more detail, like
| return {"details": "Invalid data"}, 400 | |
| return {"details": "'title' is required"}, 400 |
| @@ -1,4 +1,5 @@ | |||
| import pytest | |||
| from app.models.goal import Goal | |||
2 participants
No description provided.