-
Notifications
You must be signed in to change notification settings - Fork 718
Crow_lexy Le task_list_api #89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
bd34383
61c3168
98afc1c
eb9c199
c6e7a13
769ad1a
e7c70f0
d340937
f49cafa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| from flask import Blueprint, jsonify, request, abort, make_response | ||
| from flask import Blueprint, request, abort, make_response,Response | ||
| from ..db import db | ||
| from ..models.goal import Goal | ||
| from ..models.task import Task | ||
| from .route_utilities import validate_model | ||
| from .route_utilities import validate_model,create_model,get_models_with_filters | ||
|
|
||
|
|
||
| bp = Blueprint("goals_bp", __name__, url_prefix="/goals") | ||
|
|
@@ -12,27 +12,27 @@ | |
| @bp.post("") | ||
| def create_goal(): | ||
| request_body = request.get_json() | ||
| try: | ||
| new_goal = Goal.from_dict(request_body) | ||
| except KeyError: | ||
| return jsonify({"details": "Invalid data"}), 400 | ||
| # try: | ||
| # new_goal = Goal.from_dict(request_body) | ||
| # except KeyError: | ||
| # return jsonify({"details": "Invalid data"}), 400 | ||
|
|
||
| db.session.add(new_goal) | ||
| db.session.commit() | ||
| # db.session.add(new_goal) | ||
| # db.session.commit() | ||
|
|
||
| return jsonify(new_goal.to_dict()), 201 | ||
| #return jsonify(new_goal.to_dict()), 201 | ||
| return create_model(Goal,request_body) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice update to use
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just updated |
||
|
|
||
|
|
||
| @bp.get("") | ||
| def get_all_goals(): | ||
| goals = Goal.query.order_by(Goal.id).all() | ||
| return jsonify([goal.to_dict() for goal in goals]), 200 | ||
|
|
||
| filters = request.args.to_dict() | ||
| return get_models_with_filters(Goal,filters) | ||
|
|
||
| @bp.get("/<id>") | ||
| def get_one_goal(id): | ||
| goal = validate_model(Goal, id) | ||
| return jsonify(goal.to_dict()), 200 | ||
| return goal.to_dict() | ||
|
|
||
|
|
||
|
|
||
|
|
@@ -45,7 +45,7 @@ def update_goal(id): | |
| goal.title = request_body["title"] | ||
|
|
||
| db.session.commit() | ||
| return jsonify(goal.to_dict()), 200 | ||
| return goal.to_dict() | ||
|
|
||
|
|
||
|
|
||
|
|
@@ -56,8 +56,8 @@ def delete_goal(id): | |
| db.session.delete(goal) | ||
| db.session.commit() | ||
|
|
||
| return jsonify({"message": f'Goal {goal.id} successfully deleted'}), 204 | ||
|
|
||
| #return jsonify({"message": f'Goal {goal.id} successfully deleted'}), 204 | ||
| return Response(status=204, mimetype="application/json") | ||
|
|
||
| #nested | ||
| @bp.post("/<goal_id>/tasks") | ||
|
|
@@ -70,18 +70,16 @@ def add_tasks_to_goal(goal_id): | |
| for task in goal.tasks: | ||
| task.goal_id = None | ||
|
|
||
|
|
||
|
|
||
| for task_id in task_ids: | ||
| task = validate_model(Task, task_id) | ||
| task.goal_id = goal.id | ||
|
|
||
| db.session.commit() | ||
|
|
||
| return jsonify({ | ||
| return { | ||
| "id": goal.id, | ||
| "task_ids": task_ids | ||
| }), 200 | ||
| }, 200 | ||
|
|
||
|
|
||
|
|
||
|
|
@@ -91,10 +89,13 @@ def get_tasks_for_goal(goal_id): | |
|
|
||
| tasks_response = [task.to_dict() for task in goal.tasks] | ||
|
|
||
| goal_dict = goal.to_dict() | ||
| goal_dict["tasks"] = tasks_response | ||
|
|
||
| return jsonify(goal_dict), 200 | ||
| response_body = { | ||
| "id": goal.id, | ||
| "title": goal.title, | ||
| "tasks": tasks_response | ||
| } | ||
|
|
||
| return response_body,200 | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -22,9 +22,7 @@ def create_task(): | |||||||||||
| db.session.commit() | ||||||||||||
| return new_task.to_dict(), 201 | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| @bp.get("") | ||||||||||||
|
|
||||||||||||
| def get_all_tasks(): | ||||||||||||
| query = db.select(Task) | ||||||||||||
|
|
||||||||||||
|
|
@@ -63,76 +61,56 @@ def get_all_tasks(): | |||||||||||
|
|
||||||||||||
| result_list =[task.to_dict()for task in tasks] | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small spacing issue
Suggested change
|
||||||||||||
|
|
||||||||||||
| return jsonify(result_list or []),200 | ||||||||||||
|
|
||||||||||||
| return result_list or [] | ||||||||||||
|
|
||||||||||||
| @bp.get("/<id>") | ||||||||||||
|
|
||||||||||||
| def get_one_task(id): | ||||||||||||
|
Comment on lines
+56
to
+57
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blank line between decorator and function it is acting on.
Suggested change
|
||||||||||||
|
|
||||||||||||
| task =validate_model(Task,id) | ||||||||||||
| return task.to_dict() | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| @bp.put("/<id>") | ||||||||||||
| def replace_task(id): | ||||||||||||
| task = validate_model(Task, id) | ||||||||||||
|
|
||||||||||||
| request_body = request.get_json() | ||||||||||||
|
|
||||||||||||
| # if request_body["is_complete"]: | ||||||||||||
| # task.completed_at = datetime.utcnow() | ||||||||||||
| # else: | ||||||||||||
| # task.completed_at = None | ||||||||||||
| task.title = request_body["title"] | ||||||||||||
|
|
||||||||||||
| task.description = request_body["description"] | ||||||||||||
|
|
||||||||||||
| db.session.commit() | ||||||||||||
|
|
||||||||||||
| return Response(status = 204,mimetype ="application/json") | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small spacing issues. We don't include spaces around the
Suggested change
|
||||||||||||
|
|
||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The spacing between and inside of functions is inconsistent across the file. I would like to see this revisited if you are already working in this file. This is something that will come up in code reviews in engineering teams, code style is important to readability. This is something we need to work into our review process for ourselves before we open pull requests. |
||||||||||||
| @bp.delete("/<id>") | ||||||||||||
| def del_task(id): | ||||||||||||
| task = validate_model(Task, id) | ||||||||||||
|
|
||||||||||||
| db.session.delete(task) | ||||||||||||
|
|
||||||||||||
| db.session.commit() | ||||||||||||
|
|
||||||||||||
| return Response(status = 204,mimetype ="application/json") | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| # @bp.patch("/<id>/mark_complete") | ||||||||||||
| # def mark_complete(id): | ||||||||||||
| # task = validate_model(Task, id) | ||||||||||||
| # task.completed_at = datetime.utcnow() | ||||||||||||
| # db.session.commit() | ||||||||||||
| # return Response(status=204, mimetype="application/json") | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| @bp.patch("/<id>/mark_incomplete") | ||||||||||||
| def mark_incomplete(id): | ||||||||||||
| task = validate_model(Task, id) | ||||||||||||
|
|
||||||||||||
| task.completed_at = None | ||||||||||||
| db.session.commit() | ||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| @bp.patch("/<id>/mark_complete") | ||||||||||||
| def mark_task_complete(id): | ||||||||||||
| task = validate_model(Task, id) | ||||||||||||
| #task = Task.query.get(id) | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove commented code |
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| task.completed_at = datetime.now(timezone.utc) | ||||||||||||
| db.session.commit() | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| slack_token = os.environ.get("SLACK_BOT_TOKEN") | ||||||||||||
| slack_channel = os.environ.get("SLACK_CHANNEL", "#test_task_slack_api") | ||||||||||||
| message = f"Task *{task.title}* has been completed!" | ||||||||||||
|
|
@@ -144,8 +122,8 @@ def mark_task_complete(id): | |||||||||||
|
|
||||||||||||
| json={"channel": slack_channel, "text": message} | ||||||||||||
| ) | ||||||||||||
| if current_app.config.get("TESTING"): | ||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||
| #if current_app.config.get("TESTING"): | ||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice restructuring so the message is only sent in production mode, but we always return the same response. |
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| return jsonify({"task": task.to_dict()}), 200 | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As part of these updates, please follow best practices to remove the old code once the refactor is working as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure!