Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Procfile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
web: gunicorn 'app:create_app()'
10 changes: 9 additions & 1 deletion app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from flask import Flask
from flask import Flask, render_template
from flask_sqlalchemy import SQLAlchemy
from flask_migrate import Migrate
import os
Expand All @@ -12,6 +12,7 @@

def create_app(test_config=None):
app = Flask(__name__)
app.url_map.strict_slashes = False

Choose a reason for hiding this comment

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

Nice way to set this across all your blueprints!

app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False

if test_config is None:
Expand All @@ -30,5 +31,12 @@ def create_app(test_config=None):
migrate.init_app(app, db)

# Register Blueprints here
from .routes import tasks_bp, goals_bp
app.register_blueprint(tasks_bp)
app.register_blueprint(goals_bp)
Comment on lines +34 to +36

Choose a reason for hiding this comment

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

👍

One thing we started to touch on in the video store live code was that we can split routes into multiple files. We can make a routes folder, and put routes for each endpoint into separate files, named for their model. Then we can use the name bp for the blueprint in each file since it would be the only blueprint in the file. Then these imports might look like:

    from .routes import task, goal
    app.register_blueprint(task.bp)
    app.register_blueprint(goal.bp)


@app.route("/")
def index():
return render_template("index.html")
Comment on lines +38 to +40

Choose a reason for hiding this comment

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

Nice one-off route for your documentation page!


return app
2 changes: 2 additions & 0 deletions app/models/goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@

class Goal(db.Model):
goal_id = db.Column(db.Integer, primary_key=True)
title = db.Column(db.String)
tasks = db.relationship("Task", backref="goal", lazy=True)

Choose a reason for hiding this comment

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

👍

There are lots of interesting values that lazy can be set to. True is a synonym for "select", and works great here. But check out some of the other options. 😄

5 changes: 5 additions & 0 deletions app/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@

class Task(db.Model):
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(

Choose a reason for hiding this comment

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

👍

'goal.goal_id'), nullable=True)
274 changes: 273 additions & 1 deletion app/routes.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,274 @@
from flask import Blueprint
import requests
from dotenv import load_dotenv
import os
import datetime
from sqlalchemy import desc
from app import db
from app.models.task import Task
from app.models.goal import Goal
from flask import request, Blueprint, make_response, jsonify
tasks_bp = Blueprint("tasks", __name__, url_prefix="/tasks")
goals_bp = Blueprint("goals", __name__, url_prefix="/goals")
Comment on lines +10 to +11

Choose a reason for hiding this comment

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

We could consider putting all the routes related to the tasks endpoint in their own file, and doing the same for the goals routes.


load_dotenv()

Choose a reason for hiding this comment

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

I'm curious why you decided to put the dotenv stuff here rather than in the app/__init__.py



@tasks_bp.route("/", methods=["GET", "POST"])

Choose a reason for hiding this comment

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

The logic for GET and POST doesn't share any code, so we could consider putting the logic for each in separate functions, maybe get_tasks and create_task.

def handle_tasks():
if request.method == "GET":
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(desc(Task.title))

Choose a reason for hiding this comment

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

Can also be written this way, which avoids the need to import desc

            tasks = Task.query.order_by(Task.title.desc())

else:
tasks = Task.query.all()
Comment on lines +19 to +25

Choose a reason for hiding this comment

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

👍


tasks_response = []
for task in tasks:
if not task.completed_at:
is_complete = False
else:
is_complete = True
tasks_response.append({
"id": task.task_id,
"title": task.title,
"description": task.description,
"is_complete": is_complete
})
Comment on lines +33 to +38

Choose a reason for hiding this comment

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

There are many places in your routes where you build a dictionary like this (or very similar). Consider making a helper method, either here in the routes file (e.g. def task_to_dict(task)) or as an instance method of the Task model class (e.g. def to_dict(self)).

return make_response(jsonify(tasks_response), 200)

elif request.method == "POST":
request_body = request.get_json()
keys = ['title', 'description', 'completed_at']
for key in keys:
if key not in request_body:
task_response = {"details": "Invalid data"}
return make_response(task_response, 400)
Comment on lines +43 to +47

