Conversation
utes file still present but commented out
…correct in Postman
yangashley
left a comment
yangashley
left a comment
There was a problem hiding this comment.
Great work on task-list-api! Let me know if you have questions about my review comments
| @@ -1,5 +1,26 @@ | |||
| from app import db | |||
There was a problem hiding this comment.
Remember to make an empty init.py file in any package folder/subfolder. app and routes have one, but we should have one here in the models folder as well.
|
|
||
|
|
||
| class Goal(db.Model): | ||
| goal_id = db.Column(db.Integer, primary_key=True) |
There was a problem hiding this comment.
My personal preference is to name id attributes just id and then when you have a goal object you can write goal.id instead of goal.goal_id.
| if tasks: | ||
| build_dict["tasks"] = [task.to_dict() for task in self.tasks] |
There was a problem hiding this comment.
Nicely done! You could even remove tasks=False from the parameter and have the check on line 17 be if self.tasks: instead
| if tasks: | ||
| build_dict["tasks"] = [task.to_dict() for task in self.tasks] | ||
|
|
||
| return build_dict |
There was a problem hiding this comment.
A more descriptive name for this dictionary might be goal_dict or goal_response_dict
|
|
||
| class Task(db.Model): | ||
| task_id = db.Column(db.Integer, primary_key=True) | ||
| task_id = db.Column(db.Integer, primary_key=True, autoincrement=True) |
There was a problem hiding this comment.
Same comment as in goal, prefer primary key just to be called id
| return make_response({"details": "Invalid data"}, 400) | ||
|
|
||
|
|
||
| def get_all_items(cls): |
| try: | ||
| new_item = cls.from_dict(request_body) | ||
| db.session.add(new_item) | ||
| db.session.commit() | ||
| return make_response({cls.__name__.lower(): new_item.to_dict()}, 201) | ||
| except: | ||
| return make_response({"details": "Invalid data"}, 400) |
There was a problem hiding this comment.
Nice error handling in case the request_body has invalid data
| load_dotenv() | ||
|
|
||
| def send_slack_message(completed_task): | ||
| TOKEN = os.environ['SLACK_API_TOKEN'] |
There was a problem hiding this comment.
You'll work on APIs where a route will have to make more than one external call to an API so we should be more specific about the kind of token we're using. This variable could be called SLACK_API_TOKEN
| try: | ||
| message = f"Someone just completed the task {completed_task.title}" | ||
| payload = { | ||
| "channel": CHANNEL_ID, | ||
| "text": message | ||
| } | ||
|
|
||
| requests.post(SLACK_URL, data = payload, headers = AUTH_HEADERS) | ||
|
|
||
| except: | ||
| print("There was an error making the call to Slack") |
| from app import db | ||
| import os | ||
|
|
||
| load_dotenv() |
There was a problem hiding this comment.
We don't need to invoke load_dotenv() since it was already executed in the create_app() method on startup. When that happens all the values in the .env file are loaded up and we can just import os in this file and grab the values without needing to call load_dotenv() again
2 participants
No description provided.