Skip to content
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

Pine - Mac P. #71

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
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()'
6 changes: 4 additions & 2 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@
import os
from dotenv import load_dotenv


db = SQLAlchemy()
migrate = Migrate()
load_dotenv()


def create_app(test_config=None):
app = Flask(__name__)
app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False
Expand All @@ -30,5 +28,9 @@ def create_app(test_config=None):
migrate.init_app(app, db)

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

return app
87 changes: 87 additions & 0 deletions app/goal_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
from flask import Blueprint, jsonify, abort, request
from app import db
from app.models.goal import Goal
from app.models.task import Task
from dotenv import load_dotenv

load_dotenv()
Copy link

Choose a reason for hiding this comment

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

We don't get the value of any environment variables in this file, so we don't need this line.


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

def valid_id(model, id):
"""If ID is an int, returns the model object with that ID.
If ID is not an int, returns 400.
If model object with that ID doesn't exist, returns 404."""
try:
id = int(id)
except:
abort(400, {"error": "invalid id"})
return model.query.get_or_404(id)
Comment on lines +11 to +19
Copy link

Choose a reason for hiding this comment

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

Very cool method with great docstring! However, it's duplicated across both of your routes files. Consider adding an additional file, perhaps named validation.py where this function can live. Then each of your routes files can do from validation import valid_id.


@goals_bp.route("", methods=["POST", "GET"])
def handle_goals():
"""Handles post and get requests for /goals"""
Copy link

Choose a reason for hiding this comment

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

Nice docstring here and throughout all your code

request_body = request.get_json()

if request.method == "POST":
if "title" not in request_body:
return {"details": "Invalid data"}, 400

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

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

goals_dict = {}
goals_dict["goal"] = new_goal.to_dict()
return goals_dict, 201

if request.method == "GET":
goals = Goal.query.all()
goals_response = [goal.to_dict() for goal in goals]
Copy link

Choose a reason for hiding this comment

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

Nice list comprehension!


return jsonify(goals_response), 200

@goals_bp.route("/<goal_id>", methods=["GET", "PUT", "DELETE"])
def handle_one_goal(goal_id):
"""Handles get, put, and delete requests for one goal \
with a given id at goals/<goal_id>"""
goal_dict = {}
goal = valid_id(Goal, goal_id)
Copy link

Choose a reason for hiding this comment

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

Good use of your validator


if request.method == "GET":
goal_dict["goal"] = goal.to_dict()
return goal_dict, 200

if request.method == "PUT":
request_body = request.get_json()
goal.title = request_body["title"]
goal_dict["goal"] = goal.to_dict()
db.session.commit()
return goal_dict, 200

if request.method == "DELETE":
if goal:
Copy link

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 check here if we have a goal. The call to valid_id on line 50 will throw a 404 if the goal doesn't exist, so we're guaranteed to have something valid if we make it down here.

db.session.delete(goal)
db.session.commit()
return {"details": f"Goal {goal.goal_id} \"{goal.title}\" successfully deleted"}, 200
Copy link

Choose a reason for hiding this comment

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

Nice use of escaping to include quote literals in your string.


@goals_bp.route("/<goal_id>/tasks", methods=["POST", "GET"])
def handle_tasks_realted_to_goal(goal_id):
"""Handles post and get requets for a goal with a given ID \
and all tasks related to it at /goals/<goal_id>/tasks"""
goal = valid_id(Goal, goal_id)

if request.method == "POST":
request_body = request.get_json()
goal.tasks = [valid_id(Task, task_id) for task_id in request_body["task_ids"]]
Copy link

Choose a reason for hiding this comment

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

Gorgeous!


db.session.commit()

return {"id": int(goal_id), "task_ids": request_body["task_ids"]}, 200
Copy link

Choose a reason for hiding this comment

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

Nice job re-using the task ids from the request instead of having to remake them yourself.


if request.method == "GET":
goal_dict = goal.to_dict()
goal_dict["tasks"] = [valid_id(Task, task.task_id).to_dict() for task in goal.tasks]

return goal_dict, 200
12 changes: 10 additions & 2 deletions app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
from flask import current_app
from app import db


class Goal(db.Model):
""""Database model with goal_id, title, and a backref relationship to tasks"""
goal_id = db.Column(db.Integer, primary_key=True)
title = db.Column(db.Text, nullable=False)
tasks = db.relationship("Task", backref="goal", lazy=True)
Copy link

Choose a reason for hiding this comment

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

👍🏻


def to_dict(self):
"""Returns a dictionary with Goal ID and title."""
return({
"id": self.goal_id,
"title": self.title
})
36 changes: 35 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,40 @@
from flask import current_app
from app import db


class Task(db.Model):
"""Database model with task_id, title, description, \
data and time for completion, \
and relationship with the one goal it is connected to"""
Comment on lines +5 to +7
Copy link

Choose a reason for hiding this comment

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

Nice docstring! However, when you're doing triple quotes, you don't need to include backslashes for multi-line strings.

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)

