Skip to content

Maybellene Aung C22 task list #42

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 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 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
5 changes: 5 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
from .db import db, migrate
from .models import task, goal
import os
from .routes.task_routes import bp as tasks_bp
from .routes.goal_routes import bp as goals_bp

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

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


return app
28 changes: 27 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import Mapped, mapped_column, relationship
from ..db import db
from flask import abort, make_response

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.

Consider whether the this column for Goal should be nullable. It feels unconventional to allow someone to create a goal that gets saved to the DB without a title.

tasks: Mapped[list["Task"]] = relationship(back_populates="goal")

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

return goal_as_dict
Comment on lines +11 to +15

Choose a reason for hiding this comment

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

Can also directly return a dictionary literal

Suggested change
goal_as_dict = {}
goal_as_dict["id"] = self.id
goal_as_dict["title"] = self.title
return goal_as_dict
return {
"id": self.id,
"title": self.title
}



@classmethod
def obj_from_dict(cls, goal_data):
title = goal_data.get("title", None)

if not title:
response = {"details": "Invalid data"}
abort(make_response(response, 400))

new_goal = cls(
title=title
)

return new_goal

43 changes: 42 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,46 @@
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 flask import make_response, abort
from sqlalchemy import ForeignKey

class Task(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.

As with Goal, consider which of the columns for Task should be nullable and which should not be.

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


# book object to dict representation

Choose a reason for hiding this comment

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

Task object?

def obj_to_dict(self):
task_as_dict = {}
task_as_dict["id"] = self.id
if self.goal:
task_as_dict["goal_id"] = self.goal.id
task_as_dict["title"] = self.title
task_as_dict["description"] = self.description
task_as_dict["is_complete"] = True if self.completed_at is not None else False

Choose a reason for hiding this comment

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

Consider encapsulating this logic into a helper function, maybe something like is_complete that calculates what the value should be for the attribute and call it here.

Suggested change
task_as_dict["is_complete"] = True if self.completed_at is not None else False
task_as_dict["is_complete"] = self.is_complete()


return task_as_dict

@classmethod
# create instance from request body
def obj_from_dict(cls, task_data):
title = task_data.get("title", None)
description = task_data.get("description", None)

if not title or not description:
response = {"details": "Invalid data"}
abort(make_response(response, 400))

new_task = cls(
title=title,
description=description,
completed_at=task_data.get("completed_at", None)

)

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, Response, request, abort, make_response

Choose a reason for hiding this comment

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

Response, abort, and make_response are imported but never accessed so should be removed.

from ..models.goal import Goal
from ..db import db
from ..models.task import Task
from .route_utilities import validate_model

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

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

db.session.add(new_goal)
db.session.commit()
response = {"goal": new_goal.obj_to_dict()}
return response, 201

@bp.post("/<goal_id>/tasks")
def add_tasks_to_goals(goal_id):
goal = validate_model(Goal, goal_id)
request_body = request.get_json()
task_ids = request_body["task_ids"]
tasks = Task.query.filter(Task.id.in_(task_ids)).all()

Choose a reason for hiding this comment

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

The <Model>.query syntax has been deprecated since SQLAlchemy 2.0 and should not be used for new development.

Review the docs here

Here, I'd probably use your validate_model method since task_ids are values that are grabbed from the request body that the client sent with the request. Since we can't assume that they're valid, we can validate:

Suggested change
tasks = Task.query.filter(Task.id.in_(task_ids)).all()
tasks = [validate_model(Task, task_id) for task_id in task_ids]

goal.tasks = tasks

db.session.commit()
response = {
"id": goal.id,
"task_ids": task_ids

Choose a reason for hiding this comment

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

Notice that task_ids comes from the request body (line 23). Instead of taking the client-provided value, it would be nice to return a response that has task ids that we get from the goal that we fetched from the db on line 21. This way we know for sure that we're returning data from our source of truth (the DB) instead of just echoing back what the user sent to the API (what we get from request_body["task_ids"])

You'd have to grab tasks from goal like:

tasks_from_goal = goal.tasks
task_ids_from_goal = [task.id for task in tasks_from_goal]

 return {
        "id": int(goal_id),
        "task_ids": task_ids_from_goal
}

}

return response, 200


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

goals_response = [goal.obj_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.obj_to_dict()}, 200

# get all tasks of a certain goal
@bp.get("/<goal_id>/tasks")
def get_all_tasks_of_goal(goal_id):
goal = validate_model(Goal, goal_id)

tasks_list = [task.obj_to_dict() for task in goal.tasks]
print(tasks_list)
response = {
"id": goal.id,
"title": goal.title,
"tasks": tasks_list
}

return response, 200


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

request_body = request.get_json()
goal.title = request_body["title"]

db.session.commit()

response = {"goal": goal.obj_to_dict()}
return response, 200


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

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

response = {"details": f"Goal {goal.id} \"{goal.title}\" successfully deleted"}
return response, 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

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} was not found"}
abort(make_response(response, 404))

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

Choose a reason for hiding this comment

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

Response, is imported but never accessed so should be removed.

from ..models.task import Task
from ..db import db
from datetime import datetime
import requests
import os


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

# make a POST request to /tasks with the following HTTP request body
@bp.post("")
def create_task():
request_body = request.get_json()
new_task = Task.obj_from_dict(request_body)

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

response = {"task": new_task.obj_to_dict()}

return response, 201

# GET request to /tasks
@bp.get("")
def get_all_task():
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())
else:
query = query.order_by(Task.id)
tasks = db.session.scalars(query)

tasks_response = [task.obj_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.obj_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"]

db.session.commit()

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

@bp.patch("/<task_id>/mark_complete")
def update_complete(task_id):
task = validate_model(Task, task_id)
task.completed_at = datetime.utcnow()

db.session.commit()

path = "https://slack.com/api/chat.postMessage"
token = os.environ.get("SLACK_BOT_TOKEN")
channel_id = "C07V4J7ABF1"
Comment on lines +67 to +69

Choose a reason for hiding this comment

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

Nice job using variables to reference these values so you don't have string literals embedded below. Since these are constant variables, they should be named with all capital letters.


headers = {
"Authorization": f"Bearer {token}"
}
data = {
"channel": channel_id,
"text": f"Someone just completed the task {task.title}"
}

requests.post(path, headers=headers, json=data)

Choose a reason for hiding this comment

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

Prefer all the logic relating to making a call to the Slack API be encapsulated in a helper function (maybe something like call_slack_api). This would keep this route more concise and reduce the number of responsibilities the logic has to account for.


response = {"task": task.obj_to_dict()}
return response, 200


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

db.session.commit()

response = {"task": task.obj_to_dict()}
return response, 200

@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} \"{task.title}\" successfully deleted"}

def validate_model(Task, task_id):
try:
task_id = int(task_id)
except:
response = {"message": f"Task {task_id} is invalid"}
abort(make_response(response, 400))

query = db.select(Task).where(Task.id == task_id)
task = db.session.scalar(query)

if not task:
response = {"message": f"Task {task_id} was not found"}
abort(make_response(response, 404))

return task
Comment on lines +103 to +117

Choose a reason for hiding this comment

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

Looks like this duplicates the logic in validate_model in route_utilities.py. This should be removed so that you can use the more generic version of this helper function


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