Choose a reason for hiding this comment

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

👍

Nice extensible way to check for fields in the request. Note that this would still let us create a task with an empty title (either empty string or explicitly set to null in the JSON body). We don't have a test that checks this, but it would be good to keep in mind.

Additionally, we should be doing similar checks when PUTting a task as well (even though there aren't tests for it). So we could also think about moving checks like this into validation helpers so that they are easier to reuse elsewhere.

We could even think about adding a class method to Task to create a new Task using the dictionary from a request body

    @classmethod
    def from_dict(values):
        # create a new task, and set the model values from the values passed in
        # be sure to validate that all required values are present, we could return `None` or raise an error if needed
        return new_task


new_task = Task(title=request_body["title"],
description=request_body["description"],
completed_at=request_body["completed_at"])

db.session.add(new_task)
db.session.commit()
if not request_body["completed_at"]:
is_complete = False
else:
is_complete = True
task_response = {"task":
{"id": new_task.task_id,
"title": new_task.title,
"description": new_task.description,
"is_complete": is_complete}
}
return make_response(task_response, 201)


@tasks_bp.route("/<task_id>", methods=["GET", "PUT", "DELETE"])
def handle_task(task_id):
task = Task.query.get(task_id)
if not task:
return make_response(f"Task #{task_id} Not Found", 404)
Comment on lines +70 to +72

Choose a reason for hiding this comment

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

👍

We could also consider using get_or_404.

We do need to do this check for GET, PUT, and DELETE requests, but we could still think about splitting these into separate functions (e.g. get_task, update_task, and delete_task).


if request.method == "GET":
if not task.completed_at:
is_complete = False
else:
is_complete = True
task_response = {"task": {
"id": task.task_id,
"title": task.title,
"description": task.description,
"is_complete": is_complete,
}}
if task.goal_id:
task_response["task"]["goal_id"] = task.goal_id
Comment on lines +85 to +86

Choose a reason for hiding this comment

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

👍

Nice way to conditionally add the goal_id field.

return make_response(task_response, 200)
elif request.method == "PUT":
form_data = request.get_json()
if ("completed_at" not in form_data or
form_data["completed_at"] == None):
is_complete = False
else:
is_complete = True
task.title = form_data["title"]
task.description = form_data["description"]
task.completed_at = form_data["completed_at"]
Comment on lines +95 to +97

Choose a reason for hiding this comment

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

I mentioned this already above, but we should be sure that the same fields required for POSTing are included here for PUT. PUT replaces the value for the supplied task id, so we should ensure that all of the values required to represent a Task are provided with the same restrictions as we had when creating it in the first place.

db.session.commit()
task_response = {"task": {
"id": task.task_id,
"title": task.title,
"description": task.description,
"is_complete": is_complete
}}
return make_response(task_response, 200)

elif request.method == "DELETE":
db.session.delete(task)
db.session.commit()
task_response = {
"details": f"Task {task.task_id} \"{task.title}\" successfully deleted"
}
return make_response(task_response, 200)


@tasks_bp.route("/<task_id>/mark_complete", methods=["PATCH"])
def handle_task_complete(task_id):
task = Task.query.get(task_id)
if not task:
return make_response(f"Task #{task_id} Not Found", 404)

task.completed_at = datetime.datetime.utcnow()
db.session.commit()
channel = "channel=task-notifications"
text = f"text=Someone just completed the task {task.title}"

path = f"https://slack.com/api/chat.postMessage?{channel}&{text}"

Choose a reason for hiding this comment

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

Rather than us being responsible for adding query params into the url directly, we can make use of the params named param on the call to requests.post. Doing so would allow the requests library itself to do any necessary encoding of our data values, and we get to work with a nice Python dictionary.

Even better, since this is a post request, we can send the parameters in the post body using the data named parameter (similar to when we pass things as json but the way the values are packed into the request is a little different).


SLACKBOT_AUTH = os.environ.get(
"SLACKBOT_AUTH")

requests.post(path, headers={'Authorization': f'Bearer {SLACKBOT_AUTH}'})

task_response = {"task": {
"id": task.task_id,
"title": task.title,
"description": task.description,
"is_complete": True
}}
return make_response(task_response, 200)


@tasks_bp.route("/<task_id>/mark_incomplete", methods=["PATCH"])
def handle_task_incomplete(task_id):
# get current data, set the thing
task = Task.query.get(task_id)
if not task:
return make_response(f"Task #{task_id} Not Found", 404)

task.completed_at = None
db.session.commit()

task_response = {"task": {
"id": task.task_id,
"title": task.title,
"description": task.description,
"is_complete": False
}}
return make_response(task_response, 200)


@goals_bp.route("/", methods=["GET", "POST"])
def handle_goals():

Choose a reason for hiding this comment

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

Similar feedback about splitting these functions, and moving validation and dictionary-handling logic around that I made for Task will also apply to Goals. These are common themes for any model with CRUD operations.

if request.method == "GET":
goals = Goal.query.all()

goals_response = []

for goal in goals:
goals_response.append({
"id": goal.goal_id,
"title": goal.title
})
return make_response(jsonify(goals_response), 200)

elif request.method == "POST":
request_body = request.get_json()
if 'title' not in request_body:
goal_response = {"details": "Invalid data"}
return make_response(goal_response, 400)

new_goal = Goal(title=request_body["title"])

db.session.add(new_goal)
db.session.commit()

goal_response = {"goal":
{"id": new_goal.goal_id,
"title": new_goal.title,
}}
return make_response(goal_response, 201)


@goals_bp.route("/<goal_id>", methods=["GET", "PUT", "DELETE"])
def handle_goal(goal_id):
goal = Goal.query.get(goal_id)
if not goal:
return make_response(f"Goal #{goal_id} Not Found", 404)

if request.method == "GET":
goal_response = {"goal": {
"id": goal.goal_id,
"title": goal.title
}}
return make_response(goal_response, 200)

elif request.method == "PUT":
form_data = request.get_json()
if not 'title' in form_data:
goal_response = {"details": "Invalid data"}
return make_response(goal_response, 400)

goal.title = form_data["title"]
db.session.commit()

goal_response = {"goal": {
"id": goal.goal_id,
"title": goal.title
}}
return make_response(goal_response, 200)

elif request.method == "DELETE":
db.session.delete(goal)
db.session.commit()
goal_response = {
"details": f"Goal {goal.goal_id} \"{goal.title}\" successfully deleted"
}
return make_response(goal_response, 200)


@goals_bp.route("/<goal_id>/tasks", methods=["POST", "GET"])
def handle_goal_tasks(goal_id):
goal = Goal.query.get(goal_id)
if not goal:
return make_response(f"Goal #{goal_id} Not Found", 404)

if request.method == "POST":
request_body = request.get_json()
task_ids = request_body["task_ids"]
for task_id in task_ids:
task = Task.query.get(task_id)
# if not task:
# return make_response(f"Task #{task_id} Not Found", 404)
Comment on lines +242 to +243

Choose a reason for hiding this comment

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

Curious why you commented this out. We should check to make sure that each task id we lookup is valid.

task.goal_id = goal_id

Choose a reason for hiding this comment

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

I personally sort of like this style of setting the relationship, since this is really what is happening even if we use the sqlalchemy helpers. However, what would happen if the goal previously had some tasks set. Do we want to add the new tasks to the existing tasks? Do we want to replace them and sever any prior task → goal relationships? What behavior is implemented here?


db.session.commit()
goal_response = {
"id": goal.goal_id,
"task_ids": task_ids
}
return make_response(jsonify(goal_response), 200)

if request.method == "GET":
tasks = Task.query.filter_by(goal_id=goal.goal_id)
tasks_response = []

for task in tasks:
if not task.completed_at:
is_complete = False
else:
is_complete = True
tasks_response.append({
"id": task.task_id,
"title": task.title,
"description": task.description,
"is_complete": is_complete,
"goal_id": task.goal_id,
})
goal_response = {
"id": goal.goal_id,
"title": goal.title,
"tasks": tasks_response
}
return make_response(jsonify(goal_response), 200)
Empty file added app/templates/__init__.py
Empty file.
Loading