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
7 changes: 7 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
from dotenv import load_dotenv # loads .env variables into os.environ
load_dotenv()

Choose a reason for hiding this comment

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

I would move this function call below all of your other imports, there is no need for it to run before them.


from flask import Flask
from .db import db, migrate
from .models import task, goal
from .routes.task_routes import bp as task_list_bp
from .routes.goal_routes import bp as goals_bp
Comment on lines +7 to +8

Choose a reason for hiding this comment

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

Nice! You are following Flask convention to by naming your Blueprints bp and then using as to import them under an alias.

import os

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

# Register Blueprints here
app.register_blueprint(task_list_bp)
app.register_blueprint(goals_bp)
Comment on lines +26 to +27

Choose a reason for hiding this comment

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

⭐️


return app
22 changes: 21 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import Mapped, mapped_column, relationship
from ..db import db
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from .task import Task

class Goal(db.Model):
id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
title: Mapped[str]

Choose a reason for hiding this comment

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

Great work using the declarative mapping here! Since we are doing any specific declarations like in id we can simply use Mapped in conjunction with a Python datatype to declare what this column will look like.

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.

Perfect! You are making a relationship attribute on the Goal model. This attribute is going to be a list of Task models. You then use relationship with back_populates to tell SQLAlchemy to sync this attribute with relationship attribute called goal on the Task model.


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

Choose a reason for hiding this comment

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


def to_dict(self):
goal_as_dict = {}
goal_as_dict["id"] = self.id
goal_as_dict["title"] = self.title

if self.tasks: # Shouldn't affect test prior to wave 6.
goal_as_dict["tasks"] = [task.to_dict() for task in self.tasks]

return goal_as_dict
Comment on lines +17 to +25

Choose a reason for hiding this comment

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

Love how you added a conditional to avoid introducing regression into your codebase. You could also modify this function to give you more control over when you want to include the tasks on a Goal dictionary. A modification of adding a default parameter:

    def to_dict(self, with_tasks=False):
        goal_as_dict = {}
        goal_as_dict["id"] = self.id
        goal_as_dict["title"] = self.title

        if with_tasks: # Shouldn't affect test prior to wave 6. 
            goal_as_dict["tasks"] = [task.to_dict() for task in self.tasks]

        return goal_as_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 sqlalchemy import ForeignKey
from typing import Optional, TYPE_CHECKING
from ..db import db
from datetime import datetime
if TYPE_CHECKING:
from .goal import Goal
Comment on lines +5 to +7

Choose a reason for hiding this comment

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

Suggested change
from datetime import datetime
if TYPE_CHECKING:
from .goal import Goal
from datetime import datetime
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.

Instead of using nullable=True in the mapped_column declaration, we should use Optional in the type annotation to indicate that completed_at can be None. With that inclusion of Optional, it will actually be enough for SQLAlchemy to determine that this column should be nullable. The change would look like the following:

 completed_at: Mapped[Optional[datetime]]

Since you have Optional here we can remove the nullable=True from mapped_column.

goal_id: Mapped[Optional[int]] = mapped_column(ForeignKey("goal.id"))
goal: Mapped[Optional["Goal"]] = relationship(back_populates="tasks")
Comment on lines +14 to +15

Choose a reason for hiding this comment

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

Great work, you added your ForeignKey constraint and relationship attribute!


@classmethod
def from_dict(cls, task_data):
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.

I don't see where you are utilizing this variable anywhere, was it mistakenly left in?

Suggested change
completed_at = task_data.get("completed_at")

new_task = cls(title=task_data["title"],
description=task_data["description"]
)

return new_task

def to_dict(self):
task_as_dict = {}
task_as_dict["id"] = self.id
task_as_dict["title"] = self.title
task_as_dict["description"] = self.description
task_as_dict["is_complete"] = self.completed_at is not None
Comment on lines +27 to +31

Choose a reason for hiding this comment

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

For readability and less repetition, I'd opt for this:

Suggested change
task_as_dict = {}
task_as_dict["id"] = self.id
task_as_dict["title"] = self.title
task_as_dict["description"] = self.description
task_as_dict["is_complete"] = self.completed_at is not None
task_as_dict = {
"id": self.id,
"title": self.title,
"description": self.description,
"is_complete": self.completed_at is not None
}


if self.goal_id:
task_as_dict["goal_id"] = self.goal_id # Shouldn't affect previous tests
Comment on lines +33 to +34

Choose a reason for hiding this comment

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

👍🏿


return task_as_dict
84 changes: 83 additions & 1 deletion app/routes/goal_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,83 @@
from flask import Blueprint
from flask import Blueprint, request, Response
from app.models.goal import Goal
from app.models.task import Task
from ..db import db
from .route_utilities import create_model, get_models_with_filters, validate_model

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

Choose a reason for hiding this comment

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

⭐️


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

@bp.get("")
def get_all_goals():
return get_models_with_filters(Goal, request.args)
Comment on lines +9 to +16

Choose a reason for hiding this comment

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

Lovely!


@bp.get("/<goal_id>")
def get_one_goal(goal_id):
goal = validate_model(Goal, goal_id)
result = {}
result["goal"] = goal.to_dict()
return result
Comment on lines +21 to +23

Choose a reason for hiding this comment

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

I'd opt for response since that is more descriptive of the data it is holding.

Suggested change
result = {}
result["goal"] = goal.to_dict()
return result
response = {}
response["goal"] = goal.to_dict()
return response


@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"]

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")
Comment on lines +36 to +42

Choose a reason for hiding this comment

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

⭐️


@bp.post("/<goal_id>/tasks")
def connect_task_to_goal(goal_id):

goal = validate_model(Goal, goal_id)