def to_dict(self):
"""Returns a dictionary with Task id, title, description, T/F is_complete,\
and goal_ids IF goal_ids are present."""
is_complete = True
if not self.completed_at:
is_complete = False

if not self.goal_id:
return({
"id": self.task_id,
"title": self.title,
"description": self.description,
"is_complete": is_complete
})

else:
return({
"id": self.task_id,
"title": self.title,
"description": self.description,
"is_complete": is_complete,
"goal_id": self.goal_id
})
Comment on lines +21 to +36
Copy link

Choose a reason for hiding this comment

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

Most of this if/else block is the same between the two conditions. Can you think of a way to condense your code here so that most of the logic is shared, and the if/else only handles the difference between the goal_id being included or not?





2 changes: 0 additions & 2 deletions app/routes.py

This file was deleted.

119 changes: 119 additions & 0 deletions app/task_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
from flask import Blueprint, jsonify, abort, request
from app import db
from app.models.task import Task
from datetime import datetime
import requests
import os
from dotenv import load_dotenv

load_dotenv()

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

def valid_id(model, id):
"""If ID is an int, returns the model object with that ID.
If ID is not an int, returns 400.
If model object with that ID doesn't exist, returns 404."""
try:
id = int(id)
except:
abort(400, {"error": "invalid id"})
return model.query.get_or_404(id)

@tasks_bp.route("", methods=["POST", "GET"])
def handle_tasks():
"""Handles post and get requests for tasks at /tasks"""
request_body = request.get_json()

if request.method == "POST":
if "title" not in request_body or "description" not in request_body \
or "completed_at" not in request_body:
return {"details": "Invalid data"}, 400

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()

tasks_dict = {}
tasks_dict["task"] = new_task.to_dict()
Comment on lines +40 to +41
Copy link

Choose a reason for hiding this comment

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

You could consider combining these two lines into one:

tasks_dict = {
    "task": new_task.to_dict()
}

return tasks_dict, 201

if request.method == "GET":
tasks_response = []
if request.args.get("sort") == "asc":
tasks = Task.query.order_by(Task.title)
elif request.args.get("sort") == "desc":
tasks = Task.query.order_by(Task.title.desc())
else:
tasks = Task.query.all()
for task in tasks:
tasks_response.append(task.to_dict())
Comment on lines +52 to +53
Copy link

Choose a reason for hiding this comment

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

This definitely works well! Can you think of what this would look like as a list comprehension instead. Either is good, it's just a matter of personal preference~


return jsonify(tasks_response), 200

@tasks_bp.route("/<task_id>", methods=["GET", "PUT", "DELETE"])
def handle_one_task(task_id):
"""Handles get, put, and delete requests \
for one task with a given id at /tasks/<task_id>"""
task_dict = {}
task = valid_id(Task, task_id)

if request.method == "GET":
task_dict["task"] = task.to_dict()
return task_dict, 200

if request.method == "PUT":
input_data = request.get_json()
task.title = input_data["title"]
task.description = input_data["description"]
if task.completed_at:
task.completed_at = input_data["completed_at"]
task_dict["task"] = task.to_dict()
db.session.commit()
return task_dict, 200

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

def post_task_completion_to_slack(task):
"""Posts a message to slack stating that someone completed a task with the task title"""
SLACK_BOT_TOKEN = os.environ.get("SLACK_BOT_TOKEN")
SLACK_CHANNEL = os.environ.get("SLACK_CHANNEL")
Comment on lines +86 to +87
Copy link

Choose a reason for hiding this comment

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

Great variable names! And I really like that the channel is configurable via environment variables as well.

You might consider putting the os.environ.gets somewhere else. Right now, your code re-reads the env vars every time a task is completed. It might be better to read these variables just once when the app starts up, similar to what happens for the DB connection string in app/init.py

url = "https://slack.com/api/chat.postMessage"
message = f"Somone just completed task {task.title}"
query_params = {
"channel": SLACK_CHANNEL,
"text": message
}
headers = {
"Authorization": f"Bearer {SLACK_BOT_TOKEN}"
}
# response variable left for debugging purposes
response = requests.post(url, data=query_params, headers=headers)
Copy link

Choose a reason for hiding this comment

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

Nice post! An extension that we could do if this were a full production application would be to check the response code that we get back. If it's some error code, we could raise some exception for our application to handle.



@tasks_bp.route("/<task_id>/<completion_status>", methods=["PATCH"])
def mark_completion_status(task_id, completion_status):
"""Returns task_dict and changes value of task.completed_at. \
If user goes to /tasks/<task_id>/mark_complete, \
task.completed_at has a value of current date and time \
and a message is posted to slack.
If user goes to /tasks/<task_id>/mark_incomplete, \
task.completed_at has a value of None"""
task_dict = {}
task = valid_id(Task, task_id)

if completion_status == "mark_complete":
task.completed_at = datetime.date
post_task_completion_to_slack(task)
elif completion_status == "mark_incomplete":
task.completed_at = None

task_dict["task"] = task.to_dict()
return task_dict, 200
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