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: 8 additions & 1 deletion app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
from .models import task, goal
from flask import Flask
from .db import db, migrate
from .models import task, goal
import os
from app.routes.task_routes import bp as task_bp
from app.routes.goal_routes import bp as goal_bp


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

# Register Blueprints here
from app.routes.task_routes import bp as task_bp
from app.routes.goal_routes import bp as goal_bp
Comment on lines +24 to +25

Choose a reason for hiding this comment

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

Nesting imports under code rather than having it at the top of a file can occasionally be necessary to avoid import cycles. However, due to how we've set up our imports (specifically how we access db), these imports can safely be listed at the top of the file.

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

return app
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] = mapped_column(db.String, nullable=False)
tasks: Mapped[list["Task"]] = relationship(back_populates="goal")


Choose a reason for hiding this comment

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

👀 Using back_populates, we would need to add another Mapped record here for the tasks.

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

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


38 changes: 37 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,41 @@
from sqlalchemy.orm import Mapped, mapped_column
from ..db import db
from datetime import datetime
from typing import Optional
from sqlalchemy.orm import Mapped, mapped_column, relationship



class Task(db.Model):
id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
title: Mapped[str] = mapped_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.

Using the Mapped approach, columns aren't nullable by default. We can leave off the nullable=False on columns for which we want to require a value. Additionally, the type of the column can be inferred by Alembic from the Mapped type, so we don't need to explicitly set the type in the mapped_column. If we then edn up with no options being passed to mapped_column, the entire mapped_column can be omitted!

description: Mapped[str] = mapped_column(db.String)
completed_at: Mapped[Optional[datetime]] = mapped_column(db.DateTime)
goal_id: Mapped[Optional[int]] = mapped_column(db.ForeignKey("goal.id"))
goal: Mapped[Optional["Goal"]] = relationship(back_populates="tasks")



def to_dict(self):
task_dict = {

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.

"id": self.id,
"title": self.title,
"description": self.description,
"is_complete": self.is_complete
}
if self.goal_id:
task_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_dict



@classmethod
def from_dict(cls, data: dict):
return cls(
title=data["title"],
description=data["description"],
Comment on lines +34 to +35

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.

completed_at=None

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

)

@property
def is_complete(self):
return self.completed_at is not None
96 changes: 95 additions & 1 deletion app/routes/goal_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,95 @@
from flask import Blueprint
from flask import Blueprint, request, jsonify, Response
from app.models.goal import Goal
from ..db import db
from app.models.task import Task

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

@bp.post("/<goal_id>/tasks")
def assign_tasks_to_goal(goal_id):
goal = db.session.get(Goal, goal_id)
if not goal:
return jsonify({"message": f"No goal with ID {goal_id} found"}), 404

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

goal.tasks = []
for task_id in task_ids:
task = db.session.get(Task, task_id)
if task:
goal.tasks.append(task)

db.session.commit()

return jsonify({
"id": goal.id,
"task_ids": task_ids
}), 200

@bp.get("/<goal_id>/tasks")
def get_tasks_for_goal(goal_id):
goal = db.session.get(Goal, goal_id)
if not goal:
return jsonify({"message": f"No goal with ID {goal_id} found"}), 404

goal_dict = goal.to_dict()
goal_dict["tasks"] = [task.to_dict() for task in goal.tasks]
Comment on lines +36 to +37

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 jsonify(goal_dict), 200

# CREATE a goal
@bp.post("")
def create_goal():

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.

data = request.get_json()

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

new_goal = Goal.from_dict(data)
db.session.add(new_goal)
db.session.commit()

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

# GET all goals
@bp.get("")
def get_goals():
goals = db.session.scalars(db.select(Goal)).all()
return jsonify([goal.to_dict() for goal in goals])

# GET one goal
@bp.get("/<goal_id>")
def get_one_goal(goal_id):
goal = db.session.get(Goal, goal_id)
if not goal:
return jsonify({"message": f"No goal with ID {goal_id} found"}), 404

return jsonify({"goal": goal.to_dict()})

# UPDATE a goal
@bp.put("/<goal_id>")
def update_goal(goal_id):
goal = db.session.get(Goal, goal_id)
if not goal:
return jsonify({"message": f"No goal with ID {goal_id} found"}), 404

data = request.get_json()
if "title" not in data:
return jsonify({"details": "Invalid data"}), 400

goal.title = data["title"]
db.session.commit()
return jsonify({"goal": goal.to_dict()})

# DELETE a goal
@bp.delete("/<goal_id>")
def delete_goal(goal_id):
goal = db.session.get(Goal, goal_id)
if not goal:
return jsonify({"message": f"No goal with ID {goal_id} found"}), 404

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

144 changes: 143 additions & 1 deletion app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,143 @@
from flask import Blueprint
from flask import Blueprint, request, jsonify, Response
from app.models.task import Task
from ..db import db
from datetime import datetime, UTC
from app.slack_helper import post_to_slack
#from dotenv import load_dotenv


import os
import requests
#load_dotenv()



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

# Helper: safely get a task by ID or return a 404 JSON response
def get_task_or_abort(task_id):

Choose a reason for hiding this comment

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

👀 While similar to the validate_model shown in the curriculum, this version doesn't take advantage of the abort method to halt processing a request for an invalid record. This results in additional error handling needing to be used everywhere this helper gets used.

