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
11 changes: 8 additions & 3 deletions app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,27 @@
from flask import Flask
from .db import db, migrate
from .models import task, goal
from .routes.task_routes import bp as tasks_bp
from .routes.goal_routes import bp as goals_bp
Comment on lines +4 to +5

Choose a reason for hiding this comment

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

👍

import os
from dotenv import load_dotenv
from flask_cors import CORS

def create_app(config=None):
load_dotenv()
app = Flask(__name__)
CORS(app)

app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
app.config['SQLALCHEMY_DATABASE_URI'] = os.environ.get('SQLALCHEMY_DATABASE_URI')

if config:
# Merge `config` into the app's configuration
# to override the app's default settings for testing
app.config.update(config)

db.init_app(app)
migrate.init_app(app, db)

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

return app
24 changes: 23 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import Mapped, mapped_column, relationship
from ..db import db
from typing import List, Optional

from typing import TYPE_CHECKING
if TYPE_CHECKING: from .task import Task
Comment on lines +5 to +6

Choose a reason for hiding this comment

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

Nice inclusion of this extra check to clear up the type warnings in VS Code.


class Goal(db.Model):
id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
title: Mapped[str]
tasks: Mapped[List["Task"]] = relationship("Task", back_populates="goal")

@classmethod
def from_dict(cls, goal_data):
new_goal = Goal(title=goal_data["title"])
return new_goal
Comment on lines +15 to +16

Choose a reason for hiding this comment

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

Since we have access to the class with the cls keyword that's necessary for defining a class method, we can use cls instead of Goal.

Using the more generic cls means that if we ever update the class name, we won't need to update line 15.

Suggested change
new_goal = Goal(title=goal_data["title"])
return new_goal
return cls(title=goal_data["title"])


def to_dict(self, include_tasks=False, include_task_ids=False):
goal_dict = {
"id": self.id,
"title": self.title
}
if include_tasks:
goal_dict["tasks"] = [task.to_dict() for task in self.tasks]
if include_task_ids:
goal_dict["task_ids"] = [task.id for task in self.tasks]
Comment on lines +18 to +26

Choose a reason for hiding this comment

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

We can simplify this instance method because we don't need to pass any arguments to it.

self.tasks will be a list with elements or an empty list so we can use this to check if we should include tasks for a goal.

Also, per the project documentation for Wave 6, the requirement is that the response body for a POST request to /goals/id/tasks should be:

{
  "id": 1,
  "task_ids": [1, 2, 3]
}

Therefore, your to_dict function does not need to create a dictionary with key tasks and value is a list of dictionaries representing tasks (what you wrote on lines 23-24) so this should be removed.

Suggested change
def to_dict(self, include_tasks=False, include_task_ids=False):
goal_dict = {
"id": self.id,
"title": self.title
}
if include_tasks:
goal_dict["tasks"] = [task.to_dict() for task in self.tasks]
if include_task_ids:
goal_dict["task_ids"] = [task.id for task in self.tasks]
def to_dict(self):
goal_dict = {
"id": self.id,
"title": self.title
}
if include_task_ids:
goal_dict["task_ids"] = [task.id for task in self.tasks]

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

