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
9 changes: 9 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
from flask import Flask
from .db import db, migrate
from .models import task, goal
from .routes.task_routes import tasks_bp
from .routes.goal_routes import goals_bp
import os
from dotenv import load_dotenv

load_dotenv()
Comment on lines +7 to +9

Choose a reason for hiding this comment

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

It should not be necessary to add this explicit call to load_dotenv here. In all the places where we expect Flask to locate these settings in the .env file, Flask will do so properly (for instance, we don't need this call in deployment, since we load the values directly in the deployed environment).


def create_app(config=None):
app = Flask(__name__)
Expand All @@ -18,5 +23,9 @@ def create_app(config=None):
migrate.init_app(app, db)

# Register Blueprints here
app.register_blueprint(tasks_bp)
app.register_blueprint(goals_bp)

return app

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

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 set this value here, since it's not used elsewhere in the application.

18 changes: 17 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,21 @@
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import Mapped, mapped_column, relationship
from ..db import db

class Goal(db.Model):

Choose a reason for hiding this comment

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

👍 Your Goal implementation is consistent with your Task model. Check the Task feedback for anything that could apply here.

id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
title: Mapped[str]
tasks: Mapped[list["Task"]] = relationship(back_populates="goal")


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

@classmethod
def from_dict(cls, goal_data):
return cls(title=goal_data["title"])

from typing import TYPE_CHECKING
if TYPE_CHECKING:
from .task import Task
30 changes: 29 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,33 @@
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import Mapped, mapped_column, relationship
from ..db import db
from datetime import datetime
from sqlalchemy import ForeignKey
from typing import Optional

class Task(db.Model):
id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
title: Mapped[str]
description: Mapped[str]
completed_at: Mapped[datetime] = mapped_column (nullable=True)

Choose a reason for hiding this comment

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

Using the Mapped approach, it's preferred to add the information about a column being nullable to the Mapped type itself, usually by using the Optional modifier (as you did a little further down). This allows VS Code to perform better type hinting, as well as setting the nullability in the schema through migrations.

goal_id: Mapped[Optional[int]] = mapped_column (ForeignKey("goal.id"))
goal: Mapped[Optional["Goal"]] = relationship(back_populates="tasks")

def to_dict(self):
task_dict = {"id": self.id,

Choose a reason for hiding this comment

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

👍 Not including the outer task "envelope" wrapper in our to_dict keeps it more general-purpose. We can use it in endpoints that require the envelope by embedding this result in an outer dictionary, or use in other cases where the wrapper is not called for in the specifications.

"title": self.title,
"description": self.description,
"is_complete": bool(self.completed_at)

Choose a reason for hiding this comment

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

Even though we only use the calculated is_complete value when converting to a result dict, and even though it's a single line, it can be useful to move this logic to a well-named helper. This would make the dict building here more self-documenting, and provides a clear name to the operation being performed.

}
Comment on lines +16 to +20

Choose a reason for hiding this comment

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

When wrapping lines, we generally indent the lines that are wrapped. The closing } can align with the original line, but should not unindent further to the left, as this is confusing to read.

        task_dict = {
            "id": self.id,
            "title": self.title,
            "description": self.description,
            "is_complete": bool(self.completed_at)
        }


if self.goal:
task_dict["goal_id"] = self.goal_id
Comment on lines +22 to +23

Choose a reason for hiding this comment

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

👍 Nice logic to add the goal data to the task dictionary only when it's present.

return task_dict


@classmethod
def from_dict(cls, task_data):
goal_id = task_data.get("goal_id")
new_task = cls(title=task_data["title"], description=task_data["description"], completed_at=task_data.get("completed_at"), goal_id=goal_id)

Choose a reason for hiding this comment

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

👍 By reading title and description directly as keys we can trigger a KeyError if they are absent, giving us a way to indicate they are required.

return new_task


99 changes: 98 additions & 1 deletion app/routes/goal_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,98 @@
from flask import Blueprint
from flask import Blueprint, request, Response, abort, make_response
from .routes_utilities import validate_model, create_model, get_models_with_filters
from ..db import db
from app.models.goal import Goal
from app.models.task import Task


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

@goals_bp.get("")

Choose a reason for hiding this comment

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

👍 Your Goal CRUD routes are consistent with your Task CRUD routes. Check the Task feedback for anything that could apply here.

def get_all_goals():
# query = db.select(Goal)

# sort_param = request.args.get("sort")
# if sort_param == "desc":
# query = query.order_by(Goal.title.desc())

# if sort_param == "asc":
# query = query.order_by(Goal.title)

# goals = db.session.scalars(query)

# goal_response = [goal.to_dict() for goal in goals]

# return goal_response
return get_models_with_filters(Goal, request.args)


