Skip to content

Cohort22 - Sphinx - TatianaSnook #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
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: 7 additions & 2 deletions app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
from flask import Flask
from .db import db, migrate
from .models import task, goal
from .models import task
from .models import goal
from .routes.task_routes import tasks_bp

Choose a reason for hiding this comment

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

Don't forget that the convention for naming blueprints is to name them bp. With each blueprint being named the same thing we will need to import them under an alias like so:

from .routes.task_routes import bp as tasks_bp

Though I see you did it for goal

from .routes.goal_routes import bp as goal_bp
import os

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

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

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

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

return app
17 changes: 16 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import Mapped, mapped_column, relationship
from ..db import db
from app.routes.route_utilities import create_model

class Goal(db.Model):
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):
response = {
"id": self.id,
"title": self.title
}
return response

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

Choose a reason for hiding this comment

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

Nice work on this file! Straightforward and concise!

30 changes: 29 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,33 @@
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import Mapped, mapped_column, relationship
from ..db import db
from sqlalchemy import String, Boolean, ForeignKey
from typing import Optional


class Task(db.Model):
id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
title: Mapped[str]
description: Mapped[str]
completed_at: Mapped[str | None] = mapped_column(String, nullable=True)
is_complete: Mapped[bool] = mapped_column(Boolean, default=False)
goal_id: Mapped[Optional[int]] = mapped_column(ForeignKey("goal.id"))
goal: Mapped[Optional["Goal"]] = relationship(back_populates="tasks")
Comment on lines +11 to +14

Choose a reason for hiding this comment

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

I see you mixed a few syntaxes here for the optional columns. I would research to see which one is most preferred and switch them all to that syntax for consistency and convention.


def to_dict(self, include_goal_id=False):
response = {
"id": self.id,
"title": self.title,
"description": self.description,
"is_complete": self.completed_at is not None,
}
if include_goal_id and self.goal_id is not None:

Choose a reason for hiding this comment

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

So the second condition is really the only condition we need to a check for here since if a task doesn't have a goal then goal_id will be None regardless. Also another thing to consider is the consistency of our responses. Sometimes adding a key (goal_id) and sometimes not makes our API less uniform.

response["goal_id"] = self.goal_id
return response

@classmethod
def from_dict(cls, task_data):
return cls(
title=task_data['title'],
description=task_data['description'],
goal_id=task_data.get("goal_id", None)
)
111 changes: 110 additions & 1 deletion app/routes/goal_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,110 @@
from flask import Blueprint
from flask import Blueprint, request, abort, make_response
from app.models.goal import Goal
from app.models.task import Task
from app.db import db
from app.routes.route_utilities import validate_model

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

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

if "title" not in request_body:
return {"details": "Invalid data"}, 400

title = request_body["title"]

new_goal = Goal(title=title)
db.session.add(new_goal)
db.session.commit()

return {"goal": new_goal.to_dict()}, 201
Comment on lines +11 to +22

Choose a reason for hiding this comment

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

I see that we have a create_model function in our utilities function, is there a reason as to why we didn't utilize it here?



@bp.get("")
def get_all_goals():
query = db.select(Goal).order_by(Goal.id)
goals = db.session.scalars(query)

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


@bp.get("/<goal_id>")
def get_single_goal(goal_id):

goal = validate_model(Goal, goal_id)
return {"goal": goal.to_dict()}

Choose a reason for hiding this comment

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

We essentially just want an outer dictionary structure with a key of the class name, perhaps we could move this into a helper function or add it to a class in order to DRY up some of our code given that we have this same logic throughout the project?



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

Isn't this code similar to our other .put method? How could we use the hasattr and setattr functions that Python provides in order to make DRY up our routes?

db.session.commit()

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


@bp.delete("/<goal_id>")
def delete_task(goal_id):
goal = validate_model(Goal, goal_id)

db.session.delete(goal)
db.session.commit()

return {"details": f'Goal {goal_id} "Build a habit of going outside daily" successfully deleted'}


@bp.post("/<goal_id>/tasks")
def post_tasks_ids_to_goal(goal_id):
goal = validate_model(Goal, goal_id)
request_body = request.get_json()
task_ids = request_body.get("task_ids", [])

