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

Sharks - Sana Pournaghshband #95

Open
wants to merge 2 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()'
9 changes: 7 additions & 2 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,18 @@ def create_app(test_config=None):
app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
"SQLALCHEMY_TEST_DATABASE_URI")

# Import models here for Alembic setup
# Import models for Alembic setup
from app.models.task import Task
from app.models.goal import Goal

db.init_app(app)
migrate.init_app(app, db)

# Register Blueprints here
# Register Blueprints
from .task_routes import task_bp
from .goal_routes import goal_bp

app.register_blueprint(task_bp)
app.register_blueprint(goal_bp)

return app
Empty file added app/external/__init__.py
Empty file.
16 changes: 16 additions & 0 deletions app/external/slack.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import os
import requests

TOKEN = os.environ.get("SLACK_BOT_TOKEN")

def notify_completed(task_title):
data = {
"channel": "test-channel",
"text": f"Someone just completed the task {task_title}"
}

headers = {
"Authorization": f"Bearer {TOKEN}"
}

requests.post('https://slack.com/api/chat.postMessage', data = data, headers = headers)
129 changes: 129 additions & 0 deletions app/goal_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
from flask import Blueprint, jsonify, make_response, abort, request
from app.models.goal import Goal
from app.models.task import Task
from app import db

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

Choose a reason for hiding this comment

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

You can create a routes directory under the app directory and then put goal_routes.py and task_routes.py in it to keep your project structure organized


#Create goal
@goal_bp.route("", methods=["POST"])
def create_goal():
request_body = request.get_json()

try:
title = request_body['title']
except KeyError:
return abort(make_response({"details": 'Invalid data'}, 400))
Comment on lines +13 to +16

Choose a reason for hiding this comment

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

Nice error handling here! We can't assume that clients will send valid requests to our API so we have to anticipate a scenario where the req doesn't have the keys your method will need to access.


new_goal = Goal(
title = title,
)
Comment on lines +18 to +20

Choose a reason for hiding this comment

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

You could create a class method for Goal called create to keep your method concise. See this exmaple in Flasky: https://github.com/ada-c17/flasky/blob/sharks/app/models/cat.py#L35-L43


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

response = {
"goal": new_goal.to_dict()
}

return make_response(response, 201)


#Get all goals or no saved goals
@goal_bp.route("", methods=["GET"])
def get_goals():
goals = Goal.query.all()

goals_response = []
for goal in goals:
goals_response.append(goal.to_dict())
Comment on lines +37 to +39

Choose a reason for hiding this comment

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

This works well and is readable, but if you'd like to incorporate list comprehensions into your code, you could write it like this:

goals_response = [goal.to_json() for goal in goals]


return make_response(jsonify(goals_response), 200)

#Get One Goal: One Saved Goal
@goal_bp.route("/<goal_id>", methods=["GET"])
def read_one_goal(goal_id):
goal = Goal.query.get(goal_id)

if not goal:
return abort(make_response({"message": f"Goal {goal_id} is not found"}, 404))
Comment on lines +46 to +49

Choose a reason for hiding this comment

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

Consider creating a validate_goal() function and using it here so that you don't need to expose additional logic in your route method. This makes your method adhere more to the single responsibility principle and also makes it a bit more concise.


response = {
"goal": goal.to_dict()
}

return response

#Update one goal
@goal_bp.route("/<goal_id>", methods = ["PUT"])
def update_one_goals(goal_id):
goal = Goal.query.get(goal_id)

if not goal:
return abort(make_response({"message": f"Goal {goal_id} is not found"}, 404))
Comment on lines +60 to +63

Choose a reason for hiding this comment

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

Same comment as above about using a validation helper method


request_body = request.get_json()

goal.title = request_body['title']

Choose a reason for hiding this comment

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

Consider adding logic here to handle the scenario where request_body might not have "title" like you did for your POST route.


db.session.commit()

return make_response({"goal": goal.to_dict()}, 200)

#Delete Goal: Deleting a Goal
@goal_bp.route("/<goal_id>", methods = ["DELETE"])
def delete_one_goal(goal_id):
goal = Goal.query.get(goal_id)

if not goal:
return abort(make_response({"message": f"Goal {goal_id} is not found"}, 404))

db.session.delete(goal)
db.session.commit()

response = {
'details': f'Goal {goal.goal_id} "{goal.title}" successfully deleted'
}

return make_response(response, 200)

@goal_bp.route("/<goal_id>/tasks", methods = ["POST"])
def add_tasks(goal_id):
request_body = request.get_json()
task_ids = request_body['task_ids']

Choose a reason for hiding this comment

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

Error handling here would make the method more robust against possible errors, like if request_body doesn't have "task_ids".


for task_id in task_ids:
task = Task.query.get(task_id)
if not task:
return abort(make_response({"message": f"Task {task_id} is not found"}, 404))

task.goal_id = goal_id

db.session.commit()

response_body = {
"id": int(goal_id),
"task_ids": task_ids
}

return make_response(response_body, 200)

@goal_bp.route("/<goal_id>/tasks", methods=["GET"])
def read_tasks(goal_id):
goal = Goal.query.get(goal_id)

if not goal:
return abort(make_response({"message": f"Goal {goal_id} is not found"}, 404))


tasks = []
for task in goal.tasks:
tasks.append(task.to_dict())

