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()'
7 changes: 7 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,12 @@ 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


108 changes: 108 additions & 0 deletions app/goal_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
from flask import Blueprint, request, make_response, jsonify, abort
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work putting goal routes logic in a separate file from task routes logic. You can create a directory called routes (like we have with models) and put your two route files in it to further organize your project

from app.models.goal import Goal
from app.models.task import Task
from app import db


goals_bp = Blueprint("goals", __name__, url_prefix="/goals")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since this file is called goal_routes.py and the file is only concerned with the Goal class, we can rename this variable to just be bp


def get_validate_model(cls, model_id):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work refactoring this method so it is generic enough to validate any kind of model.

However, you have this method repeated in both route files and they do the same thing. It would be better to put this method in a helper file and import the method into your 2 route files

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


@goals_bp.route("", methods=["POST"])
def create_goal():
request_body = request.get_json()

if "title" not in request_body:
return make_response({
"details": "Invalid data"
}, 400)
Comment on lines +27 to +30
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice error handling. Whenever our methods rely on data sent by the client we should have error handling because we should anticipate the possibility of bad data being sent to our server.

Another way to handle invalid request body from the client is to use try/except.

try:
    # create your goal here
except:
    # return error message if the request body is invalid


new_goal = Goal(title=request_body["title"])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As we demonstrated during our roundtable, you can also create a class method in the Goal class to handle turning data from the request body into an instance of Goal.


db.session.add(new_goal) # track this object
db.session.commit() # any changes that are pending commit those changes as data written in SQL

new_goal_response = {"goal": new_goal.to_dict()}
return make_response(jsonify(new_goal_response), 201)


@goals_bp.route("", methods=["GET"])
def read_all_goals():
goals = Goal.query.all()

goals_list = [goal.to_dict() for goal in goals]
return make_response(jsonify(goals_list), 200)


@goals_bp.route("/<goal_id>", methods=["GET"])
def read_one_goal(goal_id):
goal = get_validate_model(Goal, goal_id)
current_goal_response = {"goal": goal.to_dict()}

return make_response(jsonify(current_goal_response), 200)


@goals_bp.route("/<goal_id>", methods=["PUT"])
def update_goal(goal_id):
goal = get_validate_model(Goal, goal_id)