request_body = request.get_json()
tasks = []

for task_id in request_body["task_ids"]:
task = validate_model(Task, task_id)
tasks.append(task)
# If any of the task IDs are invalid, will abort before this point.
# Assume we do not want to associate any of the tasks with the goal if *any*
# of the IDs are invalid.

for task in goal.tasks:
# Unassociate previously associated tasks
task.goal_id = None

for task in tasks:
task.goal_id = goal_id # only need to update FK in task to create relationship

db.session.commit()
Comment on lines +52 to +66

Choose a reason for hiding this comment

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

@ellenjin, remember that we used relationship function on our models. This allows us to more easily to navigate the relationship between two models by using their respective relationship attributes. Instead of looping through the tasks individually we could simply leverage the .tasks attribute, a list of associated Tasks like so:

    valid_tasks = []

    for task in task_ids_list:
        task = validate_model(Task, task)
        valid_tasks.append(task) 
    
    goal.tasks = valid_tasks
    db.session.commit()


# To reach this point, all the given IDs have been verified + relationship created.
response_body = {
"id": goal.id,
"task_ids": request_body["task_ids"]
}
return response_body, 200

@bp.get("/<goal_id>/tasks")
def get_tasks(goal_id):
goal = validate_model(Goal, goal_id)
response_body = goal.to_dict()

if not goal.tasks:
response_body["tasks"] = [] # To avoid issues with previous tests
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.

By adding the with_tasks modification to your .to_dict function, you could remove this line and simply pass in with_tasks=True to your method.


return response_body, 200
49 changes: 49 additions & 0 deletions app/routes/route_utilities.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
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 {cls.__name__.lower(): new_model.to_dict()}, 201

def get_models_with_filters(cls, filters=None):
query = db.select(cls)
filters = dict(filters)
sort_order = filters.pop("sort", None)
# defaults to none if not present. Assuming we want to sort by id if 'sort' isn't given

if filters:
for attribute, value in filters.items():
if hasattr(cls, attribute):
query = query.where(getattr(cls, attribute).ilike(f"%{value}%"))
if sort_order == "desc":
sort_clause = cls.title.desc()
elif sort_order == "asc":
sort_clause = cls.title.asc()
else:
sort_clause = cls.id
models = db.session.scalars(query.order_by(sort_clause))
return [model.to_dict() for model in models]
Comment on lines +38 to +49

Choose a reason for hiding this comment

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

For your conditional blocks, you could refactor this by using a dictionary.

Suggested change
if filters:
for attribute, value in filters.items():
if hasattr(cls, attribute):
query = query.where(getattr(cls, attribute).ilike(f"%{value}%"))
if sort_order == "desc":
sort_clause = cls.title.desc()
elif sort_order == "asc":
sort_clause = cls.title.asc()
else:
sort_clause = cls.id
models = db.session.scalars(query.order_by(sort_clause))
return [model.to_dict() for model in models]
if filters:
for attribute, value in filters.items():
if hasattr(cls, attribute):
query = query.where(getattr(cls, attribute).ilike(f"%{value}%"))
if sort_order == "desc":
sort_clause = cls.title.desc()
elif sort_order == "asc":
sort_clause = cls.title.asc()
else:
sort_clause = cls.id
models = db.session.scalars(query.order_by(sort_clause))
return [model.to_dict() for model in models]

25 changes: 25 additions & 0 deletions app/routes/slack_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import os
from flask import abort, make_response
import requests

SLACK_BOT_TOKEN = os.environ.get("SLACK_BOT_TOKEN")
SLACK_API_URL = "https://slack.com/api/chat.postMessage"

def send_slack_notification(message, channel="#task-notifications"):
headers = {
"Authorization": SLACK_BOT_TOKEN,
"Content-Type": "application/json"
}

request_body = {
"channel": channel,
"text": message
}

try:
response = requests.post(SLACK_API_URL, headers=headers, json=request_body)

except: # for any exception
abort(make_response({"message": f"Slack API error"}, 400))


Comment on lines +1 to +25

Choose a reason for hiding this comment

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

Love that you used a helper function here! My only suggestion is that its more of a route utility rather than a route itself. I would move it to your route_utilities.py file.

64 changes: 63 additions & 1 deletion app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,63 @@
from flask import Blueprint
from flask import Blueprint, request, Response
from app.models.task import Task
from ..db import db
from .route_utilities import create_model, get_models_with_filters, validate_model
from datetime import datetime
from .slack_routes import send_slack_notification

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

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

@bp.get("")
def get_all_tasks():
return get_models_with_filters(Task, request.args)

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

@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()

return Response(status=204, mimetype="application/json")
Comment on lines +31 to +36

Choose a reason for hiding this comment

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

This is another opportunity to D.R.Y. up our code! Notice how in this route and the PUT route for goal_routes.py we follow the pattern of:

object.ATTRIBUTE = request_body["ATTRIBUTE"]

We use hasattr and setattr to make a helper function to update our Task and Goal model. It would look like this:

def update_model(obj, data):
    for attr, value in data.items():
        if hasattr(obj, attr):
            setattr(obj, attr, value)
    
    db.session.commit()

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

This refactor not only makes our code D.R.Y but shows that we recognize logic that has higher level usability while handling cases of keys not being found!


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

# Wave 3
@bp.patch("/<task_id>/mark_complete")
def mark_complete(task_id):
task = validate_model(Task, task_id)
task.completed_at = datetime.now()
db.session.commit()

send_slack_notification(f"Someone just completed the task {task.title}")

return Response(status=204, mimetype="application/json")
Comment on lines +47 to +55

Choose a reason for hiding this comment

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

⭐️


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

return Response(status=204, mimetype="application/json")
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