-
Notifications
You must be signed in to change notification settings - Fork 127
Amethyst-PhalesaPatton- #122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
"goal_id":self.goal_id, | ||
"title": self.title, | ||
"description": self.description, | ||
"is_complete": bool(self.completed_at) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love how you used the boolean constructor here!
new_task = Task.from_dict(request_body) | ||
|
||
except: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love how you implemented a try/except
block, here! 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 True
We can pass in the request_body
and a list of strings that are keys
and then check to see if those keys are present.
if sort_filter == "asc": | ||
tasks = Task.query.order_by(Task.title.asc()) | ||
elif sort_filter == "desc": | ||
tasks = Task.query.order_by(Task.title.desc()) | ||
else: | ||
tasks = Task.query.all() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this implementation! You did well with making the database do the heavy lifting when it came to sorting the tasks!
def read_one_task(task_id): | ||
task = validate_model(Task, task_id) | ||
if task.goal_id: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⭐️
task.description = request_body["description"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
token = os.environ.get("slack_token") | ||
slack_url = "https://slack.com/api/chat.postMessage" | ||
headers = {"Authorization":token} | ||
body = { | ||
"channel": "task-notifications", | ||
"text": f"Someone just completed the task {task.title}", | ||
} | ||
|
||
requests.post(slack_url, headers=headers, json=body) | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love how you made this into a helper function! Make sure you are putting them a place that is easy to find, I typically put mine a separate folder or at the top of the file.
db.session.commit() | ||
|
||
slack_notification(task) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!
goals_response.append(goal.to_dict_goals()) | ||
|
||
return jsonify(goals_response) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💫
return jsonify(goals_tasks), 200 | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done on this project Phalesa, 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! ✨💫🤭
No description provided.