-
Notifications
You must be signed in to change notification settings - Fork 146
Tigers - Maria S. #134
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: master
Are you sure you want to change the base?
Tigers - Maria S. #134
Changes from all commits
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,5 +1,25 @@ | ||
from app import db | ||
from sqlalchemy import Column, ForeignKey, Integer, Table | ||
from sqlalchemy.orm import relationship | ||
|
||
|
||
class Goal(db.Model): | ||
goal_id = db.Column(db.Integer, primary_key=True) | ||
title = db.Column(db.String) | ||
tasks = db.relationship('Task', back_populates='goal', lazy=True) | ||
|
||
def to_dict(self): | ||
goal_as_dict = {} | ||
goal_as_dict["id"] = self.goal_id | ||
goal_as_dict["title"] = self.title | ||
|
||
if self.tasks: | ||
goal_as_dict["tasks"] = self.tasks.title | ||
|
||
return goal_as_dict | ||
|
||
|
||
def from_json(cls, req_body): | ||
return cls( | ||
title = req_body["title"] | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,35 @@ | ||
from app import db | ||
from sqlalchemy import Column, ForeignKey, Integer, Table | ||
from sqlalchemy.orm import relationship | ||
|
||
|
||
class Task(db.Model): | ||
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. Other than that issue I pointed out, this model class for |
||
task_id = db.Column(db.Integer, primary_key=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'), nullable=True) | ||
goal = db.relationship("Goal", back_populates="tasks", lazy=True) | ||
|
||
def to_dict(self): | ||
task_as_dict = {} | ||
task_as_dict["is_complete"] = False | ||
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 doesn't match the specification, as it means that the response will always have While you fix this in your response to There are a few different ways you could have set |
||
task_as_dict["id"] = self.task_id | ||
task_as_dict["title"] = self.title | ||
task_as_dict["description"] = self.description | ||
|
||
if self.goal_id: | ||
task_as_dict["goal_id"] = self.goal_id | ||
|
||
return task_as_dict | ||
|
||
|
||
def from_json(cls, req_body): | ||
return cls( | ||
title = req_body["title"], | ||
description = req_body["description"], | ||
completed_at = req_body["completed_at"] | ||
) | ||
|
||
|
||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,243 @@ | ||
from os import abort | ||
from app import db | ||
from app.models.task import Task | ||
from app.models.goal import Goal | ||
from flask import Blueprint, jsonify, abort, make_response, request | ||
import datetime | ||
import requests | ||
import os | ||
|
||
tasks_bp = Blueprint("tasks", __name__, url_prefix="/tasks") | ||
goals_bp = Blueprint("goals", __name__, url_prefix="/goals") | ||
|
||
@goals_bp.route("/<goal_id>/tasks", methods=["POST"]) | ||
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 originally had a comment here that your file seemed to lack organization, but as I've read more, I now get that it's just these two A common pattern is to arrange routes by increasing size of endpoint URL then by HTTP verbs, which you do seem to follow below. Moving this to after the task completion routes would maintain that organization, which makes your code more maintainable as there's an obvious pattern methods are organized in. (Though it can be hard to get a good organization that works, I admit!) |
||
def add_task_list_to_goal(goal_id): | ||
goal = validate_model(Goal, goal_id) | ||
request_body = request.get_json() | ||
list_task_id = [] | ||
|
||
for task_id in request_body["task_ids"]: | ||
list_task_id.append(task_id) | ||
task = validate_model(Task, task_id) | ||
goal.tasks.append(task) | ||
db.session.commit() | ||
new_goal = { | ||
"id" : goal.goal_id, | ||
"task_ids" : list_task_id | ||
} | ||
|
||
return jsonify (new_goal), 200 | ||
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. In this case where you are returning a dictionary, you don't need Likewise, if your status code is going to be 200, you don't need to supply it in your 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. Also, watch spacing! There's an extra space between 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. There are a few other places where you use |
||
|
||
@goals_bp.route("/<goal_id>/tasks", methods=["GET"]) | ||
def rad_all_tasks(goal_id): | ||
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. Naming: Not only is this name misspelled (I'm guessing it should be I would recommend something like (Now this technically isn't a name collision because this function is distinguished from |
||
goal = validate_model(Goal, goal_id) | ||
try: | ||
response_body = { | ||
"id":goal.goal_id, | ||
"title": goal.title, | ||
"tasks": [task.to_dict() for task in goal.tasks] | ||
} | ||
for task in response_body["tasks"]: | ||
task["goal_id"] = goal.goal_id | ||
|
||
# return make_response(jsonify(response_body), 200) | ||
except: | ||
response_body = { | ||
"id":goal.goal_id, | ||
"title": goal.title, | ||
"tasks": [] | ||
} | ||
Comment on lines
+34
to
+49
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 block is a bit more complicated than you really need. Putting this code in a Like I said, this would be different if Likewise, your So doing both of those changes brings this down to: response_body = {
"id": goal.goal_id,
"title": goal.title,
"tasks": [task.to_dict() for task in goal.tasks]
return response_body |
||
|
||
return jsonify(response_body), 200 | ||
|
||
|
||
@goals_bp.route("", methods=["POST"]) | ||
def add_goal(): | ||
try: | ||
request_body = request.get_json() | ||
new_goal = Goal.from_json(Goal, request_body) | ||
db.session.add(new_goal) | ||
db.session.commit() | ||
goals_response = {} | ||
goal_in_dict = new_goal.to_dict() | ||
goals_response["goal"] = goal_in_dict | ||
|
||
return jsonify (goals_response), 201 | ||
|
||
except: | ||
response_body = {} | ||
response_body["details"] = "Invalid data" | ||
|
||
return response_body, 400 | ||
|
||
@tasks_bp.route("", methods=["POST"]) | ||
def add_task(): | ||
try: | ||
request_body = request.get_json() | ||
if "completed_at" not in request_body: | ||
request_body["completed_at"] = None | ||
|
||
new_task = Task.from_json(Task, request_body) | ||
db.session.add(new_task) | ||
db.session.commit() | ||
tasks_response = {} | ||
task_in_dict = new_task.to_dict() | ||
tasks_response["task"] = task_in_dict | ||
|
||
return jsonify (tasks_response), 201 | ||
|
||
except: | ||
response_body = {} | ||
response_body["details"] = "Invalid data" | ||
|
||
return response_body, 400 | ||
|
||
@goals_bp.route("", methods=["GET"]) | ||
def read_all_goals(): | ||
goals = Goal.query.all() | ||
goals_response = [] | ||
for goal in goals: | ||
goals_response.append(goal.to_dict()) | ||
|
||
return jsonify (goals_response), 200 | ||
|
||
@tasks_bp.route("", methods=["GET"]) | ||
def read_all_tasks(): | ||
title_query = request.args.get("title") | ||
description_query = request.args.get("description") | ||
completed_query = request.args.get("completed_at") | ||
sort_query = request.args.get("sort") | ||
|
||
if sort_query == "desc": | ||
tasks = Task.query.order_by(Task.title.desc()) | ||
elif sort_query == "asc": | ||
tasks = Task.query.order_by(Task.title.asc()) | ||
elif title_query: | ||
tasks = Task.query.filter_by(title=title_query) | ||
elif description_query: | ||
tasks = Task.query.filter_by(description=description_query) | ||
elif completed_query: | ||
tasks = Task.query.filter_by(completed_at=completed_query) | ||
else: | ||
tasks = Task.query.all() | ||
Comment on lines
+111
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. Good extra work on getting these other columns! |
||
|
||
tasks_response = [] | ||
for task in tasks: | ||
tasks_response.append(task.to_dict()) | ||
|
||
return jsonify (tasks_response), 200 | ||
|
||
@goals_bp.route("/<goal_id>", methods=["GET"]) | ||
def read_one_goal(goal_id): | ||
goal = validate_model(Goal, goal_id) | ||
goals_response = {} | ||
goal_in_dict = goal.to_dict() | ||
goals_response["goal"]=goal_in_dict | ||
|
||
return jsonify (goals_response), 200 | ||
|
||
@tasks_bp.route("/<task_id>", methods=["GET"]) | ||
def read_one_task(task_id): | ||
task = validate_model(Task, task_id) | ||
tasks_response = {} | ||
task_in_dict = task.to_dict() | ||
tasks_response["task"]=task_in_dict | ||
|
||
return jsonify (tasks_response), 200 | ||
|
||
@goals_bp.route("/<goal_id>", methods=["PUT"]) | ||
def update_goal(goal_id): | ||
goal = validate_model(Goal, goal_id) | ||
request_body = request.get_json() | ||
goal.title = request_body["title"] | ||
db.session.commit() | ||
goals_response = {} | ||
goal_in_dict = goal.to_dict() | ||
goals_response["goal"]=goal_in_dict | ||
|
||
return jsonify (goals_response), 200 | ||
|
||
@tasks_bp.route("/<task_id>", methods=["PUT"]) | ||
def update_task(task_id): | ||
task = validate_model(Task, task_id) | ||
request_body = request.get_json() | ||
task.title = request_body["title"] | ||
task.description = request_body["description"] | ||
db.session.commit() | ||
tasks_response = {} | ||
task_in_dict = task.to_dict() | ||
tasks_response["task"]=task_in_dict | ||
|
||
return jsonify (tasks_response), 200 | ||
|
||
@goals_bp.route("/<goal_id>", methods=["DELETE"]) | ||
def delete_goal(goal_id): | ||
goal = validate_model(Goal, goal_id) | ||
db.session.delete(goal) | ||
db.session.commit() | ||
response_body = {} | ||
response_body["details"] = (f'Goal {goal_id} "{goal.title}" successfully deleted') | ||
|
||
return response_body, 200 | ||
|
||
@tasks_bp.route("/<task_id>", methods=["DELETE"]) | ||
def delete_task(task_id): | ||
task = validate_model(Task, task_id) | ||
db.session.delete(task) | ||
db.session.commit() | ||
response_body = {} | ||
response_body["details"] = (f'Task {task_id} "{task.title}" successfully deleted') | ||
|
||
return response_body, 200 | ||
|
||
@tasks_bp.route("/<task_id>/mark_complete", methods=["PATCH"]) | ||
def mark_complete_on_incomplete_task(task_id): | ||
task = validate_model(Task, task_id) | ||
task.completed_at = datetime.datetime.utcnow() | ||
db.session.commit() | ||
slack_bot(f"Someone just completed the task {task.title}") | ||
task_in_dict = task.to_dict() | ||
task_in_dict["is_complete"] = True | ||
tasks_response = {"task" : task_in_dict} | ||
|
||
return jsonify (tasks_response), 200 | ||
|
||
@tasks_bp.route("/<task_id>/mark_incomplete", methods=["PATCH"]) | ||
def mark_incomplete_on_complete_task(task_id): | ||
task = validate_model(Task, task_id) | ||
task.completed_at = None | ||
db.session.commit() | ||
tasks_response = {} | ||
task_in_dict = task.to_dict() | ||
tasks_response["task"]=task_in_dict | ||
|
||
return jsonify (tasks_response), 200 | ||
|
||
# slack bot helper func | ||
def slack_bot(msg): | ||
PATH = "https://slack.com/api/chat.postMessage" | ||
SLACK_API_KEY = os.environ.get("SLACK_API_KEY") | ||
|
||
query_params = { | ||
"channel" : "#task-notifications", | ||
"text" : msg | ||
} | ||
headers = { | ||
"Authorization" : f"Bearer {SLACK_API_KEY}" | ||
} | ||
|
||
requests.post(PATH, params=query_params, headers=headers) | ||
|
||
# verify id | ||
def validate_model(cls, model_id): | ||
try: | ||
model_id = int(model_id) | ||
except: | ||
abort(make_response({"message":f"{cls.__name__} {model_id} invalid"}, 400)) | ||
|
||
model = cls.query.get(model_id) | ||
|
||
if not model: | ||
abort(make_response({"message":f"{cls.__name__} {model_id} not found"}, 404)) | ||
|
||
return model |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Generic single-database configuration. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
# A generic, single database configuration. | ||
|
||
[alembic] | ||
# template used to generate migration files | ||
# file_template = %%(rev)s_%%(slug)s | ||
|
||
# set to 'true' to run the environment during | ||
# the 'revision' command, regardless of autogenerate | ||
# revision_environment = false | ||
|
||
|
||
# Logging configuration | ||
[loggers] | ||
keys = root,sqlalchemy,alembic | ||
|
||
[handlers] | ||
keys = console | ||
|
||
[formatters] | ||
keys = generic | ||
|
||
[logger_root] | ||
level = WARN | ||
handlers = console | ||
qualname = | ||
|
||
[logger_sqlalchemy] | ||
level = WARN | ||
handlers = | ||
qualname = sqlalchemy.engine | ||
|
||
[logger_alembic] | ||
level = INFO | ||
handlers = | ||
qualname = alembic | ||
|
||
[handler_console] | ||
class = StreamHandler | ||
args = (sys.stderr,) | ||
level = NOTSET | ||
formatter = generic | ||
|
||
[formatter_generic] | ||
format = %(levelname)-5.5s [%(name)s] %(message)s | ||
datefmt = %H:%M:%S |
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.
This code causes a 500 error when calling
GET /goals/<goal_id>
if the goal has tasks associated with it. My guess is you meant something like[task.title for task in self.tasks]
, but your code here means you're trying to access thetitle
attribute of the list (really anInstrumentedList
) rather than its elements.That being said, rather than returning the title of the tasks here, if you used this for , I would recommend returning the tasks themselves, using the code you wrote for
GET /goals/<goal_id>/tasks
,[task.to_dict() for task in self.tasks]
. Handling this here would have greatly reduced the amount of code needed for yourGET /goals/<goal_id>/tasks
route as you could just returngoal.to_dict
.I do recognize that you cannot return the
"tasks"
key here as it will cause some failures in the tests you wrote forGET /goals/<goal_id>
that don't expect this key. However, I think that could be better fixed by updating the test or writing the test in a more future-proof way I'll describe in a comment on the test.