Amethyst - Megan G #112
Amethyst - Megan G #112
Conversation
| } | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, goal_data): |
| goal = db.relationship("Goal", back_populates ="tasks") | ||
|
|
||
| def to_dict(self): | ||
| if self.goal_id: |
There was a problem hiding this comment.
You always are so good at implementing guard clauses!
| def create_task(): | ||
| request_body = request.get_json() | ||
|
|
||
| if not request_body.get("title") or not request_body.get("description"): |
There was a problem hiding this comment.
Like we validated models, could we also make a general function that validates request bodies? Here's an example:
def validate_request_body(request_body, keys):
for key in keys:
if not request_body.get(key):
abort(make_response({
'Invalid Data': f'missing key: {key}'
}, 400))
return TrueWe can pass in the request_body and a list of strings that are keys and then check to see if those keys are present.
| title_query = request.args.get("sort") | ||
| if title_query == "asc": | ||
| tasks = Task.query.order_by(Task.title).all() | ||
| elif title_query == "desc": | ||
| tasks = Task.query.order_by(Task.title.desc()).all() | ||
| else: | ||
| tasks = Task.query.all() |
There was a problem hiding this comment.
Nice work on having the database sort the tasks for you!
| task.title = request_body["title"] | ||
| task.description = request_body["description"] |
There was a problem hiding this comment.
Right now, if a user sends a request without the keys title or description your server would crash. There's a couple of ways to handle this, you could call the validate_request function before you access the keys in request_body or you could implement a try/except block.
| "Authorization": f"Bearer {os.environ.get('SLACK_KEY')}" | ||
| } | ||
|
|
||
| slack_request = requests.post(slack_url, headers=headers, json=slack_data) |
There was a problem hiding this comment.
Nice work on having your Slack post sent after the logic of marking a task complete! We don't want to send out any false positive alerts just in case our logic fails during the update!
| def validate_model(cls, model_id): | ||
| try: | ||
| model_id = int(model_id) | ||
| except: | ||
| abort(make_response({"details":f"Invalid data"}, 400)) | ||
|
|
||
| model = cls.query.get(model_id) | ||
|
|
||
| if not model: | ||
| abort(make_response({"details":f"{cls.__name__} ID not found"}, 404)) | ||
|
|
||
| return model |
There was a problem hiding this comment.
Make sure that your helper functions are in a place that is easy to find, I typically put them in the top of the file or in another folder somewhere else.
There was a problem hiding this comment.
Well done on this project, I didn't have much to comment on and that is a good thing! Keep up the good work! Really looking forward to what you create in the frontend! Please feel free to reach out if you have any questions about the feedback that I left! ✨💫🤭
| for task_id in request_body["task_ids"]: | ||
| task = validate_model(Task, task_id) | ||
| task.goal_id = goal.goal_id |
There was a problem hiding this comment.
Very nice for loop, Megan! You ate this! 🤭
| tasks_response = [] | ||
| for task in goal.tasks: | ||
| tasks_response.append(task.to_dict()) |
There was a problem hiding this comment.
How do you think your code would change if you made this logic into a class function?
|
|
||
| request_body = request.get_json() | ||
|
|
||
| goal.title = request_body["title"] |
There was a problem hiding this comment.
Refer to my comment about error handling, on line 62 in task_routes.py
No description provided.