Skip to content

Submit Task List #121

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 17 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()'
10 changes: 8 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,13 @@ def create_app(test_config=None):
migrate.init_app(app, db)

# Register Blueprints here
from .routes.task_routes import task_bp
from .routes.goal_routes import bp

app.register_blueprint(task_bp)
app.register_blueprint(bp)

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

return app
18 changes: 16 additions & 2 deletions app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
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)
tasks = db.relationship("Task", back_populates="goal", lazy=True)

def to_dict_goal(self):
return {
"id": self.goal_id,
"title": self.title
}

@classmethod
def from_dict(cls, data_dict):
if "title" in data_dict:
new_obj = cls(title=data_dict["title"])

return new_obj
30 changes: 28 additions & 2 deletions app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
from app import db

from datetime import datetime

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)
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):
completed = True if self.completed_at else False

task_dict = {
"id": self.task_id,
"title": self.title,
"description": self.description,
"is_complete": completed,
}

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

return task_dict

@classmethod
def from_dict(cls, task_data):

return Task(title=task_data["title"], description=task_data["description"], completed_at=None)
1 change: 0 additions & 1 deletion app/routes.py

This file was deleted.

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


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

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

goals_list = [goal.to_dict_goal() for goal in goals]

return make_response(jsonify(goals_list), 200)

@bp.route("/<goal_id>", methods=["GET"])
def read_one_goal(goal_id):
goal = validate_model(Goal, goal_id)
if goal:
return {
"goal": goal.to_dict_goal()}, 200
else:
response_body = {
"details": "Invalid data"}
return make_response(jsonify(response_body))

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

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


new_goal = Goal.from_dict(request_body)

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

return jsonify({
"goal": new_goal.to_dict_goal()
}), 201


@bp.route("/<goal_id>", methods=["PUT"])
def update_goal(goal_id):
goal = validate_model(Goal, goal_id)
request_body = request.get_json()

request_body = request.get_json()

if "title" in request_body:

Choose a reason for hiding this comment

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

In your POST request for goals, instead of checking if "title" is in request_body, you check to see if "title" is absent from the request body. If it is absent, then you returned an error message to let the client know that the data was invalid.

That's the more appropriate logic for routes that use client data to create or update entities in the database. So on line 60, we should invert the logic to check if "title" is missing and if it is, you would return an error message.

If "title" is present, then you can update goal.title, commit the change, and return the updated goal

goal.title = request_body["title"]
db.session.commit()
response_body = {
"goal": goal.to_dict_goal()
}
Comment on lines +61 to +65

Choose a reason for hiding this comment

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

Watch your indentation here. Lines 62 and 63 will execute even if "title" is not in request_body since they not indented inside the body if the if statement.

return make_response(jsonify(response_body, 200))

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

Choose a reason for hiding this comment

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

No need to check if goal, the validate_model() method will return a valid goal if it doesn't return an error


goal_dict = {
"details": f"Goal {goal_id} \"{goal.title }\" successfully deleted"}
Comment on lines +73 to +74

Choose a reason for hiding this comment

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

It makes a little more sense to write lines 73 and 74 right before you pass goal_dict to jsonify() on line 78, just in terms of flow and readability.

Also, we are returning a dict but it is not a goal. It is a dict with a key "details" and then a string. It would be more appropriate to call this dict a response


db.session.delete(goal)
db.session.commit()
return jsonify(goal_dict), 200


@bp.route("/<goal_id>/tasks", methods=["GET"])
def goal_tasks(goal_id):
goal = validate_model(Goal, goal_id)

goal_dict = {
"id": goal.goal_id,
"title": goal.title,
"tasks": []
}

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

return jsonify(goal_dict), 200

@bp.route("/<goal_id>/tasks", methods=["POST"])
def sending_list_of_task_ids_to_goal(goal_id):
goal= validate_model(Goal, goal_id)
request_body = request.get_json()

goal.tasks = []

for task_id in request_body["task_ids"]:
task = validate_model(Task, task_id)
goal.tasks.append(task)

db.session.commit()

Choose a reason for hiding this comment

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

Notice the indentation of line 107, it's inside the for loop. This means that every time the for loop executes, db.session.commit() will execute too. We can let the loop execute adding validated tasks to goal.tasks and then outside the for loop, just run db.session.commit() just once


response = {
"id": goal.goal_id,
"task_ids": request_body["task_ids"]
}
return make_response(jsonify(response), 200)