@goals_bp.get("/<id>")
def get_one_Goal(id):
goal = validate_model(Goal, id)
return {"goal":goal.to_dict()}

@goals_bp.put("/<id>")
def update_Goal(id):
goal = validate_model(Goal, id)
request_body = request.get_json()

goal.title = request_body["title"]

db.session.commit()
return Response(status=204, mimetype="application/json")


@goals_bp.delete("/<id>")
def delete_Goal(id):
goal = validate_model(Goal, id)

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

return Response(status=204, mimetype="application/json")

@goals_bp.post("")
def post_new_Goal():
request_body = request.get_json()

# try:
# new_goal = Goal.from_dict(request_body)
# except KeyError as error:
# response = {"details": "Invalid data"}
# abort(make_response(response, 400))

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

# return make_response({"goal": new_goal.to_dict()}, 201)
return create_model(Goal, request_body)

@goals_bp.post("/<goal_id>/tasks")
def create_tasks_with_goal(goal_id):
goal = validate_model(Goal, goal_id)
request_body = request.get_json()

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

task_ids = request_body["task_ids"]

goal.tasks.clear()

for task_id in task_ids:
task = validate_model(Task, task_id)
task.goal_id = goal.id
Comment on lines +80 to +84

Choose a reason for hiding this comment

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

Since we need to validate each Task id anyway, if we stored the resulting tasks in a list, the replacement of the tasks for the goal could be accomplished using the tasks property as

    goal.tasks = tasks

which avoids the need to manually clear the goal association from the existing tasks.


db.session.commit()

return {"id": goal.id, "task_ids": task_ids}, 200


@goals_bp.get("/<goal_id>/tasks")
def get_tasks_by_goal(goal_id):
goal = validate_model(Goal, goal_id)
response = {"id": goal.id, "title": goal.title, "tasks": [task.to_dict() for task in goal.tasks]}

Choose a reason for hiding this comment

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

Notice that the id and title keys here are the same as for a regular Goal GET, for which we wrote the to_dict helper. Here, we could use our to_dict to get the regular result dictionary, and then add in the tasks key with the loaded task data.

return response



41 changes: 41 additions & 0 deletions app/routes/routes_utilities.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
from ..db import db
from flask import abort, make_response

def validate_model(cls, id):
try:
id = int(id)
except:
abort(make_response({"message": f"{cls.__name__} id {id} is invalid."}, 400))

query = db.select(cls).where(cls.id == id)
model = db.session.scalar(query)
if not model:
abort(make_response({"message": f"{cls.__name__} id {id} is not found."}, 404))
return model

def create_model(cls, model_data):
try:
new_model = cls.from_dict(model_data)
except KeyError as error:
response = {"details": "Invalid data"}
abort(make_response(response, 400))

db.session.add(new_model)
db.session.commit()

return make_response({cls.__name__.lower(): new_model.to_dict()}, 201)

Choose a reason for hiding this comment

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

👍 Nice customization of create_model to account for the outer wrapper "envelope" in a way that works for either model type.




def get_models_with_filters(cls, filters=None):
query = db.select(cls)

if filters:
for attribute, value in filters.items():
if hasattr(cls, attribute):
query = query.where(getattr(cls, attribute).ilike(f"%{value}%"))

models = db.session.scalars(query.order_by(cls.id))
models_response = [model.to_dict() for model in models]
return models_response

108 changes: 107 additions & 1 deletion app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,107 @@
from flask import Blueprint
from flask import Blueprint, request, Response, abort, make_response
from .routes_utilities import validate_model, create_model, get_models_with_filters
from ..db import db
from app.models.task import Task
from datetime import datetime
import os
import requests

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

@tasks_bp.get("")
def get_all_tasks():
query = db.select(Task)

sort_param = request.args.get("sort")
if sort_param == "desc":
query = query.order_by(Task.title.desc())

if sort_param == "asc":
query = query.order_by(Task.title)

tasks = db.session.scalars(query)
Comment on lines +15 to +22

Choose a reason for hiding this comment

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

Rather than coding this sort logic directly into the route, we could take an approach similar to get_models_with_filters from the curriculum which would allow us to move the sort logic into a shared function. Since both Goals and Tasks have a title column, this would give us the ability to sort both Tasks and Goals by their titles!

Choose a reason for hiding this comment

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

If no sort is specified, it's still a good idea to sort on the record ids, as this will give us a consistent ordering if records are modified. Otherwise, records will be returned in the internal ordering used by a table, which could change from request to request.


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

Choose a reason for hiding this comment

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

👍 Nice use of a list comprehension to build the result response and convert the records to dicts all in a concise, readable structure.


return tasks_response

@tasks_bp.get("/<id>")
def get_one_task(id):
task = validate_model(Task, id)
return {"task":task.to_dict()}

