Skip to content

Whitney Shake - Zoisite - Task List #113

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

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def create_app(test_config=None):

if test_config is None:
app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
"SQLALCHEMY_DATABASE_URI")
"RENDER_DATABASE_URI")
else:
app.config["TESTING"] = True
app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
Expand All @@ -30,5 +30,10 @@ def create_app(test_config=None):
migrate.init_app(app, db)

# Register Blueprints here
from .routes.task_routes import tasks_bp
app.register_blueprint(tasks_bp)

from .routes.goal_routes import goals_bp
app.register_blueprint(goals_bp)

return app
21 changes: 21 additions & 0 deletions app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,26 @@
from app import db
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to make an empty init.py file in any package folder/subfolder. app and routes have one, but we should have one here in the models folder as well.




class Goal(db.Model):
goal_id = db.Column(db.Integer, primary_key=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My personal preference is to name id attributes just id and then when you have a goal object you can write goal.id instead of goal.goal_id.

title = db.Column(db.String)
tasks = db.relationship("Task", back_populates="goal", lazy=True)

def to_dict(self, tasks=False):

build_dict = {
"id": self.goal_id,
"title": self.title
}

if tasks:
build_dict["tasks"] = [task.to_dict() for task in self.tasks]
Comment on lines +17 to +18
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done! You could even remove tasks=False from the parameter and have the check on line 17 be if self.tasks: instead


return build_dict
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more descriptive name for this dictionary might be goal_dict or goal_response_dict


@classmethod
def from_dict(cls, build_dict):
new_goal = cls(title=build_dict["title"])
return new_goal

39 changes: 38 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,41 @@


class Task(db.Model):
task_id = db.Column(db.Integer, primary_key=True)
task_id = db.Column(db.Integer, primary_key=True, autoincrement=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as in goal, prefer primary key just to be called id

title = db.Column(db.String)
description = db.Column(db.String)
completed_at = db.Column(db.DateTime(timezone=True), nullable=True)
goal_id = db.Column(db.Integer, db.ForeignKey("goal.goal_id"), nullable=True)
Comment on lines +8 to +9
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default value for nullable is True, so we don't need to list this explicitly.

goal = db.relationship("Goal", back_populates="tasks")


def to_dict(self):

task_dict = dict(
id = self.task_id,
title = self.title,
description = self.description,
is_complete = self.is_task_complete()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 another way you could write this i to use a ternary like:

is_complete = True if self.completed_at else False

)

if self.goal_id:
task_dict["goal_id"] = self.goal_id

return task_dict

def is_task_complete(self):
if self.completed_at == None:
return False
else:
return True

@classmethod
def from_dict(cls, task_dict):
new_task = Task(
title=task_dict["title"],
description=task_dict["description"],
)
return new_task



1 change: 0 additions & 1 deletion app/routes.py

This file was deleted.

Empty file added app/routes/__init__.py
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 yep, we need an __init__.py in each package

Empty file.
59 changes: 59 additions & 0 deletions app/routes/goal_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
from flask import Blueprint, make_response, request
from app.models.goal import Goal
from app.models.task import Task
from app.routes.helpers import validate_model, create_item, get_all_items, get_one_item, update_item, delete_item
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting approach! Abstracting away all the logic for the routes into a helper file definitely makes all of your goal routes very concise. It works here since goals and tasks are pretty similar overall. In the future, you may find that there isn't as much similarity between models and you'll need to write your logic in each of your routes.

from app import db

goals_bp = Blueprint("goals", __name__, url_prefix="/goals")

@goals_bp.route("", methods=["POST"])
def create_goal():
return create_item(Goal)


@goals_bp.route("", methods=["GET"])
def get_all_goals():
return get_all_items(Goal)


@goals_bp.route("/<goal_id>", methods=["GET"])
def get_one_goal(goal_id):
return get_one_item(Goal, goal_id)


@goals_bp.route("<goal_id>", methods=["PUT"])
def update_task(goal_id):
return update_item(Goal, goal_id)


@goals_bp.route("<goal_id>", methods=["DELETE"])
def delete_goal(goal_id):
return delete_item(Goal, goal_id)


@goals_bp.route("/<goal_id>/tasks", methods=['POST'])
def create_goal_with_tasks(goal_id):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking we're not creating new goals, but associating existing tasks with existing goals. Maybe a name like associate_goal_with_tasks might be more descriptive


goal = validate_model(Goal, goal_id)
request_body = request.get_json()
task_ids = request_body.get("task_ids")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you used .get() to get the task_ids. Consider passing a default value that is an empty list, like: task_ids = request_body.get("task_ids", [])

As is, if the key "task_ids" is not in request_body then the variable task_ids will reference None. Then when you try to loop over None, you'll run into another unhandled exception (Nonetype is not iterable or something like that)


for task_id in task_ids:
task = validate_model(Task, task_id)
task.goal_id = goal.goal_id

db.session.commit()

message = {
"id": goal.goal_id,
"task_ids": task_ids
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer that the list of task ids that you return to the client comes from the goal object that you validated, not from the request body. It's a slight difference, but request_body["task_ids"] gets the list from what the client originally sent. But you validated goal and the tasks and then associated them with each other on line 43. After that validating and associating, we can use goal.tasks to get the task ids "from the source" and return to the client.

}

return make_response(message, 200)


@goals_bp.route("/<goal_id>/tasks", methods=['GET'])
def get_all_tasks_one_goal(goal_id):

goal = validate_model(Goal, goal_id)
return make_response(goal.to_dict(tasks=True), 200)
140 changes: 140 additions & 0 deletions app/routes/helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
from flask import jsonify, abort, make_response, request
from dotenv import load_dotenv
import requests
from datetime import datetime
from app import db
import os

load_dotenv()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to invoke load_dotenv() since it was already executed in the create_app() method on startup. When that happens all the values in the .env file are loaded up and we can just import os in this file and grab the values without needing to call load_dotenv() again


def send_slack_message(completed_task):
TOKEN = os.environ['SLACK_API_TOKEN']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll work on APIs where a route will have to make more than one external call to an API so we should be more specific about the kind of token we're using. This variable could be called SLACK_API_TOKEN

AUTH_HEADERS = {
"Authorization": f"Bearer {TOKEN}"
}
CHANNEL_ID = "C0561UUDX4K"
SLACK_URL = "https://slack.com/api/chat.postMessage"

try:
message = f"Someone just completed the task {completed_task.title}"
payload = {
"channel": CHANNEL_ID,
"text": message
}

requests.post(SLACK_URL, data = payload, headers = AUTH_HEADERS)

except:
print("There was an error making the call to Slack")
Comment on lines +18 to +28
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍



def validate_model(cls, model_id):
try:
model_id = int(model_id)
except:
abort(make_response({"details": "Invalid data"}, 400))

model = cls.query.get(model_id)

if not model:
message = {"details": "Invalid data"}
abort(make_response(message, 404))

return model


def create_item(cls):

request_body = request.get_json()

try:
new_item = cls.from_dict(request_body)
db.session.add(new_item)
db.session.commit()
return make_response({cls.__name__.lower(): new_item.to_dict()}, 201)
except:
return make_response({"details": "Invalid data"}, 400)
Comment on lines +50 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice error handling in case the request_body has invalid data



def get_all_items(cls):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


sort_query = request.args.get("sort")

if sort_query == "asc":
items = cls.query.order_by(cls.title)

elif sort_query == "desc":
items = cls.query.order_by(cls.title.desc())

else:
items = cls.query.all()

if request.args.get("title"):
items = cls.query.filter(cls.title== request.args.get("title"))

items_response = [item.to_dict() for item in items]

return jsonify(items_response), 200


def get_one_item(cls, model_id):

item = validate_model(cls, model_id)
return make_response({cls.__name__.lower(): item.to_dict()}), 200


def update_item(cls, model_id):
try:
item = validate_model(cls, model_id)
request_body = request.get_json()

for key, value in request_body.items():
setattr(item, key, value)

db.session.commit()

return make_response({cls.__name__.lower(): item.to_dict()}, 200)
Comment on lines +87 to +96
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job adding error handling for the PUT route because we might find that a client sent over incorrect data in the request body (like would happen in a POST route)


except:
return jsonify({"Message": "Invalid id"}), 404


def delete_item(cls, model_id):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


item = validate_model(cls, model_id)

db.session.delete(item)
db.session.commit()

message = {"details": f"{cls.__name__} {model_id} \"{item.title}\" successfully deleted"}
return make_response(message, 200)


def mark_item_complete(cls, model_id):
try:
new_item = validate_model(cls, model_id)
except:
return jsonify({"Message": "Invalid id"}), 404

new_item.completed_at = datetime.utcnow()

send_slack_message(new_item)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job pulling this logic into a helper function


db.session.commit()

return jsonify({"task": new_item.to_dict()}), 200


def mark_item_incomplete(cls, model_id):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


try:
new_item = validate_model(cls, model_id)
except:
return jsonify({"Message": "Invalid id"}), 404

new_item.completed_at = None

db.session.commit()

return jsonify({"task": new_item.to_dict()}), 200

41 changes: 41 additions & 0 deletions app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
from flask import Blueprint
from app.models.task import Task
from app.routes.helpers import create_item, get_all_items, get_one_item, update_item, delete_item, mark_item_complete, mark_item_incomplete


tasks_bp = Blueprint("tasks", __name__, url_prefix="/tasks")


@tasks_bp.route("", methods=["POST"])
def create_task():
return create_item(Task)


@tasks_bp.route("", methods=["GET"])
def get_all_tasks():
return get_all_items(Task)


@tasks_bp.route("/<task_id>", methods=["GET"])
def get_one_task(task_id):
return get_one_item(Task, task_id)


@tasks_bp.route("<task_id>", methods=["PUT"])
def update_task(task_id):
return update_item(Task, task_id)


@tasks_bp.route("<task_id>", methods=["DELETE"])
def delete_task(task_id):
return delete_item(Task, task_id)


@tasks_bp.route("<task_id>/mark_complete", methods=["PATCH"])
def mark_task_complete(task_id):
return mark_item_complete(Task, task_id)


@tasks_bp.route("<task_id>/mark_incomplete", methods=["PATCH"])
def mark_task_incomplete(task_id):
return mark_item_incomplete(Task, task_id)
Comment on lines +36 to +41
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might argue that the code in mark_item_complete and mark_item_incomplete can be written directly in these routes since we know this logic probably won't be reused in the Goal routes. I feel this way because the logic in those helpers are directly related to these routes and so we might as well just write them here.

1 change: 1 addition & 0 deletions migrations/README
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Generic single-database configuration.
45 changes: 45 additions & 0 deletions migrations/alembic.ini
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
Loading