Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
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
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ At submission time, no matter where you are, submit the project via Learn.
This project is designed to fulfill the features described in detail in each wave. The tests are meant to only guide your development.

1. [Setup](ada-project-docs/setup.md)
1. [Testing](ada-project-docs/testing.md)
1. [Testing](ada-project-docs/testing.md)
code coverage: pytest --cov=app --cov-report html --cov-report term
1. [Wave 1: CRUD for one model](ada-project-docs/wave_01.md)
1. [Wave 2: Using query params](ada-project-docs/wave_02.md)
1. [Wave 3: Creating custom endpoints](ada-project-docs/wave_03.md)
Expand Down
9 changes: 7 additions & 2 deletions app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
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.

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

from dotenv import load_dotenv
load_dotenv()
import os

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.

Suggested change
load_dotenv()
import os
import os
load_dotenv()

def create_app(config=None):
Expand All @@ -10,13 +14,14 @@ def create_app(config=None):
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

Choose a reason for hiding this comment

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

Suggested change

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


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

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("Task", 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.


def to_dict(self, include_tasks=False):
goal_dict = {
"id": self.id,
"title": self.title,
}

if include_tasks:
goal_dict["tasks"] = [task.to_dict(include_goal_id=True) for task in self.tasks]
Comment on lines +12 to +19

Choose a reason for hiding this comment

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

This is great! Nice work on the include_tasks default parameter! This is actually a suggestion I mention, so its good to see that you've implemented it already!


return goal_dict

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

Choose a reason for hiding this comment

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

👍🏿

34 changes: 33 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,37 @@
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
if TYPE_CHECKING:
from .goal import Goal
from ..db import db
from datetime import datetime

class Task(db.Model):
id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
title: Mapped[str] = mapped_column(nullable=False)
description: Mapped[str]
completed_at: Mapped[datetime] = mapped_column(nullable=True)
Comment on lines +11 to +13

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 or nullable=False in the mapped_column declaration, we should use Optional in the type annotation to indicate whether a column can be None or not. With that in/exclusion of Optional, it will actually be enough for SQLAlchemy to determine that this column should be nullable. The change would look like the following:

 title: Mapped[str]
 completed_at: Mapped[Optional[datetime]]

goal_id: Mapped[Optional[int]] = mapped_column(ForeignKey("goal.id"))
goal: Mapped[Optional["Goal"]] = relationship("Goal", 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.

Perfect!


def to_dict(self, include_goal_id=False):
task_dict = {
"id": self.id,
"title": self.title,
"description": self.description,
"is_complete": self.completed_at is not None

Choose a reason for hiding this comment

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

}

if include_goal_id and self.goal_id is not None:
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.

👍🏿


return task_dict

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

return new_task
90 changes: 89 additions & 1 deletion app/routes/goal_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,89 @@
from flask import Blueprint
from flask import Blueprint, abort, make_response, request, Response
from dotenv import load_dotenv
from app.models.goal import Goal
from app.models.task import Task
from .route_utilities import validate_model, create_model_from_dict
import os
import requests
from ..db import db
load_dotenv()

Choose a reason for hiding this comment

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

You have this function call in your __init__.py so you can remove it here being that it only needs to be called once. Plus, it's best to have it in your application entry point, __init__.py.

Suggested change
load_dotenv()



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.

👍🏿


# create a new goal
@bp.post("")
def create_goal():
request_body = request.get_json()

try:
new_goal = create_model_from_dict(Goal, request_body)
except KeyError as e:
response = {"details": "Invalid data"}
abort(make_response(response, 400))
Comment on lines +19 to +23

Choose a reason for hiding this comment

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

Since the expected error will arise from create_model_from_dict We could move this try/except inside of that function. This will result in be Single Responsibility Principle--the route function will no handle the error of another function.


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

Choose a reason for hiding this comment

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

Notice how here and inside of the POST route for tasks you have the very similar code. You get back the model and status code and then build the response tuple. This only difference between the two is the key for the outer dictionary. We could refactor create_model_from_dict to dynamically create this response tuple and then simply return the result.

Please refer to my comment in your create_model_from_dict function about what that refactor could look like.


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

goals = db.session.scalars(query)

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

# get one goal
@bp.get("/<goal_id>")
def get_one_goal(goal_id):
goal = validate_model(Goal, goal_id)

return {"goal": goal.to_dict()}, 200
Comment on lines +37 to +41

Choose a reason for hiding this comment

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

👍🏿


# update one goal
@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")

# delete one goal
@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 +57 to +63

Choose a reason for hiding this comment

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

Nice! Consider adding a few blank lines within your function to separate logically distinct sections of the code (e.g., setup, processing, return). According to PEP 8, blank lines can improve readability by visually grouping related operations. Right now, everything runs together, which can make it harder to quickly understand the structure and flow of your function.

Suggested change
@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.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")


# add list of tasks to a goal
@bp.post("/<goal_id>/tasks")
def add_tasks_to_goal(goal_id):
goal = validate_model(Goal, goal_id)
request_body = request.get_json()

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

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

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

Choose a reason for hiding this comment

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

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


db.session.commit()

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

# getting tasks of one goal
@bp.get("/<goal_id>/tasks")
def get_tasks_of_goal(goal_id):
goal = validate_model(Goal, goal_id)

return goal.to_dict(include_tasks=True), 200

Choose a reason for hiding this comment

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

🫡

25 changes: 25 additions & 0 deletions app/routes/route_utilities.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
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} is 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} is not found"}
abort(make_response(response, 404))

