Conversation
| class Goal(db.Model): | ||
| id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) | ||
| title: Mapped[str] | ||
| tasks: Mapped[list["Task"]] = relationship(back_populates="goal") |
| return { | ||
| "id": self.id, | ||
| "title" : self.title | ||
| #"tasks" : [task.to_dict() for task in self.tasks] |
There was a problem hiding this comment.
It would be nice to conditionally return tasks if a goal has them.
You could create a goal dictionary and then add the tasks if necessary.
goal = {
"id": self.id,
"title" : self.title
}
if self.tasks:
goal["tasks"] = [task.to_dict() for task in self.tasks]
return goal| def from_dict(cls, goal_data): | ||
| return cls( | ||
| title = goal_data["title"] | ||
| #tasks = [Task.from_dict(task_data) for task_data in goal_data["tasks"]] |
There was a problem hiding this comment.
Same as my comment above. How could you conditionally pass tasks if you needed to?
| @@ -1 +1,70 @@ | |||
| from flask import Blueprint No newline at end of file | |||
| from flask import Blueprint, request, Response, abort, make_response | |||
There was a problem hiding this comment.
| from flask import Blueprint, request, Response, abort, make_response | |
| from flask import Blueprint, request, Response |
abort and make_response are imported but not accessed so they should be removed.
| title: Mapped[str] | ||
| description: Mapped[str] | ||
| completed_at: Mapped[datetime | None] = mapped_column(nullable=True) | ||
| goal_id: Mapped[Optional[int]] = mapped_column(ForeignKey("goal.id")) |
There was a problem hiding this comment.
👍 The foreign key goes on the model that represents the 'many' side of the one-to-many relationship. Since a goal has many tasks, the goal's id should be the foreign key for a task.
| task.completed_at = datetime.now() | ||
| task.title="My Beautiful Task" | ||
| db.session.commit() | ||
| slack_url = "https://slack.com/api/chat.postMessage" |
There was a problem hiding this comment.
Constant variables should be named with all caps
| slack_url = "https://slack.com/api/chat.postMessage" | |
| SLACK_URL = "https://slack.com/api/chat.postMessage" |
| slack_request_body = { | ||
| "channel": "#task-notifications", |
There was a problem hiding this comment.
Prefer the channel name to be referenced by a constant variable too.
| slack_request_body = { | |
| "channel": "#task-notifications", | |
| SLACK_CHANNEL = "#task-notifications" | |
| slack_request_body = { | |
| "channel": SLACK_CHANNEL, |
| def mark_task_complete(task_id): | ||
| task = validate_model(Task, task_id) | ||
| task.completed_at = datetime.now() | ||
| task.title="My Beautiful Task" |
There was a problem hiding this comment.
The task title shouldn't be hardcoded to "My Beautiful Task" like we showed in an example. The task.title should just be whatever the task's current title.
| task.title="My Beautiful Task" |
There was a problem hiding this comment.
👍 Your added assertions for this test file look good to me.
|
|
||
| # Assert | ||
| assert response.status_code == 400 | ||
| assert "details" in response_body |
There was a problem hiding this comment.
| assert "details" in response_body |
We can remove this assertion because you check the shape of response_body below on line 168-170, which includes the key "details".
2 participants
No description provided.