Conversation
kelsey-steven-ada
left a comment
kelsey-steven-ada
left a comment
There was a problem hiding this comment.
Congrats on your first solo project API! I've left some comments and questions across the PR, please take a look when you can and reply here or on Slack if there's anything I can clarify =]
| from .routes.task_routes import tasks_bp | ||
| app.register_blueprint(tasks_bp) | ||
| from .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!
| try: | ||
| model_id = int(model_id) | ||
| except: | ||
| abort(make_response(jsonify({"message":f"{cls.__name__} {model_id} invalid"}), 400)) |
There was a problem hiding this comment.
To keep code shorter and easier to read, I'd suggest splitting this line up.
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.
| "id": self.goal_id, | ||
| "title": self.title, | ||
| } | ||
| if tasks: |
There was a problem hiding this comment.
Nice use of an extra parameter to make the function more flexible!
| "id": self.task_id, | ||
| "title": self.title, | ||
| "description": self.description, | ||
| "is_complete": bool(self.completed_at) |
There was a problem hiding this comment.
Great choice to derive the value of is_complete from self.completed_at!
| if "completed_at" not in task_data: | ||
| task_data["completed_at"] = None |
There was a problem hiding this comment.
Nice conditional to ensure you avoid a KeyError if the user didn't send a completed_at value ^_^
| @goals_bp.route("/<goal_id>", methods=["GET"]) | ||
| def get_one_goal(goal_id): | ||
| goal = validate_model(Goal, goal_id) | ||
| response = {"goal": goal.to_dict()} |
There was a problem hiding this comment.
Great way to add what you need for the response while getting to reuse the to_dict function!
| goal = validate_model(Goal, goal_id) | ||
| updated_data = request.get_json() | ||
|
|
||
| goal.title = updated_data["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?
| sort_direction = request.args.get("sort") | ||
|
|
||
| if sort_direction == "asc": | ||
| tasks = Task.query.order_by(Task.title) | ||
| elif sort_direction == "desc": | ||
| tasks = Task.query.order_by(Task.title.desc()) | ||
| else: | ||
| tasks = Task.query.all() | ||
|
|
||
| results = [task.to_dict() for task in tasks] |
There was a problem hiding this comment.
We could simplify the if/else tree a little if we get a reference to the Task.query object first:
task_query = Task.query
sort_direction = request.args.get("sort")
if sort_direction == "asc":
task_query = task_query.order_by(Task.title.asc())
elif sort_direction == "desc":
task_query = task_query.order_by(Task.title.desc())
results = [task.to_dict() for task in task_query.all()]| db.session.commit() | ||
|
|
||
| url = "https://slack.com/api/chat.postMessage" | ||
| SLACK_API_TOKEN = os.environ.get("SLACK_API_TOKEN") |
There was a problem hiding this comment.
SLACK_API_TOKEN is a local variable of the complete_one_task function, so we'd typically see their name lowercased here. We use all caps for constant values, typically when declared where they're accessible across a file or project.
2 participants
No description provided.