@bp.route("/<goal_id>/tasks", methods=["GET"])

Choose a reason for hiding this comment

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

Flask won't be able to match this route because you have the exact same route declared on line 81. What is the purpose of this method? Your route above that has the method goal_tasks gets the tasks associated with a goal.

def getting_tasks_of_one_goal(goal_id):
goal=validate_model(Goal, goal_id)

goals_list = [goal.to_dict_goal() for goal in goal.tasks]
return jsonify(goals_list), 200
103 changes: 103 additions & 0 deletions app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
from flask import Blueprint, abort, jsonify, make_response, request
from app import db
from app.models.task import Task
import datetime
from app import os
from .validate_model import validate_model
import os
import requests

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

"""Wave 1"""

@task_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 {"details": "Invalid data"}, 400

new_task = Task.from_dict(request_body)
db.session.add(new_task)
db.session.commit()

return jsonify({
"task": new_task.to_dict()
}), 201

@task_bp.route("", methods=["GET"])
def read_all_tasks():
title_query = request.args.get("title")
sort_query = request.args.get("sort")

if sort_query == "asc":
tasks = Task.query.order_by(Task.title).all()
elif sort_query == "desc":
tasks = Task.query.order_by(Task.title.desc()).all()
elif title_query:
tasks = Task.query.get(title=title_query)
else:
tasks = Task.query.all()
Comment on lines +35 to +41

Choose a reason for hiding this comment

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

the body of these conditional statements are indented too far (lines 35, 37, 39, 41)


task_list = [task.to_dict() for task in tasks]

return make_response(jsonify(task_list), 200)


@task_bp.route("/<task_id>", methods=["GET"])
def read_one_task(task_id):
task = validate_model(Task, task_id)

return make_response(jsonify({"task":task.to_dict()}))
Comment on lines +50 to +52

Choose a reason for hiding this comment

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

Perfectly done.


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

task_dict = {
"details": f"Task {task_id} \"{task.title}\" successfully deleted"
}
Comment on lines +58 to +60

Choose a reason for hiding this comment

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

Would prefer this to get declared right before line 65 for the sake of the flow. You create it and then don't use it to delete from the database so it makes sense to create the dictionary later and then immediately return it to the client on line 65. Like I said above for your goal_routes, this dict doesn't represent an object (like task), it would be more appropriately named response


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

return jsonify(task_dict), 200

@task_bp.route("/<task_id>", methods=["PUT"])
def update_task(task_id):
task = 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

Choose a reason for hiding this comment

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

Need error handling to ensure that these two keys are in the request_body since this data comes from the client


response_body = {"task": task.to_dict()}
db.session.commit()
return jsonify(response_body), 200

"""WAVE 3 & 4"""
SLACK_TOKEN = os.environ.get('SLACK_TOKEN')
SLACK_URL = os.environ.get('SLACK_URL')
Comment on lines +80 to +81

Choose a reason for hiding this comment

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

Notice that these 2 constants are not contained in a method so they have global scope. Is that your intention?

We might consider "what methods actually need access to these 2 constants?" if they are only needed in a particular method, then it would be more appropriate to declare them inside that method to narrow their scope


@task_bp.route("/<task_id>/mark_incomplete", methods=["PATCH"])
def mark_task_incomplete(task_id):
task = validate_model(Task, task_id)
task.completed_at = None
db.session.commit()
return jsonify(task=task.to_dict()), 200

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

headers = {"Authorization":f"Bearer {SLACK_TOKEN}"}
data = {
"channel":"task-notifications",
"text": f"Someone just completed the task {task.title}."
}
res = requests.post(SLACK_URL, headers=headers, data=data)
Comment on lines +96 to +101

Choose a reason for hiding this comment

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

Nice work implementing Wave 4. Since these lines of code have a specific function not related to marking the task complete, it would make sense to pull this into a helper method and call that method here. Doing so would keep this route concise and constrain the responsibility of it too

db.session.commit()
return jsonify({"task":task.to_dict()}), 200
15 changes: 15 additions & 0 deletions app/routes/validate_model.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from flask import abort, make_response, jsonify

def validate_model(cls, model_id):

Choose a reason for hiding this comment

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

Nice job pulling this into a separate file and importing it in two different places. A suggestion for the file name is to call it helper.py

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:
response = f"{cls.__name__} #{model_id} was not found"
abort(make_response({"message": response}, 404))

return model
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