Skip to content

Sphinx - Salma Anany #27

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 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 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
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,6 @@ dmypy.json
.pytype/

# Cython debug symbols
cython_debug/
cython_debug/

.idea
12 changes: 11 additions & 1 deletion app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import os

from dotenv import load_dotenv
from flask import Flask

from .db import db, migrate
from .models import task, goal
import os
from .routes.goal_routes import goal_bp
from .routes.task_routes import tasks_bp

load_dotenv()


def create_app(config=None):
app = Flask(__name__)
Expand All @@ -16,6 +24,8 @@ def create_app(config=None):

db.init_app(app)
migrate.init_app(app, db)
app.register_blueprint(tasks_bp)
app.register_blueprint(goal_bp)

# Register Blueprints here

Expand Down
5 changes: 3 additions & 2 deletions app/db.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from flask_sqlalchemy import SQLAlchemy
from flask_migrate import Migrate
from flask_sqlalchemy import SQLAlchemy

from .models.base import Base

db = SQLAlchemy(model_class=Base)
migrate = Migrate()
migrate = Migrate()
3 changes: 2 additions & 1 deletion app/models/base.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from sqlalchemy.orm import DeclarativeBase


class Base(DeclarativeBase):
pass
pass
22 changes: 21 additions & 1 deletion app/models/goal.py

Choose a reason for hiding this comment

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

Consider adding the class method from_dict to this model so you don't need to create an instance of Goal in your routes (example from Flasky)

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


class Goal(db.Model):
id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
title: Mapped[str] = mapped_column(nullable=False)
tasks = relationship('Task', back_populates='goal', cascade='all, delete-orphan')

def to_dict(self, no_tasks=True):
result = {"id": self.id, "title": self.title, }
if not no_tasks:
tasks = []
for task in self.tasks:
tasks.append(task.to_dict())
result["tasks"] = tasks
return result

def from_dict(self, data:dict[str, any]):
if id in data:
self.id = data.get("id")
if "title" in data:
self.title = data["title"]
return self
30 changes: 29 additions & 1 deletion app/models/task.py

Choose a reason for hiding this comment

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

Same comment as in goal.py, consider adding the class method from_dict to this model.

Original file line number Diff line number Diff line change
@@ -1,5 +1,33 @@
from sqlalchemy.orm import Mapped, mapped_column
from datetime import datetime

from sqlalchemy import Integer, ForeignKey
from sqlalchemy.orm import Mapped, mapped_column, relationship

from ..db import db


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)

Choose a reason for hiding this comment

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

This white space should be removed

goal_id = db.Column(Integer, ForeignKey('goal.id'), nullable=True)
goal = relationship('Goal', back_populates='tasks')

def to_dict(self):
result = dict(
id=self.id,
title=self.title,
description=self.description,
is_complete=self.completed_at is not None)
if self.goal is not None:
result["goal_id"] = self.goal.id
return result
def from_dict(self, data):
self.id = data["id"]
self.title = data["title"]
self.description = data["description"]
self.goal_id = data["goal_id"]
self.completed_at = None if data["is_complete"] == False else datetime.now()
return self
Empty file added app/routes/__init__.py
Empty file.
104 changes: 103 additions & 1 deletion app/routes/goal_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,103 @@
from flask import Blueprint
from flask import Blueprint, make_response, abort, request

from app import db
from app.models.goal import Goal
from app.models.task import Task

