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
4 changes: 2 additions & 2 deletions ada-project-docs/optional-enhancements.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Optional Enhancements
dour# Optional Enhancements

## Goal

Expand Down Expand Up @@ -42,7 +42,7 @@ Here are some ideas to start:
- Create a class method in `Task` named `from_json()`
- Converts JSON into a new instance of `Task`
- Takes in a dictionary in the shape of the JSON our API receives in the create and update routes
- Returns an instance of `Task`
- Returns an instance of `Task`

### Use List Comprehensions

Expand Down
9 changes: 4 additions & 5 deletions ada-project-docs/wave_01.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Tasks should contain these attributes. **The tests require the following columns

### Tips

- To work with date information, we can import the `datetime` data type with the import line `from datetime import datetime`.
- To work with date information, we can import the `datetime` data type with the import line `from datetime import datetime`.
- SQLAlchemy supports optional, or _nullable_, columns with specific syntax.
- Don't forget to run:
- `flask db init` once during setup
Expand All @@ -50,7 +50,7 @@ Tasks should contain these attributes. **The tests require the following columns

### CLI

In addition to testing your code with pytest and postman, you can play test your code with the CLI (Command Line Interface) by running `python3 cli/main.py`.
In addition to testing your code with pytest and postman, you can play test your code with the CLI (Command Line Interface) by running `python3 cli/main.py`.

The flask server needs to be running first before running the CLI.

Expand Down Expand Up @@ -122,8 +122,7 @@ As a client, I want to be able to make a `GET` request to `/tasks` when there ar

#### Get One Task: One Saved Task

As a client, I want to be able to make a `GET` request to `/tasks/1` when there is at least one saved task and get this response:

As a client, I want to be able to make a `GET` request to `/tasks/1` when there is at least one saved task and get this respons
`200 OK`

