-
Notifications
You must be signed in to change notification settings - Fork 128
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
C19 Kunzite Shay, Amber #110
base: main
Are you sure you want to change the base?
Changes from all commits
b597d9a
5376fef
2d9f87a
11d7486
3de4377
b4b15eb
add8427
78c3334
e39e3f2
4b2410d
6dde8eb
f134c99
0ff118e
9a0da93
464b153
1cb93c7
d82308a
b926255
e47e8e0
f265d54
71bc18e
fb965f5
147d77f
834ce13
b1900cd
8e7ac87
c083977
6c7b13f
ff7f05e
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 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,124 @@ | ||||||||||||
from app import db | ||||||||||||
from app.models.task import Task | ||||||||||||
from app.models.goal import Goal | ||||||||||||
from flask import Blueprint, jsonify, make_response, request, abort | ||||||||||||
from sqlalchemy.types import DateTime | ||||||||||||
from sqlalchemy.sql.functions import now | ||||||||||||
import requests, json | ||||||||||||
import os | ||||||||||||
|
||||||||||||
goal_bp = Blueprint("goal_bp", __name__, url_prefix="/goals") | ||||||||||||
|
||||||||||||
def validate_model_goal(cls, model_id): | ||||||||||||
try: | ||||||||||||
model_id = int(model_id) | ||||||||||||
except: | ||||||||||||
abort(make_response({"message": f"{cls.__name__} {model_id} invalid"}, 400)) | ||||||||||||
|
||||||||||||
goal = cls.query.get(model_id) | ||||||||||||
|
||||||||||||
if not goal: | ||||||||||||
abort(make_response({"message": f"{cls.__name__} {model_id} not found"}, 404)) | ||||||||||||
|
||||||||||||
return goal | ||||||||||||
Comment on lines
+12
to
+23
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. This is identical to the model validation function for |
||||||||||||
|
||||||||||||
@goal_bp.route("", methods=["POST"]) | ||||||||||||
def create_goal(): | ||||||||||||
request_body = request.get_json() | ||||||||||||
is_valid_goal = "title" in request_body | ||||||||||||
if not is_valid_goal: | ||||||||||||
Comment on lines
+28
to
+29
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. This works, but it is more Pythonic to combine this: if "title" not in request_body: |
||||||||||||
abort(make_response({"details": "Invalid data"}, 400)) | ||||||||||||
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. How could we make this error message more descriptive for the user to understand what is missing? Maybe we can say that |
||||||||||||
|
||||||||||||
new_goal = Goal.goal_from_dict(request_body) | ||||||||||||
|
||||||||||||
db.session.add(new_goal) | ||||||||||||
db.session.commit() | ||||||||||||
|
||||||||||||
response_body = { | ||||||||||||
"goal": new_goal.goal_to_dict() | ||||||||||||
} | ||||||||||||
|
||||||||||||
return make_response(jsonify(response_body), 201) | ||||||||||||
|
||||||||||||
@goal_bp.route("", methods=["GET"]) | ||||||||||||
def get_all_goals(): | ||||||||||||
# goal = validate_model_goal() | ||||||||||||
sort_query = request.args.get("sort") | ||||||||||||
if sort_query == "asc": | ||||||||||||
goals = Goal.query.order_by(Goal.title) | ||||||||||||
elif sort_query == "desc": | ||||||||||||
goals = Goal.query.order_by(Goal.title.desc()) | ||||||||||||
else: | ||||||||||||
goals = Goal.query.all() | ||||||||||||
|
||||||||||||
goals_response = [goal.goal_to_dict() for goal in goals] | ||||||||||||
return jsonify(goals_response) | ||||||||||||
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. Oops, don't forget your status code! Technically, it will add one by default, but consider being explicit and always adding the status code. |
||||||||||||
|
||||||||||||
@goal_bp.route("/<goal_id>", methods=["GET"]) | ||||||||||||
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. 👍 |
||||||||||||
def get_one_goal(goal_id): | ||||||||||||
goal = validate_model_goal(Goal, goal_id) | ||||||||||||
response_body = { | ||||||||||||
"goal": goal.goal_to_dict() | ||||||||||||
} | ||||||||||||
return jsonify(response_body) | ||||||||||||
|
||||||||||||
@goal_bp.route("/<goal_id>", methods=["PUT"]) | ||||||||||||
def update_goal(goal_id): | ||||||||||||
goal = validate_model_goal(Goal, goal_id) | ||||||||||||
request_body = request.get_json() | ||||||||||||
goal.title = request_body["title"] | ||||||||||||
|
||||||||||||
db.session.commit() | ||||||||||||
|
||||||||||||
response_body = { | ||||||||||||
"goal": goal.goal_to_dict() | ||||||||||||
} | ||||||||||||
return response_body | ||||||||||||
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. Careful here! While this does work, it is very different form the previous routes you've done. Try to keep them as consistent as possible. If you used |
||||||||||||
|
||||||||||||
@goal_bp.route("/<goal_id>", methods=["DELETE"]) | ||||||||||||
def delete_goal(goal_id): | ||||||||||||
goal = validate_model_goal(Goal, goal_id) | ||||||||||||
response_body = { | ||||||||||||
"details": f'Goal {goal.goal_id} \"{goal.title}\" successfully deleted' | ||||||||||||
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! We can also do this: {"details": f'Goal {goal_id} "{goal.title}" successfully deleted'} |
||||||||||||
} | ||||||||||||
|
||||||||||||
db.session.delete(goal) | ||||||||||||
db.session.commit() | ||||||||||||
|
||||||||||||
return response_body | ||||||||||||
|
||||||||||||
@goal_bp.route("/<goal_id>/tasks", methods=["POST"]) | ||||||||||||
def create_task(goal_id): | ||||||||||||
# validate goal_id | ||||||||||||
goal = validate_model_goal(Goal, goal_id) | ||||||||||||
|
||||||||||||
request_body = request.get_json() | ||||||||||||
|
||||||||||||
db.session.add(request_body) | ||||||||||||
db.session.commit() | ||||||||||||
|
||||||||||||
tasks_resp = [tasks_resp for task in ask.task_with_goal_to_dict] | ||||||||||||
goal.goal_to_dict_with_task() // t | ||||||||||||
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. Hm, what are you dividing by here? What is |
||||||||||||
|
||||||||||||
return jsonify(response_body) | ||||||||||||
|
||||||||||||
@goal_bp.route("/<goal_id>/tasks", methods=["GET"]) | ||||||||||||
def get_tasks_one_goal(goal_id): | ||||||||||||
|
||||||||||||
goal = validate_model_goal(Goal, goal_id) | ||||||||||||
Comment on lines
+106
to
+108
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. Let's make sure our code block is flush against the function definition!
Suggested change
|
||||||||||||
|
||||||||||||
tasks_response = [] | ||||||||||||
for task in goal.tasks: | ||||||||||||
tasks_response.append( | ||||||||||||
{ | ||||||||||||
"id": task.task_id, | ||||||||||||
"title": task.title, | ||||||||||||
"description": task.description, | ||||||||||||
"completed_at": None | ||||||||||||
"goal_id": | ||||||||||||
Comment on lines
+117
to
+118
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. Ah, looks like we forgot to finish out this dictionary. I reran the tests after fixing this, and they all passed, so maybe sometimes during editing this got erased.
Suggested change
|
||||||||||||
} | ||||||||||||
) | ||||||||||||
# | ||||||||||||
# tasks_response = [response_dict for task in goal.tasks] | ||||||||||||
Comment on lines
+121
to
+122
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. Let's get rid of this since we aren't using it!
Suggested change
|
||||||||||||
|
||||||||||||
return jsonify(tasks_response) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,4 +2,33 @@ | |
|
||
|
||
class Goal(db.Model): | ||
goal_id = db.Column(db.Integer, primary_key=True) | ||
goal_id = db.Column(db.Integer, primary_key=True, autoincrement=True) | ||
title = db.Column(db.String) | ||
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. A cool thing we can do is make sure that data with required fields aren't created with NULL values with the argument |
||
tasks = db.relationship("Task", back_populates="goal", lazy=True) | ||
|
||
@classmethod | ||
def goal_from_dict(cls, goal_data): | ||
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. Since this is an instance method designed solely for the |
||
new_goal = Goal(title=goal_data["title"]) | ||
return new_goal | ||
|
||
def goal_to_dict(self): | ||
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. Same here! Let's change this to |
||
goal_as_dict = {} | ||
|
||
goal_as_dict["id"] = self.goal_id | ||
goal_as_dict["title"] = self.title | ||
if self.tasks: | ||
task_list = [] | ||
for task in self.tasks: | ||
task_list.append(task.task_to_dict(self)) | ||
goal_as_dict["tasks"] = task_list | ||
|
||
return goal_as_dict | ||
|
||
def goal_to_dict_with_task(self): | ||
goal_as_dict = {} | ||
|
||
goal_as_dict["id"] = self.goal_id | ||
goal_as_dict["title"] = self.title | ||
goal_as_dict["tasks"] = self.tasks | ||
|
||
return goal_as_dict | ||
Comment on lines
+27
to
+34
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. Looks like you incorporated tasks into the previous method |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,46 @@ | ||
from app import db | ||
from app.models.goal import Goal | ||
|
||
|
||
class Task(db.Model): | ||
task_id = db.Column(db.Integer, primary_key=True) | ||
"""Task definition""" | ||
task_id = db.Column(db.Integer, primary_key=True, autoincrement=True) | ||
title = db.Column(db.String) | ||
description = db.Column(db.String) | ||
completed_at = db.Column(db.DateTime, nullable=True) | ||
goal_id = db.Column(db.Integer, db.ForeignKey("goal.goal_id")) | ||
goal = db.relationship("Goal", back_populates="tasks") | ||
|
||
@classmethod | ||
def task_from_dict(cls, task_data): | ||
"""Input task as a dictionary. Assumes None/null for completed_at""" | ||
|
||
if not "completed_at" in task_data: | ||
task_data["completed_at"] = None | ||
|
||
new_task = Task(title=task_data["title"], | ||
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. What happens if the class name is changed on line 5? We'd have to remember to change it here, too! Or! we can use the new_task = cls(title=task_data["title"], |
||
description=task_data["description"], | ||
completed_at=task_data["completed_at"]) | ||
return new_task | ||
|
||
def task_to_dict(self): | ||
"""Output task information as a dictionary""" | ||
task_as_dict = {} | ||
|
||
task_as_dict["id"] = self.task_id | ||
task_as_dict["title"] = self.title | ||
task_as_dict["description"] = self.description | ||
task_as_dict["is_complete"] = self.completed_at != None | ||
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. Good job! We can even turn this into a ternary expression: task_as_dict["is_complete"] = True if self.completed_at else False or with the task_as_dict["is_complete"] = bool(self.completed_at) |
||
|
||
return task_as_dict | ||
|
||
def task_with_goal_to_dict(self): | ||
task_as_dict = {} | ||
|
||
task_as_dict["id"] = self.task_id | ||
task_as_dict["title"] = self.title | ||
task_as_dict["description"] = self.description | ||
task_as_dict["is_complete"] = self.completed_at != None | ||
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. Good idea using an expression to assign a true or false value! An alternative option might be : bool(self.completed_at) |
||
task_as_dict["goal_id"] = self.goal_id | ||
|
||
return task_as_dict |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -1 +1,161 @@ | ||||||||
from flask import Blueprint | ||||||||
from app import db | ||||||||
from app.models.task import Task | ||||||||
from app.models.goal import Goal | ||||||||
from flask import Blueprint, jsonify, make_response, request, abort | ||||||||
from sqlalchemy.types import DateTime | ||||||||
from sqlalchemy.sql.functions import now | ||||||||
Comment on lines
+5
to
+6
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. We should kept sqlalchemy model functionality and import in the class files themselves, so let's great rid of these. Continue below for more feedback on this.
Suggested change
|
||||||||
import requests, json | ||||||||
import os | ||||||||
# import logging | ||||||||
# from slack_sdk import WebClient | ||||||||
# from slack_sdk.errors import SlackApiError | ||||||||
Comment on lines
+9
to
+11
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. Let's get rid of these since we aren't using them
Suggested change
|
||||||||
|
||||||||
|
||||||||
task_list_bp = Blueprint("task_list_bp", __name__, url_prefix="/tasks") | ||||||||
|
||||||||
def validate_model_task(cls, model_id): | ||||||||
try: | ||||||||
model_id = int(model_id) | ||||||||
except: | ||||||||
abort(make_response({"message": f"{cls.__name__} {model_id} invalid"}, 400)) | ||||||||
|
||||||||
task = cls.query.get(model_id) | ||||||||
|
||||||||
if not task: | ||||||||
abort(make_response({"message": f"{cls.__name__} {model_id} not found"}, 404)) | ||||||||
|
||||||||
return task | ||||||||
Comment on lines
+16
to
+27
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. Let's use the one already defined in |
||||||||
|
||||||||
@task_list_bp.route("", methods=["POST"]) | ||||||||
def create_task(): | ||||||||
request_body = request.get_json() | ||||||||
is_valid_task = "title" in request_body and "description" in request_body | ||||||||
if not is_valid_task: | ||||||||
Comment on lines
+32
to
+33
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. Oh very interesting approach! Any reason you decided to make a variable first rather than combine the two lines together? I'm curious! |
||||||||
abort(make_response({"details": "Invalid data"}, 400)) | ||||||||
|
||||||||
new_task = Task.task_from_dict(request_body) | ||||||||
|
||||||||
db.session.add(new_task) | ||||||||
db.session.commit() | ||||||||
|
||||||||
response_body = { | ||||||||
"task": new_task.task_to_dict() | ||||||||
} | ||||||||
return make_response(jsonify(response_body), 201) | ||||||||
|
||||||||
|
||||||||
@task_list_bp.route("", methods=["GET"]) | ||||||||
def get_all_tasks(): | ||||||||
sort_query = request.args.get("sort") | ||||||||
if sort_query == "asc": | ||||||||
tasks = Task.query.order_by(Task.title) | ||||||||
elif sort_query == "desc": | ||||||||
tasks = Task.query.order_by(Task.title.desc()) | ||||||||
else: | ||||||||
tasks = Task.query.all() | ||||||||
|
||||||||
tasks_response = [task.task_to_dict() for task in tasks] | ||||||||
Comment on lines
+49
to
+57
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. This would make a good filtering helper function! |
||||||||
return jsonify(tasks_response) | ||||||||
|
||||||||
@task_list_bp.route("/<task_id>", methods=["GET"]) | ||||||||
def get_one_task(task_id): | ||||||||
task = validate_model_task(Task, task_id) | ||||||||
response_body = { | ||||||||
"task": task.task_to_dict() | ||||||||
} | ||||||||
return jsonify(response_body) | ||||||||
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. Oops, you forgot your status code! It will implicitly return a status code, but we should add it ourselves to be in control of what is returned |
||||||||
|
||||||||
@task_list_bp.route("/<task_id>", methods=["PUT"]) | ||||||||
def update_task(task_id): | ||||||||
task = validate_model_task(Task, task_id) | ||||||||
request_body = request.get_json() | ||||||||
task.title = request_body["title"] | ||||||||
task.description = request_body["description"] | ||||||||
|
||||||||
db.session.commit() | ||||||||
|
||||||||
response_body = { | ||||||||
"task": task.task_to_dict() | ||||||||
} | ||||||||
return response_body | ||||||||
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. Try to keep your responses as consistent as possible. Since you've used return jsonify(response_body), 200 |
||||||||
|
||||||||
@task_list_bp.route("/<task_id>", methods=["DELETE"]) | ||||||||
def delete_task(task_id): | ||||||||
task = validate_model_task(Task, task_id) | ||||||||
response_body = { | ||||||||
"details": f'Task {task.task_id} \"{task.title}\" successfully deleted' | ||||||||
} | ||||||||
|
||||||||
db.session.delete(task) | ||||||||
db.session.commit() | ||||||||
|
||||||||
return response_body | ||||||||
|
||||||||
@task_list_bp.route("/<task_id>/mark_complete", methods=["PATCH"]) | ||||||||
def mark_task_complete(task_id): | ||||||||
request_body = request.get_json() | ||||||||
|
||||||||
try: | ||||||||
task = Task.query.get(task_id) | ||||||||
task.completed_at = now() | ||||||||
except: | ||||||||
return abort(make_response({"message": f"{task_id} not found"}, 404)) | ||||||||
|
||||||||
db.session.commit() | ||||||||
|
||||||||
response_body = { | ||||||||
"task": task.task_to_dict() | ||||||||
} | ||||||||
|
||||||||
SLACK_BOT_TOKEN = os.environ.get("SLACK_BOT_TOKEN") | ||||||||
my_headers = { | ||||||||
"Authorization": f"Bearer {SLACK_BOT_TOKEN}" | ||||||||
} | ||||||||
|
||||||||
data_payload = { | ||||||||
"channel": "task-notifications", | ||||||||
"text": f"Someone just completed the task {task.title}" | ||||||||
} | ||||||||
|
||||||||
requests.post("https://slack.com/api/chat.postMessage", | ||||||||
headers=my_headers, | ||||||||
data=data_payload) | ||||||||
Comment on lines
+110
to
+122
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. This would be a good candidate for a helper function |
||||||||
|
||||||||
return jsonify(response_body) | ||||||||
|
||||||||
|
||||||||
# ~~Below is the the code I would have used given Slack documentation for calling their API~~ | ||||||||
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. Good job working with the documentation to find a solution! 👍 |
||||||||
|
||||||||
# client = WebClient(token=os.environ.get("SLACK_BOT_TOKEN")) | ||||||||
# logger = logging.getLogger(__name__) | ||||||||
# channel_id = "task-notifications" | ||||||||
|
||||||||
# try: | ||||||||
# # Call the chat.postMessage method using the WebClient | ||||||||
# result = client.chat_postMessage( | ||||||||
# channel=channel_id, | ||||||||
# text=f"Someone just completed the task {task.title}." | ||||||||
# ) | ||||||||
# logger.info(result) | ||||||||
|
||||||||
# except SlackApiError as e: | ||||||||
# logger.error(f"Error posting message: {e}") | ||||||||
|
||||||||
|
||||||||
|
||||||||
@task_list_bp.route("/<task_id>/mark_incomplete", methods=["PATCH"]) | ||||||||
def mark_task_incomplete(task_id): | ||||||||
request_body = request.get_json() | ||||||||
try: | ||||||||
task=Task.query.get(task_id) | ||||||||
task.completed_at = None | ||||||||
except: | ||||||||
response_body = abort(make_response({"message": f"{task_id} not found"}, 404)) | ||||||||
return response_body | ||||||||
Comment on lines
+153
to
+154
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. I believe abort(make_response({"message": f"{task_id} not found"}, 404)) |
||||||||
|
||||||||
db.session.commit() | ||||||||
response_body = { | ||||||||
"task": task.task_to_dict() | ||||||||
} | ||||||||
|
||||||||
return jsonify(response_body) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Generic single-database configuration. |
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.
We should kept sqlalchemy model functionality and import in the class files themselves, so let's great rid of these.