goal_bp = Blueprint("goal_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.

In Learn and in the livecode for Flasky, we name blueprints for each model bp. In goal_routes.py I can deduce that bp is the blueprint for Goal given the name of the file.

If we have multiple route files that each have a blueprint named bp, we can account for name collisions when importing them into app/__init__.py by using aliases:

from .routes.task_routes import bp as tasks_bp
from .routes.goal_routes import bp as goals_bp


def validate_key_in_request(request_body, key, datatype):
if key not in request_body or not isinstance(request_body[key], datatype):
abort(make_response({"details": "Invalid data"}, 400))
return request_body


def validate_goal_id(id):
if not id.isnumeric():
abort(make_response({"details": f" {id} Invalid data"}, 400))


@goal_bp.post("")
def create_goal():
request_body = validate_key_in_request(request.get_json(), "title", str)
new_goal = Goal().from_dict(request_body)
db.session.add(new_goal)
db.session.commit()
response = {
"goal": new_goal.to_dict()
}
return response, 201


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

Choose a reason for hiding this comment

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

Would be nice if all the logic for a GET request to /goals was in a helper method that can be used across different routes. Using a helper here would make this route more concise. Here's how we did it in Flasky

title_param = request.args.get("title")

query = db.select(Goal)
if title_param:
query = query.where(Goal.title.like(f"%{title_param}"))

goals = db.session.scalars(query).all()
return [goal.to_dict() for goal in goals]




@goal_bp.get("/<goal_id>")
def get_one_goal(goal_id):
goal = get_goal_by_id(goal_id)
return {"goal": goal.to_dict()}


@goal_bp.post("/<goal_id>/tasks")
def set_one_goal_tasks(goal_id):
request_body = validate_key_in_request(request.get_json(), "task_ids", list)
goal = get_goal_by_id(goal_id)
task_ids = request_body.get("task_ids", [])

tasks = db.session.query(Task).filter(Task.id.in_(task_ids)).all()
if len(tasks) != len(task_ids):
abort(make_response({"message": "One or more tasks not found"}, 404))

for task in tasks:
task.goal = goal

db.session.commit()

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


@goal_bp.put("/<goal_id>")
def update_goal(goal_id):
goal = get_goal_by_id(goal_id)
request_body = request.get_json()
goal.title = request_body["title"]
db.session.commit()
result = {"goal": goal.to_dict()}
return make_response(result, 200)


@goal_bp.delete("/<goal_id>")
def delete_goal(goal_id):
goal = get_goal_by_id(goal_id)
db.session.delete(goal)
db.session.commit()
response = {"details": f'Goal {goal_id} "{goal.title}" successfully deleted'}
return make_response(response, 200)


def get_goal_by_id(goal_id):
validate_goal_id(goal_id)
query = db.select(Goal).where(Goal.id == goal_id)
goal = db.session.scalar(query)

if not goal:
response = {"message": f"Goal {goal_id} not found"}
abort(make_response(response, 404))
return goal


@goal_bp.get("/<goal_id>/tasks")
def get_one_goal_tasks(goal_id):
return get_goal_by_id(goal_id).to_dict(False)

146 changes: 145 additions & 1 deletion app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,145 @@
from flask import Blueprint
from datetime import datetime

from flask import request, Blueprint, make_response, abort
from sqlalchemy import asc, desc

from app import db
from app.models.task import Task
from app.slack.slack_client import SlackClient
from app.slack.slackmessage import SlackMessage

tasks_bp = Blueprint("tasks_bp", __name__, url_prefix="/tasks")
slack_client = SlackClient()


@tasks_bp.post("")
def create_task():
request_body = request.get_json()
if "title" not in request_body.keys():
abort(make_response({"details": "Invalid data"}, 400))
title = request_body["title"]
if "description" not in request_body.keys():
abort(make_response({"details": "Invalid data"}, 400))
description = request_body["description"]
new_task = Task(title=title, description=description, completed_at=None)
db.session.add(new_task)
db.session.commit()
Comment on lines +17 to +26

Choose a reason for hiding this comment

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

Another great candidate for refactoring to use a create_model helper function so that this route is more concise.

When writing route logic, we might ask ourselves questions like, "Does this route need to know how to process a request, handle potential errors in the request body, create an instance of the object, and add/commit it to the DB?"

We can make our routes concise and readable by using helper methods

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


@tasks_bp.get("")
def get_all_tasks():
description_param = request.args.get("description")
title_param = request.args.get("title")
sort_param = request.args.get("sort")

query = db.select(Task)
if description_param:
query = query.where(Task.description.like(f"%{description_param}%"))

if title_param:
query = query.where(Task.title.like(f"%{title_param}"))
if sort_param == "asc":
query = query.order_by(asc(Task.title))
elif sort_param == "desc":
query = query.order_by(desc(Task.title))

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


def validate_id(id):
if not id.isnumeric():
abort(make_response({"message": f"Tasks id {id} invalid"}, 400))

Comment on lines +55 to +58

Choose a reason for hiding this comment

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

I'd expect the logic for a method like validate_id to check that an ID is valid in the database too, not just check that id is an integer. If you have both of these pieces of logic in this method and also put your error handling in this method, you would be able to DRY up many of your routes quite a bit.


@tasks_bp.get("/<task_id>")
def get_one_task(task_id):
validate_id(task_id)
query = db.select(Task).where(Task.id == task_id)
task = db.session.scalar(query)

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

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


@tasks_bp.put("/<task_id>")
def update_task(task_id):
validate_id(task_id)
query = db.select(Task).where(Task.id == task_id)
task = db.session.scalar(query)

if task is None:
abort(make_response({"message": f"Task {task_id} not found"}, 404))

request_body = request.get_json()
task.title = request_body["title"]
task.description = request_body["description"]
if "is_complete" in request_body.keys() and request_body["is_complete"]:
task.completed_at = datetime.now()
else:
task.completed_at = None
db.session.commit()
return {
"task": task.to_dict()
}


@tasks_bp.delete("/<task_id>")
def delete_task(task_id):
validate_id(task_id)
query = db.select(Task).where(Task.id == task_id)
task = db.session.scalar(query)
if not task:
response = {"message": f"{task_id} not found"}
abort(make_response(response, 404))

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

return {
"details": f'Task {task_id} "{task.title}" successfully deleted'
}


def change_task_status(task_id, is_completed):
task = Task.query.get(task_id)

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. This needs to removed to use the newer syntax:

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

See how we made a generic helper method for validating and fetching records from the DB in Flasky here on lines 10-11

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

if is_completed:
completed_date = datetime.now()
task_text = f"Someone just completed the task '{task.title}'"
else:
completed_date = None
task_text = f"Someone just marked the task '{task.title}' incomplete"

CHANNEL_NAME = "task-notifications"
notification_messages = SlackMessage(CHANNEL_NAME, task_text)
notification_was_sent = slack_client.post_message(notification_messages)
Copy link

@yangashley yangashley Nov 19, 2024

Choose a reason for hiding this comment

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

~~Recall that the project requirement was to use the Python package requests to make 3rd party API calls.

We want folks to get practice using requests and not use the SlackClient because not all 3rd party APIs wil provide you with a custom client.

@SalmaAnany Please let me know, as a repy to this comment, how you would re-write the code for calling Slack using requests.~~

Copy link
Author

Choose a reason for hiding this comment

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

I used requests, please check the client, that's my own class using requests.

Choose a reason for hiding this comment

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

Ah yes, I meant to remove this comment after I saw your SlackClient class! Thanks for pointing me there again.

if notification_was_sent:
Task.query.filter_by(id=task_id).update({Task.completed_at: completed_date})
db.session.commit()
return task


@tasks_bp.patch("/<task_id>/mark_complete")
def mark_completed_task(task_id):
validate_id(task_id)
task = change_task_status(task_id, True)
return make_response({"task": task.to_dict()}, 200)


@tasks_bp.patch("/<task_id>/mark_incomplete")
def mark_in_complete_task(task_id):
validate_id(task_id)
task = change_task_status(task_id, False)
return make_response({"task": task.to_dict()}, 200)
Empty file added app/slack/__init__.py
Empty file.
19 changes: 19 additions & 0 deletions app/slack/slack_client.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import os

import requests

from app.slack.slackmessage import SlackMessage


class SlackClient:
def __init__(self):
self.token = os.environ.get('SLACK_TOKEN')
self.url = "https://slack.com/api"
self.headers = {
"Authorization": f"Bearer {self.token}",
"Content-Type": "application/json"
}

def post_message(self, slack_message: SlackMessage):
response = requests.post(f"{self.url}/chat.postMessage", headers=self.headers, json=slack_message.__dict__)
return response.status_code == 200
4 changes: 4 additions & 0 deletions app/slack/slackmessage.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class SlackMessage:
def __init__(self, channel, text):
self.channel = channel
self.text = text
Empty file added app/sql/__init__.py

Choose a reason for hiding this comment

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

What is this directory used for?

Copy link
Author

Choose a reason for hiding this comment

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

I created it thinking I could add all the database querying functions and make the code dry, but sadly, I did not have enough time.

Empty file.
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