```json
Expand Down Expand Up @@ -192,7 +191,7 @@ The response code should be `404`.
You may choose the response body.

Make sure to complete the tests for non-existing tasks to check that the correct response body is returned.


#### Create a Task: Invalid Task With Missing Data

Expand Down
9 changes: 7 additions & 2 deletions app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
from flask import Flask
from .db import db, migrate
from .models import task, goal
from .routes.task_routes import tasks_bp
from .routes.goal_routes import goals_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')
app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False
app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get("SQLALCHEMY_DATABASE_URI")

if config:
# Merge `config` into the app's configuration
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(goals_bp)

return app
12 changes: 11 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
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]

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 odd to allow someone to create a goal that gets saved to the DB without a title.

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

def to_dict(self):
return dict(id=self.id, title=self.title)

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

Choose a reason for hiding this comment

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

👍

31 changes: 30 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,34 @@
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import Mapped, mapped_column, relationship
from sqlalchemy import ForeignKey
Comment on lines +1 to +2

Choose a reason for hiding this comment

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

relationship is imported but not accessed because on line 14 you wrote db.relationship instead of just relationship

Same with ForeignKey, you import it but don't access it because on line 13 you wrote db.ForeignKey

The most current documentation and our Learn content use syntax that does not include db.

from ..db import db
from datetime import datetime
from typing import Optional


class Task(db.Model):
id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
title: Mapped[str] = mapped_column(String, nullable=False)
description: 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")

def to_dict(self):
task_dict = {
"id": self.id,
"title": self.title,
"description": self.description,
"is_complete": task.is_complete()})
}
if self.goal_id is not None:
task_dict["goal_id"] = self.goal_id
return task_dict

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

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

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



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

try:
new_goal = Goal.from_dict(request_body)

except KeyError as error:
response = {"details": f"Invalid data - missing {error.args[0]}"}
abort(make_response(response, 400))

db.session.add(new_goal)
db.session.commit()

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


@goals_bp.get("")
def get_all_goals():
query = db.select(Goal)

title_param = request.args.get("title")
sort = request.args.get("sort", "asc") # Default to ascending order

query = db.select(Goal)

Choose a reason for hiding this comment

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

Did you accidentally write line 35 again? It repeats what's on line 30 and since you already have a Selet object, you don't need to write this again.


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

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

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

return goals_response, 200


@goals_bp.get("/<goal_id>")
def get_single_goal(goal_id):
goal = validate_goal(goal_id)

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


@goals_bp.put("/<goal_id>")
def update_goal(goal_id):
goal = validate_goal(goal_id)
request_body = request.get_json()

goal.title = request_body["title"]

db.session.commit()
response = {"goal": goal.to_dict()}
return response, 200


@goals_bp.patch("/<goal_id>/mark_complete")
def mark_goal_complete(goal_id):
goal = validate_goal(goal_id)
goal.completed_at = datetime.now()
db.session.commit()

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


@goals_bp.patch("/<goal_id>/mark_incomplete")
def mark_goal_incomplete(goal_id):
goal = validate_goal(goal_id)
goal.completed_at = None

db.session.commit()

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


@goals_bp.delete("/<goal_id>")
def delete_goal(goal_id):
goal = validate_goal(goal_id)

db.session.delete(goal)
db.session.commit()
response = {"details": f'Goal {goal_id} "{goal.title}" successfully deleted'}
return response, 200


@goals_bp.post("/<goal_id>/tasks")
def assign_tasks_to_goal(goal_id):
goal = validate_goal(goal_id)
request_body = request.get_json()
task_ids = request_body.get("task_ids", [])

tasks = [validate_model(Task, task_id) for task_id in task_ids]

goal.tasks.extend(tasks)
db.session.commit()

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

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 101). 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 99. 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.get("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": goal.id,
        "task_ids": task_ids_from_goal
}



@goals_bp.get("/<goal_id>/tasks")
def get_tasks_by_goal(goal_id):
goal = validate_goal(goal_id)
goal_dict = goal.to_dict()

Choose a reason for hiding this comment

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

While we are creating a dictionary that represents a goal, it might be more descriptive to name this variable response since we're creating a dictionary that we want to send back as a response to the client.

goal_dict["tasks"] = [task.to_dict() for task in goal.tasks]
return goal_dict


@goals_bp.get("<goal_id>/tasks/<task_id>")
def get_one_task_by_goal(goal_id, task_id):

goal = validate_goal(Goal, goal_id)
task = validate_goal(Task, task_id)

if task in goal.tasks:
goal_dict = goal.to_dict()
goal_dict["task"] = task.to_dict()
return goal_dict
return {"details": f"Task {task.id} not found for Goal {goal.id}"}, 404


def validate_goal(goal_id):
try:
goal_id = int(goal_id)
except ValueError:
abort(400, description="Invalid goal_id")

goal = Goal.query.get(goal_id)

Choose a reason for hiding this comment

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

This is deprecated syntax. <model>.query syntax has been deprecated since SQLAlchemy 2.0 and should not be used for new development. Please review how we validate models in Flasky.

The syntax we use, which follows the most up to date SQLAlchemy syntax, is:

    query = db.select(cls).where(cls.id == model_id)
    model = db.session.scalar(query)

if goal is None:
abort(make_response({"message": f"goal {goal_id} not found"}, 404))

return goal
Comment on lines +132 to +142

Choose a reason for hiding this comment

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

This method is nearly identical to validate_task in task_routes.py. To avoid repeating code, we should make a more generic helper function like validate_model that can be used across all the different routes.

You can create a file routes/route_utilities.py and write the helper function there, then import it where you need it.

120 changes: 119 additions & 1 deletion app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -1 +1,119 @@
from flask import Blueprint
from flask import Blueprint, abort, make_response, request, Response
from app.models.task import Task
from ..db import db
from datetime import datetime
from app.models.task import Task

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

Choose a reason for hiding this comment

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

Same comment as above in goal_routes.py, could be named just bp




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

try:
new_task = Task.from_dict(request_body)

except KeyError:
response = {"details": "Invalid data"}
abort(make_response(response, 400))


db.session.add(new_task)
db.session.commit()
Comment on lines +13 to +24

Choose a reason for hiding this comment

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

Same comment as above in goal_routes.py, would prefer this logic to be in a helper function so you're not duplicating very similar code across different files.


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


@tasks_bp.get("")
def get_all_tasks():
query = db.select(Task)
description_param = request.args.get("description")
if description_param:
query = query.where(Task.description.ilike(f"%{description_param}%")).order_by(Task.id)

title_param = request.args.get("title")
sort = request.args.get("sort", "asc") # Default to ascending order

query = db.select(Task)

if title_param:
query = query.where(Task.title.ilike(f"%{title_param}%"))

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

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

return tasks_response, 200


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


return {"task": task.to_dict()}
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.

👍



@tasks_bp.put("/<task_id>")
def update_task(task_id):
task = validate_task(task_id)
request_body = request.get_json()

task.title = request_body["title"]
task.description = request_body.get("description")
task.completed_at = request_body.get("completed_at")

db.session.commit()
response = {"task": task.to_dict()}
return response, 200

@tasks_bp.patch("/<task_id>/mark_complete")
def mark_task_complete(task_id):
task = validate_task(task_id)
task.completed_at = datetime.now()
db.session.commit()

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

Choose a reason for hiding this comment

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

For the sake of time, I went to your repo and found your other branch to look at your Slack implementation. Note: I cannot leave comments on a codebase. That's why we have you open PRs for your projects. In the future, please make sure all of your solution is in your PR.

I'm copy and pasting your helper function here and leaving my review comments as comments in your code

def send_slack_message(task_title):
    # constants should be named using all capital letters
    token = os.environ.get("SLACK_BOT_TOKEN")
    if not token:
        raise ValueError("Slack Bot Token is not set in environment variables.")

    headers = {"Authorization": f"Bearer {token}", "Content-Type": "application/json"}
    payload = {"channel": SLACK_CHANNEL, "text": f"Beautiful task {task_title}"}

    response = requests.post(SLACK_API_URL, headers=headers, json=payload)
    if response.status_code != 200:
        print("Failed to send Slack message:", response.json())
    else:
        print("Slack message sent successfully.")


@tasks_bp.patch("/<task_id>/mark_incomplete")
def mark_task_incomplete(task_id):
task = validate_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_task(task_id)

db.session.delete(task)
db.session.commit()
response = {
"details": f'Task {task_id} "{task.title}" successfully deleted'
}
return response, 200

def validate_task(task_id):
try:
task_id = int(task_id)
except ValueError:
response = {"details": f"Task {task_id} invalid"}
abort(make_response(response, 404))

task = db.session.get(Task,task_id)

Choose a reason for hiding this comment

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

This correctly uses the new syntax (instead of the deprecated syntax <model>.query.get.

However, db.session.get returns an object and db.session.scalar(query) returns a single value.

In our curriculum, we prefer scalars, you can read more here in Learn or review SqlAlchemy documentation.


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

return task
Loading