if not task_ids:
response = {"message": "Invalid request: missing task_ids"}
abort(make_response(response, 400))

tasks_to_add = []
for task_id in task_ids:
task = validate_model(Task, task_id)

if task not in goal.tasks:
tasks_to_add.append(task)

if tasks_to_add:
goal.tasks.extend(tasks_to_add)
Comment on lines +61 to +79

Choose a reason for hiding this comment

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

Nice work here! I see you leveraged the fact that adding a task to the goals tasks attribute would update the task to have the associated goal id on it as well.

try:
db.session.add(goal)

Choose a reason for hiding this comment

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

Do we need to add this goal since it is already present in our database?

db.session.commit()
except Exception as e:
response = {
"message": f"Failed to update goal with tasks: {str(e)}"}
abort(make_response(response, 500))

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


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

response = {
"id": goal.id,
"title": goal.title,
"tasks": [
{
"id": task.id,
"goal_id": goal.id,
"title": task.title,
"description": task.description,
"is_complete": task.completed_at is not None
}
Comment on lines +99 to +105

Choose a reason for hiding this comment

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

Is there a reason why we don't use our to_dict method?

for task in goal.tasks
]
}

return response, 200
33 changes: 33 additions & 0 deletions app/routes/route_utilities.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
from flask import abort, make_response
from app.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 = {"message": f"Invalid request: missing {error.args[0]}"}
abort(make_response(response, 400))

db.session.add(new_model)
db.session.commit()

return new_model.to_dict(), 201
118 changes: 117 additions & 1 deletion app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,117 @@
from flask import Blueprint
from flask import Blueprint, request, abort, make_response, Response
from app.models.task import Task
from app.db import db
from datetime import datetime
import os
import requests
from app.routes.route_utilities import validate_model

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

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

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

title = request_body["title"]
description = request_body["description"]
completed_at = request_body.get("completed_at", None)
is_complete = request_body.get("is_complete", False)
goal_id = request_body.get("goal_id", None)
Comment on lines +18 to +22

Choose a reason for hiding this comment

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

We could use the create_model method that we created here as well.


new_task = Task(
title=title,
description=description,
is_complete=is_complete,
goal_id=goal_id
)
db.session.add(new_task)
db.session.commit()

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


@tasks_bp.get("")
def get_all_tasks():

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

if sort_order == "asc":
query = db.select(Task).order_by(Task.title.asc())
elif sort_order == "desc":
query = db.select(Task).order_by(Task.title.desc())
else:
query = db.select(Task).order_by(Task.id)

tasks = db.session.scalars(query)
tasks_response = [task.to_dict(include_goal_id=True) for task in tasks]
Comment on lines +40 to +50

Choose a reason for hiding this comment

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

Remember in class we wrote a function called get_model_with_filters though the implementation we had doesn't work in this project, the logic could be modified used here so we could DRY up our code when getting multiple records of a table.


return tasks_response

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

include_goal_id = task.goal_id is not None
return {"task": task.to_dict(include_goal_id=include_goal_id)}


@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"]
task.completed_at = request_body.get("completed_at", None)
task.is_complete = request_body.get("is_complete", False)

db.session.commit()

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


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

url = "https://slack.com/api/chat.postMessage"
api_key = os.environ.get("SLACK_BOT_TOKEN")
headers = {"Authorization": f"Bearer {api_key}"}
request_body = {
"channel": "task-notifications",
"text": f"Task '{task.title}' has been completed!"
}

response = requests.post(url, headers=headers, data=request_body)

if response.status_code != 200:
return {"error": "Failed to send Slack notification"}, 500

return {"task": task.to_dict()}, 200
Comment on lines +84 to +97

Choose a reason for hiding this comment

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

I would move this logic into a helper function and then that helper function into our utilities file to help with separation of concerns and readability.



@tasks_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 {"task": task.to_dict()}, 200


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

db.session.delete(task)
db.session.commit()

return {"details": f'Task {task_id} "Go on my daily walk 🏞" successfully deleted'}

Choose a reason for hiding this comment

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

This response shouldn't be for a specific task, it should be dynamic to return the id and description of any arbitrary task.

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