Muniba K Zoisite C19 #128
Muniba K Zoisite C19 #128
Conversation
| from app.routes.task_routes import tasks_bp | ||
| app.register_blueprint(tasks_bp) | ||
|
|
||
| from app.routes.goal_routes import goals_bp | ||
| app.register_blueprint(goals_bp) |
There was a problem hiding this comment.
Nice choice to split up the routes into files that are more specific to the resources they work with!
| "text": f"Someone just completed the task {self.title}" | ||
| } | ||
| headers = { | ||
| 'Authorization': f'{API_KEY}' |
There was a problem hiding this comment.
The indentation is a little funky here, the common practice is to indent the contents of a dictionary literal more than the brackets.
headers = {
'Authorization': f'{API_KEY}'
}There was a problem hiding this comment.
I recommend placing this file in the routes directory to keep it close to the code that uses it.
| model = cls.query.get(model_id) | ||
|
|
||
| if not model: | ||
| abort(make_response({"message":f"{cls.__name__} {model_id} not found"}, 404)) |
There was a problem hiding this comment.
A lot happens on this line, to make the code easier to read at a glance I'd suggest splitting this line up.
|
|
||
| return model | ||
|
|
||
| def slack_mark_complete(self): |
There was a problem hiding this comment.
Great use of a helper function for the slack message!
| self.completed_at = None | ||
|
|
||
| def slack_mark_complete(self): | ||
| API_KEY = os.environ.get("API_KEY") |
There was a problem hiding this comment.
API_KEY is a local variable of the slack_mark_complete function, so we'd typically see the name lowercased here. We use all caps for constant values, typically when declared where they're accessible across a file or project.
| if self.completed_at: | ||
| is_complete = True | ||
| else: | ||
| is_complete = False |
There was a problem hiding this comment.
Great choice to derive the value of is_complete from self.completed_at! We could shorten this a little with the python bool function:
is_complete = bool(self.completed_at)| title_query = request.args.get("title") | ||
| if title_query: | ||
| goals = Goal.query.filter_by(title=title_query) | ||
|
|
||
| if request.args.get("sort") == "asc": | ||
| goals = Goal.query.order_by(Goal.title.asc()).all() | ||
|
|
||
| elif request.args.get("sort") == "desc": | ||
| goals = Goal.query.order_by(Goal.title.desc()).all() | ||
|
|
||
| else: | ||
| goals = Goal.query.all() | ||
|
|
||
| goals_response = [] | ||
| for goal in goals: | ||
| goals_response.append(goal.to_dict()) | ||
| return jsonify(goals_response), 200 |
There was a problem hiding this comment.
We can either sort or filter with this implementation. If we want to do both, we can get a reference to the class's query object that we keep updating:
model_query = Goal.query
title_query = request.args.get("title")
if title_query:
model_query = model_query.filter(Goal.title == request.args.get("title"))
if request.args.get("sort") == "asc":
model_query = model_query.order_by(Goal.title.asc())
elif request.args.get("sort") == "desc":
model_query = model_query.order_by(Goal.title.desc())
goals = model_query.all()
goals_response = [goal.to_dict() for goal in goals]|
|
||
| request_body = request.get_json() | ||
|
|
||
| goal.title = request_body["title"] |
There was a problem hiding this comment.
There's some error handling in create_goal that we could use here to alert the user if the request was missing a title key. How could we share the behavior with both functions?
| for task_id in request_body["task_ids"]: | ||
| task = validate_model(Task, task_id) | ||
| task.goal_id = goal_id | ||
| db.session.add(task) |
There was a problem hiding this comment.
task already exists in the database so we could remove this line, the commit below will save our changes.
No description provided.