@tasks_bp.put("/<id>")
def update_task(id):
task = validate_model(Task, id)
request_body = request.get_json()

task.title = request_body["title"]
task.description = request_body["description"]
Comment on lines +38 to +39

Choose a reason for hiding this comment

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

Notice how similar updating is to creating. After validating the record to update, we require certain keys to be present, others can be optional, then after updating and committing the model, we return a common response. How could we add model or route helpers to simplify our PUT route?


db.session.commit()
return Response(status=204, mimetype="application/json")

@tasks_bp.patch("/<id>/mark_complete")
def mark_completed(id):
task = validate_model(Task, id)

task.completed_at = datetime.now()

Choose a reason for hiding this comment

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

👀 You wrote a helper to send a Slack message, but you're not calling it.

db.session.commit()
Comment on lines +46 to +50

Choose a reason for hiding this comment

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

👍 Nice route to mark the task complete. We can use our validation helper to get the same behavior as the other id-based routes, leaving our route responsible only for updating the record with the current datetime, saving it, and generating the response.

One thing we might still consider is moving the actual update logic into a model helper so that the Task model class itself is responsible for "knowing" how to complete a Task.


send_slack_msg(task_title=task.title)

return Response(status=204, mimetype="application/json")

@tasks_bp.patch("/<id>/mark_incomplete")
def is_incompleted(id):
task = validate_model(Task, id)

task.completed_at = None

db.session.commit()
Comment on lines +58 to +62

Choose a reason for hiding this comment

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

👍 Nice route to mark the task incomplete. We can use our validation helper to get the same behavior as the other id-based routes, leaving our route responsible only for clearing the completion date, saving it, and generating the response.

One thing we might still consider is moving the actual update logic into a model helper so that the Task model class itself is responsible for "knowing" how to mark a Task incomplete.


return Response(status=204, mimetype="application/json")


@tasks_bp.delete("/<id>")
def delete_task(id):

Choose a reason for hiding this comment

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

Notice how similar deleting is to getting. After validating the record to delete, we delete and commit it, then return a common response. How could we add model or route helpers to simplify our DELETE route?

task = validate_model(Task, id)

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

return Response(status=204, mimetype="application/json")

@tasks_bp.post("")
def post_new_task():
request_body = request.get_json()
return create_model(Task, request_body)

Choose a reason for hiding this comment

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

👍 Great use of the route helpers to simplify our task routes!


# try:
# new_task = Task.from_dict(request_body)
# except KeyError as error:
# response = {"details": "Invalid data"}
# abort(make_response(response, 400))

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

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

def send_slack_msg(task_title):

Choose a reason for hiding this comment

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

This function encapsulates the responsibility of sending a completion notification about the provided task. Notice that we could make a further helper function that wraps the responsibility of sending a message to a specified channel. This function would then be responsible for the logic of building the messaging, and knowing what channel to use.

Even the logic to build the notification message based on the task could be in its own helper. Think about whether such a potential function would be a model method, or some other method to which we pass a Task.

👀 Notice that there's nothing currently calling this function, so no messages would be sent.

token = os.environ.get("SLACK_API_TOKEN")
url = "https://slack.com/api/chat.postMessage"

Choose a reason for hiding this comment

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

We could store the Slack API URL in the .env too. If for some reason, there was a change to the API endpoint, we could then update where the endpoint looked for it without needing to update the code itself.

headers = {
"Authorization": f"Bearer {token}",

Choose a reason for hiding this comment

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

👍 The Slack API documentation prefers passing the API key in the header rather than in the request body.

Since we're passing the body as JSON (by using the json= named parameter) Slack wouldn't even accept the token if passed in the body, so including it in the header allows our preferred style of API call, as well as following Slack's preferred approach.

"Content-Type": "application/json"
}
data = {
"channel": "task-notifications",
"text": f"Someone just completed the task {task_title}"
}

response = requests.post(url, headers=headers, json=data)

if response.status_code != 200 or not response.json().get("ok"):
print("Slack API error:", response.text)
Comment on lines +106 to +107

Choose a reason for hiding this comment

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

Nice check for whether the API call succeeded. Since the notification isn't core to the act of updating our task status, if an error occurs we wouldn't want the entire route to fail, so logging a message is something we can do.

In a full implementation, we probably wouldn't have the notification happen directly as part of the route. Instead, we would schedule a task to try to send the notification, potentially with retry logic, and logging. Note that Flask has its own logging subsystem that we would prefer to use over a regular print, but this is fine for now. The main thing is to still allow the update to complete.

1 change: 1 addition & 0 deletions migrations/README
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Single-database configuration for Flask.
50 changes: 50 additions & 0 deletions migrations/alembic.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# 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,flask_migrate

[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

[logger_flask_migrate]
level = INFO
handlers =
qualname = flask_migrate

[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