Conversation
…e), add create_model helper; all tests passing
…s passed on wave 6
Did some changes in the routes and models to pass the tests, all test…
…all tests passed on wave 6" This reverts commit 6613431.
apradoada
left a comment
apradoada
left a comment
There was a problem hiding this comment.
GREEN
Great job, Helen!
I almost feel bad for how little I was able to find with this particular project, but overall it looks really good and you should be so proud of yourself!
Your models look really good, as do the tests, endpoints and configuration!
To me, this project shows a great grasp of the concepts covered in Unit 2!
The one major thing I found was the inclusion of the common.py file! Nothing within the file was used elsewhere which makes it pretty redundant! Other than that, your Task list is pretty near perfect!
Well done!
| from typing import TYPE_CHECKING | ||
|
|
||
| if TYPE_CHECKING: | ||
| from .task import Task |
There was a problem hiding this comment.
While TYPE_CHECKING isn't necessarily required here because we have a one-to-many relationship, it is best practice and is necessary to avoid circular imports in a many-to-many relationship, so great job using it here.
Also, as stated in our Learn lessons, it does allow us to get rid of the yellow underline on line 11!
| def to_dict(self): | ||
| goal_dict = { | ||
| "id": self.id, | ||
| "title": self.title, | ||
| } | ||
| return goal_dict | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, data): | ||
|
|
||
| new_goal = cls(title=data["title"]) | ||
| return new_goal No newline at end of file |
There was a problem hiding this comment.
Your to_dict() and your from_dict() look great, Helen!
| id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) | ||
| title: Mapped[str] | ||
| description: Mapped[str] | ||
| completed_at: Mapped[Optional[datetime]] | ||
| goal_id: Mapped[Optional[int]] = mapped_column(ForeignKey("goal.id")) | ||
| goal: Mapped[Optional["Goal"]] = relationship(back_populates="tasks") |
There was a problem hiding this comment.
Just for clarity's sake, it is often a good idea to move any relationships/columns associated with the relationships to the bottom of the attributes list here like you have done! One small common pattern that you will often see is something like the following:
| id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) | |
| title: Mapped[str] | |
| description: Mapped[str] | |
| completed_at: Mapped[Optional[datetime]] | |
| goal_id: Mapped[Optional[int]] = mapped_column(ForeignKey("goal.id")) | |
| goal: Mapped[Optional["Goal"]] = relationship(back_populates="tasks") | |
| id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) | |
| title: Mapped[str] | |
| description: Mapped[str] | |
| completed_at: Mapped[Optional[datetime]] | |
| goal_id: Mapped[Optional[int]] = mapped_column(ForeignKey("goal.id")) | |
| goal: Mapped[Optional["Goal"]] = relationship(back_populates="tasks") |
| "id": self.id, | ||
| "title": self.title, | ||
| "description": self.description, | ||
| "is_complete": self.completed_at is not None |
| if self.goal: | ||
| task_dict["goal_id"] = self.goal.id |
There was a problem hiding this comment.
Nice job adding the goal id to the dictionary if it exists!
| @bp.get("<goal_id>") | ||
| def get_one_goal(goal_id): | ||
|
|
||
| goal = validate_model(Goal, goal_id) | ||
| return goal.to_dict() | ||
|
|
||
|
|
||
| @bp.put("<goal_id>") | ||
| def update_goal(goal_id): | ||
| goal = validate_model(Goal, goal_id) | ||
| request_body = request.get_json() | ||
|
|
||
| goal.title = request_body["title"] | ||
| db.session.commit() | ||
|
|
||
| return Response(status=204, mimetype="application/json") | ||
|
|
||
|
|
||
| @bp.delete("<goal_id>") | ||
| def delete_model(goal_id): | ||
| goal = validate_model(Goal, goal_id) | ||
|
|
||
| db.session.delete(goal) | ||
| db.session.commit() | ||
|
|
||
| return Response(status=204, mimetype="application/jason") No newline at end of file |
There was a problem hiding this comment.
The rest of these view functions look really good! Great job!!
| if filters: | ||
| for attribute, value in filters.items(): | ||
| if hasattr(cls, attribute): | ||
| query = query.where(getattr(cls, attribute).ilike(f"%{value}%")) | ||
|
|
||
| if attribute == "sort": | ||
| if value == "desc": | ||
| query = query.order_by(cls.title.desc()) | ||
| elif value == "asc": | ||
| query = query.order_by(cls.title.asc()) |
There was a problem hiding this comment.
You are the first person who I have seen successfully incorporate both filtering and sorting by ascending or descending within the same function! Well done!
There was a problem hiding this comment.
This set of helper functions looks practically perfect! Well done, Helen!
| url = "https://slack.com/api/chat.postMessage" | ||
| data = { | ||
| "text": f"Someone just completed the task {task.title}", | ||
| "channel": "test-slack-api" | ||
| } | ||
| headers = { | ||
| 'Content-Type': 'application/json', | ||
| 'Authorization': f"Bearer {os.environ.get('SLACK_BOT_TOKEN')}" | ||
| } | ||
| requests.post(url, headers=headers, json=data) |
There was a problem hiding this comment.
This approach to making a Slack API request looks really good, but I do think it could be removed to its own helper function elsewhere!
| from .routes.task_routes import bp as task_bp | ||
| from .routes.goal_routes import bp as goal_bp |
2 participants
No description provided.