return model

def create_model_from_dict(cls, data):
model = cls.from_dict(data)
db.session.add(model)
db.session.commit()
return model

Choose a reason for hiding this comment

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

Notice how we have the same response body structure both routes that we call this function in:

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

and

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

The only difference being we need to have either "task" or "goal" as the key. This is just the name of the model that we are making an instance of in lowercase. We could use the .__name__ attribute to dynamically name this like so:

 return {f"{cls.__name__.lower()}": model.to_dict()}, 201

This change will allow us simply call the function and return the response that it builds us.


113 changes: 112 additions & 1 deletion app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,112 @@
from flask import Blueprint
from flask import Blueprint, abort, make_response, request, Response
from dotenv import load_dotenv
from app.models.task import Task
from .route_utilities import validate_model, create_model_from_dict
from datetime import datetime, timezone
import os
import requests
from ..db import db
load_dotenv()

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

# create a new task
@bp.post("")
def create_task():
request_body = request.get_json()

try:
new_task = create_model_from_dict(Task, request_body)

except KeyError as error:
response = {"details": "Invalid data"}
abort(make_response(response, 400))
Comment on lines +18 to +23

Choose a reason for hiding this comment

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

Like in goal_routes, this logic belongs inside of create_model_from_dict.


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

# read all tasks
@bp.get("")
def get_all_tasks():
query = db.select(Task)

sort = request.args.get("sort")

if sort == "asc":
query = query.order_by(Task.title.asc())
elif sort == "desc":
query = query.order_by(Task.title.desc())

tasks = db.session.scalars(query)

tasks_response = [task.to_dict() for task in tasks]
Comment on lines +30 to +41

Choose a reason for hiding this comment

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

Nice work on handling the filtering logic for this route and the goal route. To D.R.Y. this up some more we could use similar logic to theget_models_with_filters helper function that we implemented for flasky.


return tasks_response

# read one task
@bp.get("/<task_id>")
def get_one_task(task_id):
task = validate_model(Task, task_id)

return {"task": task.to_dict(include_goal_id=True)}, 200

Choose a reason for hiding this comment

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

👍🏿


# update task
@bp.put("/<task_id>")
def update_one_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 +58 to +61

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!


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

# delete task
@bp.delete("/<task_id>")
def delete_one_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.now(timezone.utc)

db.session.commit()

# send slack message
slack_token = os.environ.get("SLACK_BOT_TOKEN")
slack_channel = os.environ.get("SLACK_CHANNEL")

if slack_token and slack_channel:
slack_message = {
"channel": slack_channel,
"text": f"Someone just completed the task {task.title}"
}

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

# sends request to slack
slack_response = requests.post("https://slack.com/api/chat.postMessage", json=slack_message, headers=headers)
Comment on lines +84 to +99

Choose a reason for hiding this comment

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

Nice work getting Slack integration in place! A suggestion for improving readability and maintaining Single Responsibility Principle (SRP) is to move the Slack API call into a separate helper function (e.g., send_slack_notification). This keeps the route function focused purely on request processing and database logic, while isolating external service communication. It’ll also make the Slack functionality easier to reuse and test in the future.

Here's an example of what that helper could look like:

def send_slack_notification(message):
    slack_token = os.environ.get("SLACK_TOKEN")
    chanel = os.environ.get("SLACK_CHANEL_ID")
    headers = {
        "Authorization": f"Bearer {slack_token}"
    }
    
    data = {
        "channel": channel,
        "text": message
    }
    
    response = requests.post("https://slack.com/api/chat.postMessage", headers=headers, json=data)


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")
Comment on lines +103 to +111

Choose a reason for hiding this comment

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

⭐️


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