While the exact helpers shown in the curriculum aren't the only way to approach building routes, we should be sure that we understand the pros and cons of including the functionality we did, and ensure we address similar concerns in our code.

try:
task_id = int(task_id)
except ValueError:
return jsonify({"message": "Invalid ID"}), 400

task = db.session.get(Task, task_id)

Choose a reason for hiding this comment

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

👍 Nice use of this helper to look up by the primary key of the Task model type.

I hadn't noticed that this is where this method got moved. It used to live on a query attribute that was attached directly to the model types, but it was deprecated when the query attribute was itself deprecated. I like using this rather than the fully written out query we have in our version.

if not task:
return jsonify({"message": f"No task with ID {task_id} found"}), 404

return task

# CREATE: POST /tasks
@bp.post("")
def create_task():
data = request.get_json()

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

new_task = Task.from_dict(data)
db.session.add(new_task)
db.session.commit()
Comment on lines +33 to +40

Choose a reason for hiding this comment

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

🔎 We could have adapted create_model from the curriculum to be used here and for the Goal create routes. This would allow us to avoid duplicating most of the logic and error handling.


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

Choose a reason for hiding this comment

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

👀 Calls to jsonify used to be required in more places. In particular, older versions of Flask wouldn't automatically handle non-dict types in responses. Currently, if returning container-based responses (e.g., dict or list), jsonify is not required, and should be omitted throughout for clarity.


# READ ALL: GET /tasks
@bp.get("")
def get_all_tasks():
sort_order = request.args.get("sort")
title_filter = request.args.get("title")

stmt = db.select(Task)

if title_filter:
stmt = stmt.where(Task.title.ilike(f"%{title_filter}%"))

if sort_order == "asc":
stmt = stmt.order_by(Task.title.asc())
elif sort_order == "desc":
stmt = stmt.order_by(Task.title.desc())
else:
stmt = stmt.order_by(Task.id.asc()) # fallback sort

tasks = db.session.scalars(stmt).all()

return jsonify([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.



# READ ONE: GET /tasks/<task_id>
@bp.get("/<task_id>")
def get_one_task(task_id):
task = get_task_or_abort(task_id)
if not isinstance(task, Task):
return task # return 404 or 400 response
Comment on lines +71 to +72

Choose a reason for hiding this comment

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

👀 Due to the get_task_or_abort not actually aborting, we need to make this check anywhere it's used.

return jsonify({"task": task.to_dict()})

# UPDATE: PUT /tasks/<task_id>
@bp.put("/<task_id>")
def update_task(task_id):
task = get_task_or_abort(task_id)
if not isinstance(task, Task):
return task

data = request.get_json()
if "title" not in data or "description" not in data:
return jsonify({"details": "Invalid data"}), 400

task.title = data["title"]
task.description = data["description"]
db.session.commit()
Comment on lines +82 to +88

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

# DELETE: DELETE /tasks/<task_id>
@bp.delete("/<task_id>")
def delete_task(task_id):
task = get_task_or_abort(task_id)
if not isinstance(task, Task):
return task

db.session.delete(task)
db.session.commit()
Comment on lines +96 to +100

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

# PATCH /tasks/<task_id>/mark_complete
@bp.patch("/<task_id>/mark_complete")
def mark_task_complete(task_id):
try:
task_id = int(task_id)
except ValueError:
return {"message": "Invalid task ID"}, 400
task = db.session.get(Task, task_id)
if not task:
return {"message": f"No task with ID {task_id} found"}, 404
Comment on lines +106 to +112

Choose a reason for hiding this comment

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

👀 Why not use your get_task_or_abort method?


task.completed_at = datetime.now(UTC)

Choose a reason for hiding this comment

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

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.

db.session.commit()

# Send Slack message
post_to_slack(f"Someone just completed the task: {task.title}")
return Response(status=204, mimetype="application/json")


#slack_url = os.environ.get("SLACK_WEBHOOK_URL")
#if slack_url:
# slack_message = {"text": f"Someone just completed the task: {task.title}"}
# requests.post(slack_url, json=slack_message)


# PATCH /tasks/<task_id>/mark_incomplete
@bp.patch("/<task_id>/mark_incomplete")
def mark_task_incomplete(task_id):
try:
task_id = int(task_id)
except ValueError:
return {"message": "Invalid task ID"}, 400
task = db.session.get(Task, task_id)
if not task:
return {"message": f"No task with ID {task_id} found"}, 404
Comment on lines +131 to +137

Choose a reason for hiding this comment

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

👀 Why not use your get_task_or_abort method?



task.completed_at = None

Choose a reason for hiding this comment

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

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.

db.session.commit()

return Response(status=204, mimetype="application/json")
23 changes: 23 additions & 0 deletions app/slack_helper.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import os
import requests

def post_to_slack(message):
slack_token = os.environ.get("SLACK_BOT_TOKEN")
slack_channel = os.environ.get("SLACK_CHANNEL_ID")

if not slack_token or not slack_channel:
return

headers = {
"Authorization": f"Bearer {slack_token}",
"Content-Type": "application/json"
}

slack_message = {
"channel": slack_channel,
"text": message
}

response = requests.post("https://slack.com/api/chat.postMessage", json=slack_message, headers=headers)
print("Slack response status:", response.status_code)
print("Slack response body:", response.text)
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.
Loading