Conversation
marciaga
left a comment
marciaga
left a comment
There was a problem hiding this comment.
Good work! Nice job with the tests as well. I left a few comments for you, one perhaps most importantly, about the Slack API key.
app/__init__.py
Outdated
| app.config['SQLALCHEMY_DATABASE_URI'] = os.environ.get( | ||
| "RENDER_DATABASE_URI") | ||
|
|
||
| # if test_config is None: |
There was a problem hiding this comment.
I'd recommend using this logic to switch between testing and non-testing environments. This is why your tests are erroring out in Learn.
| goal_id = db.Column(db.Integer, db.ForeignKey("goal.id")) | ||
|
|
||
| def to_dict_with_goal(self): | ||
| if not self.completed_at: |
| is_complete=self.is_complete, | ||
| goal_id=self.goal_id | ||
| )) | ||
| def to_dict(self): |
There was a problem hiding this comment.
Rather than have two functions that do nearly the same thing, why not adopt the same approach of using conditional logic to set the goal_id?
| @bp.route("", methods=["POST"]) | ||
| def create_a_goal(): | ||
| request_body = request.get_json() | ||
| if "title" not in request_body or not request_body["title"]: |
| def get_all_tasks(): | ||
| sorted_query = request.args.get("sort") | ||
| if sorted_query == "asc": | ||
| tasks = Task.query.order_by("title") |
There was a problem hiding this comment.
Assuming this order_by does ascending by default, without you having to explicitly call order_by(Task.title.asc())? Also, can the .all() be left off of the order_by queries (i.e. does it return what all() returns by default?
app/routes/task_routes.py
Outdated
| task.is_complete = True | ||
|
|
||
| PATH = "https://slack.com/api/chat.postMessage" | ||
| Authorization = "Bearer xoxb-5242678399683-5266537240624-SzcfKHmF2xZaq407bmmGcsdL" |
There was a problem hiding this comment.
This API key should be in a .env file (which should be in the project's .gitignore and therefore not committed). It's potentially dangerous to commit API keys, in one scenario, a malicious actor could use your API key to make calls as though it were coming from your application, which if a credit card is attached to the API account, could rack up fraudulent charges or exploit any number of other potential vulnerabilities exposed by the key. I'd recommend revoking/deleting this key.
2 participants
No description provided.