from typing import TYPE_CHECKING
if TYPE_CHECKING: 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[Optional[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.

Suggested change
completed_at: Mapped[Optional[datetime]] = mapped_column(nullable=True)
completed_at: Mapped[Optional[datetime]]

To set nullability for a table's column we should use Optional[] per the SQLAlchemy documentation. We do not need to add more specific typing information regarding nullability so we don't need to use mapped_column.

You can review Building an API Part 3 to read about the Optional syntax too.
image

image

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

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

Choose a reason for hiding this comment

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

Suggested change

Remove unnecessary blank lines (per the PEP8 style guide section on blank lines)

)
return new_task
Comment on lines +19 to +25

Choose a reason for hiding this comment

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

I left the same comment for the from_dict class method in goal.py.

I'd prefer you use the cls keyword here instead of Task by name to keep this method more generic. If you ever had to change this class name to something else, then you would need to remember to refactor line 19 too.

Suggested change
new_task = Task(
title=task_data["title"],
description=task_data["description"],
completed_at=task_data.get("completed_at")
)
return new_task
return cls(
title=task_data["title"],
description=task_data["description"],
completed_at=task_data.get("completed_at")
)


def to_dict(self):
task_dict = {
"id": self.id,
"title": self.title,
"description": self.description,
"is_complete": bool(self.completed_at)
}
if self.goal_id:
task_dict["goal_id"] = self.goal_id
return task_dict
94 changes: 93 additions & 1 deletion app/routes/goal_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,93 @@
from flask import Blueprint
from flask import Blueprint, request, Response, abort, make_response
from app.models.goal import Goal
from app.models.task import Task
from ..db import db
from .route_utilities import validate_model

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

@bp.post("")
def create_goal():
request_body = request.get_json()

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

db.session.add(new_goal)
db.session.commit()
Comment on lines +13 to +20

Choose a reason for hiding this comment

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

This works 👍

Prefer the changes we introduced in the lesson on Refactoring (Part 7 of Building an API) were incorporated here - specifically the create_model helper function.

You can review the implementation of that helper here.

Writing a helper function and then using it here makes this route create_goal have fewer responsibilities and would also make it more concise. A refactored version would just have 2 lines like this:

@bp.post("")
def create_goal():
    request_body = request.get_json()
    return create_model(Goal, request_body)


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

@bp.get("")
def get_all_goals():
goals = Goal.query.all()

Choose a reason for hiding this comment

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

Note that using all will return the results in a list. In our curriculum we use scalars. There is a difference and I encourage you to do research and review documentation to understand when you should use one over the other.

In Flasky, we get all records of a model with:

query = db.select(Cat)
query = db.select(Cat).order_by(Cat.id)
cats = db.session.scalars(query)

You can read more about scalars vs all: https://hatchjs.com/sqlalchemy-scalars-vs-all/

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

@bp.get("/<goal_id>")
def get_one_goal(goal_id):
goal = validate_model(Goal, goal_id)
return {"goal": goal.to_dict()}, 200

@bp.put("/<goal_id>")
def update_goal(goal_id):
goal = validate_model(Goal, goal_id)
request_body = request.get_json()

goal.title = request_body["title"]

Choose a reason for hiding this comment

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

If the request body does not have a key called "title" then this will throw an unhandled exception. What kind of error handling can you add to this route so that you can return a custom response to the client when they send you an incorrect request body?

db.session.commit()

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

@bp.delete("/<goal_id>")
def delete_goal(goal_id):
goal = validate_model(Goal, goal_id)
db.session.delete(goal)
db.session.commit()

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



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

Choose a reason for hiding this comment

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

🧐 You're mixing Flask syntax.

We prefer to use .post(), like you did on line 9 in this file, which is a shorthand for using .route() and passing in methods (see docs here). We should be sure to not introduce inconsistencies in our codebase which it make it harder to maintain and unpredictable.

Take care to use the syntax that a project already has OR follow the most up to date syntax per the library's documentation.

Suggested change
@bp.route("/<goal_id>/tasks", methods=["POST"])
@goals_bp.post("/<goal_id>/tasks")

def assign_tasks_to_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:
abort(400, description={"details": "Invalid data"})

Choose a reason for hiding this comment

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

Suggested change

Remove unnecessary blank line


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

Choose a reason for hiding this comment

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

Suggested change

Remove unnecessary blank line


for task in goal.tasks:
task.goal_id = None

for task in new_tasks:
task.goal_id = goal.id
Comment on lines +64 to +74

Choose a reason for hiding this comment

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

We can think of the logic here a little differently. We don't actually need a list of tasks. We need to make sure that the tasks that goal has is populated with the list of tasks the client provided.

Instead of create a list of new_tasks we could use a loop to assign these tasks to the goal.

Suggested change
new_tasks = []
for task_id in request_body["task_ids"]:
task = validate_model(Task, task_id)
new_tasks.append(task)
for task in goal.tasks:
task.goal_id = None
for task in new_tasks:
task.goal_id = goal.id
for task_id in request_body["task_ids"]:
task = validate_model(Task, task_id)
goal.tasks.append(task)
db.session.commit()


db.session.commit()

return {
"id": goal.id,
"task_ids": request_body["task_ids"]
}, 200
Comment on lines +78 to +81

Choose a reason for hiding this comment

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

request_body["task_ids"] is a list of ids that we get from the client in the request body. Rather than sending a response back to the client that echos the input task_ids the client provided to us, we should fetch the task ids directly from the goal (since we appended the new tasks to the existing goal).

Also the default status code that Flask sends is 200 so we can leave that off here.

Suggested change
return {
"id": goal.id,
"task_ids": request_body["task_ids"]
}, 200
return {
"id": goal.id,
"task_ids": [task.id for task in goal.tasks]
}




@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.

❗️Prefer to use .get() to keep your codebase's route definitions consistent.

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

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

return {
"id": goal.id,
"title": goal.title,
"tasks": [task.to_dict() for task in goal.tasks]
}, 200
18 changes: 18 additions & 0 deletions app/routes/route_utilities.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from flask import abort, make_response
from ..db import db
from app.models.task import Task
from app.models.goal import Goal
Comment on lines +3 to +4

Choose a reason for hiding this comment

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

Notice that you import these two models, but don't reference them anywhere. We should remove unused imports so that our codebase doesn't become cluttered.

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


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

model = db.session.get(cls, model_id)

if not model:

abort(make_response({"message": f"{cls.__name__.lower()} {model_id} not found"}, 404))

return model
137 changes: 136 additions & 1 deletion app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,136 @@
from flask import Blueprint
from flask import Blueprint, abort,make_response, request, Response, current_app

Choose a reason for hiding this comment

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

Nitpick: inconsistent spacing

Suggested change
from flask import Blueprint, abort,make_response, request, Response, current_app
from flask import Blueprint, abort, make_response, request, Response, current_app

from app.models.task import Task
from ..db import db
from datetime import datetime
from sqlalchemy import asc, desc

Choose a reason for hiding this comment

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

These imports are not used and should be removed

Suggested change
from sqlalchemy import asc, desc

from .route_utilities import validate_model
import requests
import os

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

@bp.post("")
def create_task():
request_body = request.get_json()

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

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

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


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

title_param = request.args.get("title")
if title_param:
query = query.where(Task.title.ilike(f"%{title_param}%"))

description_param = request.args.get("description")
if description_param:
query = query.where(Task.description.ilike(f"%{description_param}%"))

sort_param = request.args.get("sort")
query = query.order_by(get_sort_order(sort_param))

tasks = db.session.scalars(query)
tasks_response = [task.to_dict() for task in tasks]
return tasks_response


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


@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"]
Comment on lines +60 to +61

Choose a reason for hiding this comment

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

These two lines of code will throw unhandled exceptions if the request body does not have "title" key or "description" key.

How could you refactor this route so that you can send back a custom error response if the client sends an incorrect request body?



completed_at_value = request_body.get("completed_at")
if completed_at_value:
try:
task.completed_at = datetime.fromisoformat(completed_at_value)
except ValueError:
response = {"message": "Invalid datetime format for completed_at. Expected ISO 8601 format."}
abort(make_response(response, 400))
else:
task.completed_at = None

db.session.commit()

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


@bp.delete("/<task_id>")
def delete_task(task_id):
task = validate_model(Task,task_id)
db.session.delete(task)
db.session.commit()

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


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

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

# Slack
slack_token = os.environ.get("SLACK_BOT_TOKEN")
channel = os.environ.get("SLACK_CHANNEL", "task-notifications")
Comment on lines +96 to +97

Choose a reason for hiding this comment

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

These are constant values so we should name these variables accordingly.

Suggested change
slack_token = os.environ.get("SLACK_BOT_TOKEN")
channel = os.environ.get("SLACK_CHANNEL", "task-notifications")
SLACK_TOKEN = os.environ.get("SLACK_BOT_TOKEN")
CHANNEL = os.environ.get("SLACK_CHANNEL", "task-notifications")

if slack_token:
try:
requests.post(
"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.

Prefer this string value to be referenced by a constant variable too

SLACK_URL = "https://slack.com/api/chat.postMessage"
requests.post(SLACK_URL, 
# rest of your logic

headers={
"Authorization": f"Bearer {slack_token}",
"Content-Type": "application/json"
},
json={
"channel": channel,
"text": f"Someone just completed the task {task.title}"
}
)
except Exception as e:
current_app.logger.error(f"Slack error: {e}")
Comment on lines +96 to +112

Choose a reason for hiding this comment

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

Prefer all of this logic to be in a helper function (maybe like call_slack_api()). Using a helper function would make this route more concise and single-responsibility.


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


@bp.patch("/<task_id>/mark_incomplete")
def mark_task_incomplete(task_id):
task = validate_model(Task, task_id)

task.completed_at = None
db.session.commit()

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


def get_sort_order(sort_param):
if sort_param == "asc":
return Task.title.asc()
elif sort_param == "desc":
return Task.title.desc()
elif sort_param == "id":
return Task.id.asc()
elif sort_param == "is_complete":
return Task.is_complete.asc()
return Task.id
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