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
4 changes: 4 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
from .db import db, migrate
from .models import task, goal
import os
from .routes.task_routes import tasks_bp
from .routes.goal_routes import goals_bp

def create_app(config=None):
app = Flask(__name__)
Expand All @@ -18,5 +20,7 @@ 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
16 changes: 15 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import Mapped, mapped_column, relationship
from ..db import db
# from .task import Task

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):
goal_as_dict = {}
goal_as_dict["id"] = self.id
goal_as_dict["title"] = self.title
return goal_as_dict

@classmethod
def from_dict(cls, goal_data):
new_goal = cls(title=goal_data["title"])
return new_goal
31 changes: 30 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,34 @@
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import Mapped, mapped_column, relationship
from sqlalchemy import ForeignKey
from typing import Optional
from ..db import db
from datetime import datetime
# from .goal import Goal

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(default=None, 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_as_dict = {
"id": self.id,
"title": self.title,
"description": self.description,
"is_complete": True if self.completed_at else False

Choose a reason for hiding this comment

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

👍 Yes, we can compute this from completed_at. I'm not sure off the top of my head what might be considered a falsy datetime read from the DB might be, but to be a bit more focused, we could do something like

            "is_complete": self.completed_at is not None

We could also consider moving the calculation to a helper function—even though it's only a single expression—to make it clear how we're expecting to interpret this result by giving it a clear function name

}
if self.completed_at:
task_as_dict["completed_at"] = self.completed_at
Comment on lines +23 to +24

Choose a reason for hiding this comment

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

This wasn't asked for in the output, but ends up not getting in the way of any tests.

We didn't have any tests sending datatime information since there's no standard JSON datetime representation. Working with is_complete and toggling completion through the dedicated endpoints avoided the need to do so!

if self.goal_id:
task_as_dict["goal_id"] = self.goal_id
Comment on lines +25 to +26

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_as_dict


@classmethod
def from_dict(cls, task_data):
new_task = cls(title=task_data["title"],
description=task_data["description"])
Comment on lines +32 to +33

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.

Choose a reason for hiding this comment

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

Though none of our tests attempt to pass completed_at, this is something we should still consider handling. The main reason we didn't provide an example is because we couldn't be certain how folks would represent the completed at (string or datetime) nor could we be sure what datetime format they would be expecting (there's no official standard for passing datetime data in JSON). This is also why we simplified the Task response representation to use is_complete rather than returning the actual datetime information.

However, once we have settled on our own method of representing completed_at, we can also allow the caller to pass appropriately formatted data, reading it from the data dictionary in a way that would let it remain optional

return new_task
112 changes: 111 additions & 1 deletion app/routes/goal_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,111 @@
from flask import Blueprint
from flask import Blueprint, request, abort, make_response, Response
from app.models.goal import Goal
from ..db import db
# from app.routes.task_routes import validate_task
from .route_utilities import validate_model, create_model
from app.models.task import Task



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

@goals_bp.post("")
def create_tasks():

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.

request_body = request.get_json()
return {"goal": create_model(Goal, request_body)[0]}, 201
# try:
# new_goal = Goal.from_dict(request_body)
# except KeyError as error:
# response_body = {"details": "Invalid data"}
# abort(make_response(response_body, 400))

# db.session.add(new_goal)
# db.session.commit()
# response = {"goal": new_goal.to_dict()}
# return response, 201

@goals_bp.get("")
def get_all_goals():
query = db.select(Goal)

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

# tasks = db.session.scalars(query.order_by(Task.id))
goals = db.session.execute(query).scalars()

goals_response = []
for goal in goals:
goals_response.append(goal.to_dict())
return goals_response



@goals_bp.get("/<goal_id>")
def get_one_goal(goal_id):
# goal = validate_goal(goal_id)
goal = validate_model(Goal, goal_id)
response = {"goal": goal.to_dict()}

return response



# def validate_goal(goal_id):
# query = db.select(Goal).where(Goal.id == goal_id)
# goal = db.session.scalar(query)

# if not goal:
# response = {"message": f"Goal {goal_id} not found"}
# abort(make_response(response, 404))

# return goal

@goals_bp.put("/<goal_id>")
def update_goal(goal_id):
# goal = validate_goal(goal_id)
goal = validate_model(Goal, goal_id)
request_body = request.get_json()
goal.title = request_body["title"]
goal.id = request_body["id"]
db.session.commit()
return Response(status=204, mimetype="application/json")


@goals_bp.delete("/<goal_id>")
def delete_goal(goal_id):
# goal = validate_goal(goal_id)
goal = validate_model(Goal, goal_id)
db.session.delete(goal)
db.session.commit()
return Response(status=204, mimetype="application/json")

@goals_bp.get("/<goal_id>/tasks")
def get_tasks_by_goal(goal_id):
# goal = validate_goal(goal_id)
goal = validate_model(Goal, goal_id)
response = goal.to_dict()
response["tasks"] = [task.to_dict() for task in goal.tasks]
Comment on lines +90 to +91

Choose a reason for hiding this comment

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

👍 Nice reuse of your to_dict methods to build up your response. Rather than leaving this logic here in the route, we could move it to an additional Goal method. Perhaps to_dict_with_tasks, or we could add a parameter to the main Goal to_dict that determines whether or not to include the Task details.

return response

@goals_bp.post("/<goal_id>/tasks")
def send_tasks_to_goals(goal_id):
# goal = validate_goal(goal_id)
goal = validate_model(Goal, goal_id)

request_body = request.get_json()
task_list = request_body["task_ids"]
goal.tasks.clear() #removes existing tasks assoc w the goal

for task_id in request_body["task_ids"]:
new_task = validate_model(Task, task_id)
new_task.goal_id = goal_id

db.session.commit()
response = {"id": goal.id,
"task_ids": request_body["task_ids"]}

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

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

query = db.select(cls).where(cls.id == model_id)
model = db.session.scalar(query)

if not model:
response = {"message": f"{cls.__name__} {model_id} not found"}
abort(make_response(response, 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 new_model.to_dict(), 201
98 changes: 97 additions & 1 deletion app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,97 @@
from flask import Blueprint
from flask import Blueprint, request, abort, make_response, Response
from app.models.task import Task
from ..db import db
from datetime import datetime
from .route_utilities import validate_model, create_model
import os
import requests



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

@tasks_bp.post("")
def create_tasks():
request_body = request.get_json()
return {"task": create_model(Task, request_body)[0]}, 201

Choose a reason for hiding this comment

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

Extracting the model dictionary from the packed response can be done in this way, but alternatively, we could modify the internal behavior of create_model to generate a response more in line with the project specification. There's nothing special about the return value shown in the curriculum version, and so if we're working in a situation where a different output format is needed, we should feel comfortable adapting the behavior to our needs.



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

return response


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

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

Choose a reason for hiding this comment

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

👍 Nice gradual build-up of the query instance to allow us to add in sorting if present. This approach would allow for easier inclusion of other features like filtering or paging later on.

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

# tasks = db.session.scalars(query.order_by(Task.id))

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 = db.session.execute(query).scalars()

tasks_response = []
for task in tasks:
tasks_response.append(task.to_dict())
Comment on lines +40 to +42

Choose a reason for hiding this comment

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

This pattern (empty result, iterate, process each record, append) can be expressed cleanly in a list comprehension.

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

return tasks_response


@tasks_bp.put("/<task_id>")
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"]
db.session.commit()
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.

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?

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

@tasks_bp.delete("/<task_id>")
def delete_task(task_id):
task = validate_model(Task, task_id)
db.session.delete(task)
db.session.commit()
Comment on lines +57 to +59

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?

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


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

send_msg(task)

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

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


@tasks_bp.patch("/<task_id>/mark_incomplete")
def mark_incomplete(task_id):
task = validate_model(Task, task_id)
task.completed_at = None

Choose a reason for hiding this comment

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

👀 Currently, your code assumes that a task will have an is_complete attribute that is used to return the completed status when building a response dict. But notice here that if we mark a task incomplete, the is_complete attribute is not also set back to False. My overall recommendation is still to rework the model logic so that we no longer need a literal is_complete column, but if we are using that column, it should be used consistently. Currently, this will leave a task with is_complete being True, but the completed_at being empty, leaving the values out of sync (which is a danger of having both values).


db.session.commit()
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.

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


def send_msg(task):

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.

channel_id = os.environ.get("TESTER_CHANNEL")
token = os.environ.get("SLACK_BOT_TOKEN")
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.

passing the API key in the header rather than in the request body.

Notice that later, the use of the data= object will try to send the request body as x-www-form-urlencoded data rather than as JSON (overriding the Content-Type header). In fact, choosing to use the data= or json= argument on requests.post will automatically set the appropriate header. Setting the key in the header will work with either encoding style, but if we had sent the request with a JSON body (preferred) then it would be required that we send the key

"Content-Type": "application/json"
}
data = {
"channel": channel_id,
"text": f"Someone just completed the task {task.title}"
}
response = requests.post("https://slack.com/api/chat.postMessage", headers=headers, data=data)

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.

Choose a reason for hiding this comment

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

Prefer to use json= rather than data=.

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