response_body = {
"id": int(goal_id),
"title": goal.title,
"tasks": tasks
}

return make_response(response_body, 200)
Comment on lines +123 to +129

Choose a reason for hiding this comment

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

You could call your Goal instance method to_dict() directly in line 129 and then remove lines 123-126.

Reusing your to_dict() function would help keep this method short and sweet. You can also update to_dict() to handle setting tasks in the dictionary too instead of doing it in the route.

For example:

    def to_dict(self):
        goal_dict = {
                        "id": self.id,
                        "title": self.title
                        }
        if self.tasks:
                   goal_dict["tasks"] = self.tasks
        return goal_dict

So line 129 would look like this:

return make_response(goal.to_dict(), 200)

You could also remove lines 119-121 since this different approach wouldn't need the tasks variable

11 changes: 9 additions & 2 deletions app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
from app import db


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, nullable=False)

Choose a reason for hiding this comment

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

Nice work using nullable=False here

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

def to_dict(self):
return {
"id": self.goal_id,
"title": self.title,
}
24 changes: 22 additions & 2 deletions app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
from app import db


'''
Task is a todo list task
'''
class Task(db.Model):
task_id = db.Column(db.Integer, primary_key=True)
task_id = db.Column(db.Integer, primary_key=True, autoincrement=True)
title = db.Column(db.String, nullable=False)

Choose a reason for hiding this comment

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

Nice work using nullable=False here

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

def to_dict(self):
d = {

Choose a reason for hiding this comment

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

Consider using a more descriptive name for this variable, something like task_dict

"id": self.task_id,
"title": self.title,
"description": self.description,
"is_complete": True if self.completed_at else False
}

if self.goal_id:
d['goal_id'] = self.goal_id

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

This file was deleted.

134 changes: 134 additions & 0 deletions app/task_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
from flask import Blueprint, jsonify, make_response, abort, request
from app.models.task import Task
from app import db
from datetime import datetime
from app.external import slack

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

#Create task
@task_bp.route("", methods=["POST"])
def create_task():
request_body = request.get_json()

try:
title = request_body['title']
description = request_body['description']
except KeyError:
return abort(make_response({"details": 'Invalid data'}, 400))

new_task = Task(
title = title,
description = description,
completed_at = request_body.get("completed_at")
)
Comment on lines +20 to +24

Choose a reason for hiding this comment

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

Consider also writing a create() class method for Task too so that you would only need to write new_task = create(request_body) here


db.session.add(new_task)
db.session.commit()

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

return make_response(response, 201)


#Get all tasks or no saved tasks
@task_bp.route("", methods=["GET"])
def get_tasks():

sort_query = request.args.get("sort")
if sort_query == "asc":
tasks = Task.query.order_by(Task.title.asc()).all()
elif sort_query == "desc":
tasks = Task.query.order_by(Task.title.desc()).all()
Comment on lines +41 to +44

Choose a reason for hiding this comment

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

with order_by() you don't need to use chain .all() to the end of this statement.

else:
tasks = Task.query.all()


tasks_response = []
for task in tasks:
tasks_response.append(task.to_dict())

return make_response(jsonify(tasks_response), 200)

#Get One Task: One Saved Task
@task_bp.route("/<task_id>", methods=["GET"])
def read_one_task(task_id):
task = Task.query.get(task_id)

if not task:
return abort(make_response({"message": f"Task {task_id} is not found"}, 404))

response = {
"task": task.to_dict()
}

return response

#Update one task
@task_bp.route("/<task_id>", methods = ["PUT"])
def update_one_tasks(task_id):
task = Task.query.get(task_id)

if not task:
return abort(make_response({"message": f"Task {task_id} is not found"}, 404))

request_body = request.get_json()

task.title = request_body['title']
task.description = request_body['description']
Comment on lines +77 to +80

Choose a reason for hiding this comment

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

In addition to error handling in POST requests, PUT requests would also be more robust with error handing to make sure we don't have unhandled KeyErrors


if 'completed_at' in request_body:
task.completed_at = request_body['completed_at']

db.session.commit()

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

#Delete Task: Deleting a Task
@task_bp.route("/<task_id>", methods = ["DELETE"])
def delete_one_task(task_id):
task = Task.query.get(task_id)

if not task:
return abort(make_response({"message": f"Task {task_id} is not found"}, 404))

db.session.delete(task)
db.session.commit()

response = {
'details': f'Task {task.task_id} "{task.title}" successfully deleted'
}

return make_response(response, 200)

@task_bp.route("/<task_id>/mark_complete", methods = ["PATCH"])
def mark_complete(task_id):
task = Task.query.get(task_id)

if not task:
return abort(make_response({"message": f"Task {task_id} is not found"}, 404))
Comment on lines +108 to +111

Choose a reason for hiding this comment

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

When you start to see repeated code or repeated patterns of code, it's usually a good indicator that a helper function can be created and used


task.completed_at = datetime.now()
db.session.commit()

slack.notify_completed(task.title)

Choose a reason for hiding this comment

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

👍 having this imported helper method keeps your method nice and short


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

@task_bp.route("/<task_id>/mark_incomplete", methods = ["PATCH"])
def mark_incomplete(task_id):
task = Task.query.get(task_id)

if not task:
return abort(make_response({"message": f"Task {task_id} is not found"}, 404))

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

return make_response({
"task": task.to_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.
Loading