Rock Saharai Cante #66
Conversation
There was a problem hiding this comment.
Fantastic job Saharai! You met all the learning goals and did so many cool enhancements. I had a lot of fun reading your code and seeing you experiment with lambda functions, helper methods/functions, and sorting! I challenge you to continue using these in future projects.
I've added a few comments about refactoring. Feel free to let me know if you ever revisit this project and would like another review in the future 👀 .
Overall, great job on this project. Keep up the hard work!
| from .routes import task_bp | ||
| from .routes import goal_bp | ||
| app.register_blueprint(task_bp) | ||
| app.register_blueprint(goal_bp) | ||
|
|
There was a problem hiding this comment.
You can combine imports like so:
| from .routes import task_bp | |
| from .routes import goal_bp | |
| app.register_blueprint(task_bp) | |
| app.register_blueprint(goal_bp) | |
| from .routes import task_bp, goal_bp | |
| app.register_blueprint(task_bp) | |
| app.register_blueprint(goal_bp) | |
| from app.models.task import Task | ||
| from sqlalchemy import Table, Column, Integer, ForeignKey | ||
| from sqlalchemy.orm import relationship | ||
| from sqlalchemy.ext.declarative import declarative_base |
There was a problem hiding this comment.
hm... Table, Column, etc. should all be coming from your db instance of sqlalchemy from __init__.py. So I don't think you need the sqlalchemy imports in this file as it's a little redundant.
| if self.goal_id == None: | ||
| task_dict={ | ||
| "id": self.task_id, | ||
| "title": self.title, | ||
| "description": self.description, | ||
| "is_complete": self.is_complete() | ||
| } | ||
| else: | ||
| task_dict={ | ||
| "id": self.task_id, | ||
| "goal_id": self.goal_id, | ||
| "title": self.title, | ||
| "description": self.description, | ||
| "is_complete": self.is_complete() | ||
| } | ||
| return task_dict |
There was a problem hiding this comment.
Nice! Great job Saharai. You could refactor this into a single dictionary like so:
| if self.goal_id == None: | |
| task_dict={ | |
| "id": self.task_id, | |
| "title": self.title, | |
| "description": self.description, | |
| "is_complete": self.is_complete() | |
| } | |
| else: | |
| task_dict={ | |
| "id": self.task_id, | |
| "goal_id": self.goal_id, | |
| "title": self.title, | |
| "description": self.description, | |
| "is_complete": self.is_complete() | |
| } | |
| return task_dict | |
| task_dict={ | |
| "id": self.task_id, | |
| "title": self.title, | |
| "description": self.description, | |
| "is_complete": self.is_complete() | |
| } | |
| if self.goal_id: | |
| task_dict['goal_id'] = self.goal_id | |
| return task_dict |
| def get_tasks(): | ||
|
|
||
| title_query = request.args.get("title") | ||
| if title_query: | ||
| tasks = Task.query.filter_by(title=title_query) | ||
|
|
||
| else: | ||
| tasks = Task.query.all() | ||
|
|
||
| tasks_response = [] | ||
| for task in tasks: | ||
| tasks_response.append(task.to_json()) | ||
|
|
||
| if "asc" in request.full_path: | ||
| sort_tasks=sorted(tasks_response, key = lambda i: i['title']) | ||
| return jsonify(sort_tasks) | ||
|
|
||
| elif "desc" in request.full_path: | ||
| sort_tasks=sorted(tasks_response, key = lambda i: i['title'], reverse=True) | ||
| return jsonify(sort_tasks) | ||
|
|
||
| else: | ||
| return jsonify(tasks_response) |
| if all(keys in request_body for keys in ("title","description","completed_at"))== False: | ||
| return { | ||
| "details": "Invalid data" | ||
| },400 |
There was a problem hiding this comment.
Clever way of checking for values in the request body !
| task = Task.query.get(task_id) | ||
|
|
||
| if task is None: | ||
| return make_response ("",404) | ||
| else: | ||
| return {"task":task.to_json()},200 |
There was a problem hiding this comment.
You can refine this function into a fewer lines by using Flask's own get_or_404() method
| task = Task.query.get(task_id) | |
| if task is None: | |
| return make_response ("",404) | |
| else: | |
| return {"task":task.to_json()},200 | |
| task = Task.query.get_or_404(task_id) | |
| return {"task":task.to_json()},200 |
| def mark_incomplete(task_id): | ||
| task = Task.query.get(task_id) | ||
| if task is None: | ||
| return make_response ("",404) | ||
|
|
||
| task.completed_at=None | ||
| db.session.commit() | ||
|
|
||
| return { | ||
| "task": task.to_json() | ||
| }, 200 |
There was a problem hiding this comment.
Nice this function is really clean!
| url = f"https://slack.com/api/chat.postMessage?channel=task-notifications&text=Someone just completed the task {task.title}" | ||
|
|
||
|
|
||
|
|
||
| token=os.environ.get("bot_user_token") | ||
|
|
||
|
|
||
| headers_dict={"Authorization":token} | ||
| response = requests.request("POST", url, headers=headers_dict) | ||
| return { | ||
| "task": { | ||
| "id": task.task_id, | ||
| "title": task.title, | ||
| "description":task.description, | ||
| "is_complete": task.is_complete() | ||
| } | ||
| }, 200 |
There was a problem hiding this comment.
Great job in getting your slack bot to work. A future refactor can include putting this logic into a helper function.
| sort_goals=sorted(goals_response, key = lambda i: i['title']) | ||
|
|
||
| return jsonify(sort_goals) | ||
|
|
||
| elif "desc" in request.full_path: | ||
| sort_goals=sorted(goals_response, key = lambda i: i['title'], reverse=True) |
There was a problem hiding this comment.
LOVING the use of sorted() and the lambda function 🔥
| goal=Goal.query.get(goal_id) | ||
| task = Task.query.get(goal_id) | ||
|
|
||
|
|
||
| if not goal: | ||
| return make_response("",404) | ||
| else: | ||
|
|
||
| outer_dict=goal.goal_to_json() | ||
| if task==None: | ||
| outer_dict['tasks']=[] | ||
| else: | ||
| inner_dict=task.to_json() | ||
|
|
||
| inner_dict["goal_id"]=goal.goal_id | ||
| outer_dict['tasks']=[inner_dict] | ||
| db.session.commit() | ||
| return outer_dict,200 |
There was a problem hiding this comment.
Great job! This was a very clean way of getting the tasks for a specific goal_id
No description provided.