request_body = request.get_json()
goal.title = request_body["title"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We need error handling when we handle UPDATE method (CRUD) just like you have error handline for CREATE method for POST request because you are relying on data passed to the server from a client.

If they pass you a request body like {"title_name": "updated title name"} then your method will throw an unhandled KeyError on line 62


db.session.commit()

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


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

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

return make_response({"details": f'Goal {goal_id} "{goal.title}" successfully deleted'}, 200)


@goals_bp.route("/<goal_id>/tasks", methods=["POST"])
def sending_list_of_task_ids_to_goal(goal_id):
goal = get_validate_model(Goal, goal_id)

request_body = request.get_json()
task_ids = request_body["task_ids"]

goal.tasks = [Task.query.get(task_id) for task_id in task_ids]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

query.get(task_id) does not handle invalid values for task_id. We can use the get_validate_model() method here to have error handling

goal.tasks = [task_id.get_validate_model() for task_id in task_ids]


db.session.commit() # any changes that are pending commit those changes as data written in SQL

return make_response(jsonify({
"id": goal.goal_id,
"task_ids": task_ids
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Instead of using task_ids variable which comes from request_body["task_ids"], we could get the task ids from goal itself.

You could create a helper method in goal.py to grab the task_ids from an instance of goal:

 def task_ids(self):
        return [task.id for task in self.tasks]

}), 200)


@goals_bp.route("/<goal_id>/tasks", methods=["GET"])
def getting_list_of_tasks_by_goal(goal_id):
goal = get_validate_model(Goal, goal_id)

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

return make_response(jsonify({
"id": goal.goal_id,
"title": goal.title,
"tasks": tasks_list
}), 200)
Comment on lines +104 to +107
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you use your to_dict() method from the Goal class here? You can add some logic to to_dict so that it can conditionally set "tasks" key when needed.


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

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

Choose a reason for hiding this comment

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

Since a goal without a title doesn't make much sense, we should add nullable=false to this attribute.

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

def to_dict(self):
result_dict = {}
result_dict["id"] = self.goal_id
result_dict["title"] = self.title

return result_dict
23 changes: 22 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,25 @@


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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add nullable=false so that you don't get task with "title": null

description = db.Column(db.String)
completed_at = db.Column(db.DateTime, nullable=True)

# Many tasks to one goal
goal_id = db.Column(db.Integer, db.ForeignKey('goal.goal_id'), nullable=True)
goal = db.relationship("Goal", back_populates="tasks")


def to_dict(self):
result_dict = {}
result_dict["id"] = self.task_id
result_dict["title"] = self.title
result_dict["description"] = self.description
result_dict["is_complete"] = bool(self.completed_at) # False if completed_at is null

if self.goal_id:
result_dict["goal_id"] = self.goal_id
Comment on lines +22 to +23
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍


return result_dict

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

This file was deleted.

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


tasks_bp = Blueprint("tasks", __name__, url_prefix="/tasks")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can rename this to bp


def get_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


@tasks_bp.route("", methods=["GET"])
def read_all_tasks():
order_by = request.args.get("sort")

if order_by == 'asc':
tasks = Task.query.order_by(asc('title')).all()
elif order_by == 'desc':
tasks = Task.query.order_by(desc('title')).all()
else:
tasks = Task.query.all()

tasks_list = [task.to_dict() for task in tasks]
return make_response(jsonify(tasks_list), 200)


@tasks_bp.route("/<task_id>", methods=["GET"])
def read_one_task(task_id):
task = get_validate_model(Task, task_id)
current_task_response = {"task": task.to_dict()}

return make_response(jsonify(current_task_response), 200)
Comment on lines +44 to +46
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Another way to write this would look like:

return make_response(jsonify(dict(task=task.to_dict())))



@tasks_bp.route("", methods=["POST"])
def create_task():
request_body = request.get_json()

if "title" not in request_body or "description" not in request_body:
return make_response({
"details": "Invalid data"
}, 400)

new_task = Task(title=request_body["title"],
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Line 58 is concerned with creating an instance of the Task class. You could argue that the logic in this method create_task() should only be concerned with creating a valid response to return to the client. Therefore, the logic on line 58 should be moved into the Task class so that the Task class is responsible for creating an instance of itself and we can just invoke the method here.

If you create a class method from_dict() in the Task class, then line 58 would just be:

new_task = Task.from_dict(request_body) 

That way we don't need to expose the logic that belongs in Task in a route

description=request_body["description"])

db.session.add(new_task) # track this object
db.session.commit() # any changes that are pending commit those changes as data written in SQL
current_task_response = {"task": new_task.to_dict()}
return make_response(jsonify(current_task_response), 201)


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

request_body = request.get_json()
task.title = request_body["title"]
task.description = request_body["description"]
Comment on lines +72 to +73
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Need some error handling here in case the request_body is invalide


db.session.commit()

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


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

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

return make_response({"details": f'Task {task_id} "{task.title}" successfully deleted'}, 200)


@tasks_bp.route("/<task_id>/mark_complete", methods=["PATCH"])
def update_incompleted_task_to_complete(task_id):
task = get_validate_model(Task, task_id)

task.completed_at = datetime.now()

db.session.commit()

slack_bot(task)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice job using a helper function to contain the logic for making a request to Slack's API. This makes your method more concise and readable

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


@tasks_bp.route("/<task_id>/mark_incomplete", methods=["PATCH"])
def update_completed_task_to_incomplete(task_id):
task = get_validate_model(Task, task_id)
task.completed_at = None

db.session.commit()

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


def slack_bot(task):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider renaming this method to incorporate a verb to clarify what this method is doing. Something like make_req_to_slack_bot would work

PATH = "https://slack.com/api/chat.postMessage"
SLACK_API_KEY = os.environ.get("SLACK_API")

bot_params = {
"channel": "task-notifications",
"text": f"Someone just completed the task {task.title}"
}

requests.post(PATH, data=bot_params, headers={"Authorization": SLACK_API_KEY})

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