Maple-FaithKauwe #81
Maple-FaithKauwe #81
Conversation
|
Great job Faith! I added some comments on refactoring your code. Think about using helper functions or moving code to your models to clean up the |
Co-authored-by: Trenisha Goslee <[email protected]>
| description = db.Column(db.String) | ||
| completed_at = db.Column(db.DateTime, nullable = True) | ||
| goal_id = db.Column(db.Integer, db.ForeignKey("goal.goal_id"), nullable = True) | ||
| #goal =db.relationship("Goal", back_populates ="task") |
There was a problem hiding this comment.
you can remove this now that you are not using it
| return make_response({"task": {"id": new_task.task_id, | ||
| "title": new_task.title, | ||
| "description": new_task.description, | ||
| "is_complete": is_complete}}, 201) |
There was a problem hiding this comment.
This is code is repeated multiple times in this file ... think about creating a helper function to put this information. You could also move this to your task model and create an instance method you can use in routes.py
| if title_query: | ||
| # filter_by returns a list of objects/ records that match the query params | ||
|
|
||
| tasks = Task.query.filter_by(title = title_query) | ||
| # what part of the Task.query is actually accessing the DB? | ||
| elif order_by_query == 'asc': | ||
| # getting all tasks fr the db | ||
| tasks = Task.query.order_by(Task.title).all() | ||
| # query_all return list of objects. loop through objects and add to empt list, task_response | ||
| # as requested formatted JSON objects | ||
| elif order_by_query == 'desc': | ||
| tasks = Task.query.order_by(desc(Task.title)).all() | ||
| # this else covers any search for all tasks, without any query params | ||
| else: | ||
| tasks = Task.query.order_by(Task.title).all() |
There was a problem hiding this comment.
💃🏽 great way to combine filtering by title and sorting
| task = Task.query.get(task_id) #either get Task back or None | ||
| if task is None: | ||
| # couldn't figure out another way to return no response body, researched abort | ||
| abort(404) |
There was a problem hiding this comment.
you could do something like return make_response(jsonify(None), 404)
| try: | ||
| query_params = {"channel": "slack-api-test-channel", | ||
| "text": f"Someone just completed the task {task.title}"} | ||
| header_param = {"Authorization": "Bearer "+ os.environ.get("slack_oauth_token")} | ||
| slack_post_body = requests.post(slack_path, params=query_params, headers= header_param) | ||
| except TypeError: | ||
| pass |
| if task is None: | ||
| abort(404) |
There was a problem hiding this comment.
another suggestion instead of abort could be
| if task is None: | |
| abort(404) | |
| if not task: | |
| return "", 404 |
| return make_response({"goal": {"id": new_goal.goal_id, | ||
| "title": new_goal.title}}, 201) |
There was a problem hiding this comment.
same comment from tasks. You can make a helper function to handle the dict for goals or you could move it to the goal model and create an instance method.
No description provided.