Conversation
CheezItMan
left a comment
CheezItMan
left a comment
There was a problem hiding this comment.
Really nice work Glenda, you hit all the learning goals here. Well done. I made a few minor suggestions, but this is great work.
One minor additional suggestion, it might help readability to break up the long route functions you define into smaller functions which simply handle one task. You can make a route function which handles a specific path/method combination rather than one function which handles all the methods for a specific path.
| "title": self.title | ||
| } | ||
|
|
||
| def to_json_three(self): |
| def to_json(self): | ||
|
|
||
| return { | ||
| "id": self.goal_id, | ||
| "title": self.title | ||
| } |
| goal_id = db.Column(db.Integer, db.ForeignKey('goal.goal_id'), nullable=True) | ||
|
|
||
| # created a helper function to convert completed_at to is_complete. | ||
| def to_json(self): |
| import os | ||
| from app.models.goal import Goal | ||
|
|
||
| path = "https://slack.com/api/chat.postMessage" |
There was a problem hiding this comment.
Suggestion: Make variables you want to stay stuck to a specific value, in all capital letters. We normally make constants in all caps. Also change the name to specify which path you want.
| path = "https://slack.com/api/chat.postMessage" | |
| SLACK_PATH = "https://slack.com/api/chat.postMessage" |
| key = os.environ.get("AUTHORIZATION") | ||
|
|
||
| query_params = { | ||
| "text": f"Someone just completed the task {task.title}", | ||
| "channel": "task-notifications" | ||
|
|
||
| } | ||
|
|
||
| query_headers = { | ||
| "authorization": f"Bearer {key}" | ||
| } | ||
| response = requests.post(path, params=query_params, headers=query_headers ) | ||
| response_body = response.json() |
There was a problem hiding this comment.
Just a note: This would make a great helper function.
|
|
||
| print(response) | ||
|
|
||
|
|
| print(response) | ||
|
|
||
|
|
||
| db.session.commit() |
There was a problem hiding this comment.
I would also send the slack message, after you have successfully saved the task to the database.
| return jsonify({"task": task.to_json()}),200 | ||
|
|
||
|
|
||
| goals_bp = Blueprint("goals", __name__, url_prefix="/goals") |
There was a problem hiding this comment.
I suggest you define the Blueprints together at the top, just to make it easier to review all the blueprints available.
| @goals_bp.route("/<goal_id>/tasks", methods=["GET"], strict_slashes=False) | ||
| def get_goals_task(goal_id): |
There was a problem hiding this comment.
Small suggestion, add a blank line between route definitions
| @goals_bp.route("/<goal_id>/tasks", methods=["GET"], strict_slashes=False) | |
| def get_goals_task(goal_id): | |
| @goals_bp.route("/<goal_id>/tasks", methods=["GET"], strict_slashes=False) | |
| def get_goals_task(goal_id): |
| @tasks_bp.route("", methods=["POST", "GET"], strict_slashes=False) | ||
| def tasks(): |
There was a problem hiding this comment.
Small readability suggestion, it could be good to break up these large functions into smaller functions. So one function to handle the GET request and the other handles the POST request.
2 participants
No description provided.