Conversation
marciaga
left a comment
marciaga
left a comment
There was a problem hiding this comment.
Great work! See individual comments for more details. Overall, the structure and presentation of the routes and models is good as well as the added tests.
| task_as_dict["goal_id"] = self.goal_id | ||
| task_as_dict["title"] = self.title | ||
| task_as_dict["description"] = self.description | ||
| task_as_dict["is_complete"] = bool(self.completed_at) |
|
|
||
| bp = Blueprint("tasks", __name__, url_prefix="/tasks") | ||
|
|
||
| def validate_model(cls, model_id): |
There was a problem hiding this comment.
Since validate_model is duplicated in task routes as well as goal routes, we could extract it into a separate route_helpers file and import it as needed!
| "id":new_task.task_id, | ||
| "title": new_task.title, | ||
| "description": new_task.description, | ||
| "is_complete": bool(new_task.completed_at) |
There was a problem hiding this comment.
Even though a new task is logically not going to be completed in the same request in which it's being created, I like how you're handling it here instead of hard-coding is_complete = False.
| SLACK_API_TOKEN = os.environ.get("SLACK_API_TOKEN") | ||
|
|
||
| data= {"channel":"task-notifications", "text":f"Someone just completed the task {task.title}"} | ||
| headers = {"Authorization": SLACK_API_TOKEN} |
There was a problem hiding this comment.
Looking at this value obtained from the environment, I suspect your file would contain something like SLACK_API_TOKEN=Bearer xoxb-... however I'd recommend using an f-string to add the "Bearer " text so that in case the API token needs to be included elsewhere without the "Bearer " prepended to it, it will be reusable.
|
|
||
| task.completed_at = datetime.datetime.utcnow() | ||
|
|
||
| slack_bot(task) |
There was a problem hiding this comment.
We might want to wrap this in a try/except since if the call to Slack fails, we will presumably still want to return a success message to the client.
2 participants
No description provided.