-
Notifications
You must be signed in to change notification settings - Fork 44
Phoenix (C22)- Jenny M #24
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
base: main
Are you sure you want to change the base?
Changes from all commits
752ab93
4b53ff0
fc7cd02
8350622
8f45e33
15cc755
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,13 @@ | ||
from flask import Flask | ||
from .db import db, migrate | ||
from .models import task, goal | ||
from .routes.task_routes import tasks_bp | ||
import os | ||
from dotenv import load_dotenv | ||
from .routes.goal_routes import goals_bp | ||
|
||
load_dotenv() | ||
SLACKBOT_TOKEN = os.environ.get("SLACKBOT_TOKEN") | ||
Comment on lines
+9
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because we aren't using the Slackbot token in this file, we don't need to bring it in here! |
||
|
||
def create_app(config=None): | ||
app = Flask(__name__) | ||
|
@@ -18,5 +24,9 @@ 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 |
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] | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just make sure to remove unnecessary empty lines like this in the PR! |
||
tasks: Mapped[list["Task"]] = relationship(back_populates="goal") | ||
|
||
@classmethod | ||
def from_dict(cls, task_data): | ||
return cls( | ||
title=task_data["title"] | ||
) |
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 datetime import datetime | ||
from ..db import db | ||
from sqlalchemy import ForeignKey | ||
from typing import Optional | ||
from app.models.goal import Goal | ||
from app.routes.route_utilities import check_complete | ||
|
||
class Task(db.Model): | ||
|
||
__tablename__ = 'tasks' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't seem as though you are using tablename anywhere else so we don't need to include this! |
||
|
||
id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) | ||
title: Mapped[str] | ||
description: Mapped[str] | ||
completed_at: Mapped[datetime | None] = mapped_column(nullable=True) | ||
|
||
goal_id: Mapped[Optional[int]] = mapped_column(ForeignKey("goal.id")) | ||
goal: Mapped[Optional["Goal"]] = relationship(back_populates="tasks") | ||
|
||
@classmethod | ||
def from_dict(cls, task_data): | ||
new_task = cls(title=task_data["title"], description=task_data["description"], completed_at=task_data["completed_at"]) | ||
return new_task | ||
|
||
def to_dict(self): | ||
return dict( | ||
id=self.id, | ||
goal_id=self.goal_id, | ||
title=self.title, | ||
description=self.description, | ||
is_complete=check_complete(self.completed_at) | ||
Comment on lines
+26
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While it feels very complete to return the dictionary in its entirety here, the goal_id is an optional attribute. When you have an optional attribute, you should only add it to the returned dictionary if it exists! This is a simple conditional that we can build in here! Also, I see where you are coming from with the check_complete helper function. My one concern is that this is coming from the routes utilities in the routes directory. While it is not forbidden to use helper methods from the tasks in your models, it can get confusing. You could use a ternary instead to ascertain the truthiness of is_complete. |
||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,113 @@ | ||
from flask import Blueprint | ||
from flask import Blueprint, request, abort, make_response | ||
from app.models.goal import Goal | ||
from app.routes.route_utilities import validate_model | ||
from ..db import db | ||
from app.models.task import Task | ||
goals_bp = Blueprint("goals_bp", __name__, url_prefix="/goals") | ||
|
||
@goals_bp.post("") | ||
def create_goal(): | ||
|
||
request_body = request.get_json() | ||
|
||
if "title" not in request_body: | ||
return {"details": "Invalid data"}, 400 | ||
|
||
new_goal = Goal(title=request_body["title"]) | ||
Comment on lines
+13
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a good check. One way we could actually combine it with the creation of the new goal would be to use a try/except block. This might look something like this:
Notice that we've also made use of the Goal model's from_dict method here! |
||
db.session.add(new_goal) | ||
db.session.commit() | ||
|
||
response = { | ||
"goal": { | ||
"id": new_goal.id, | ||
"title": new_goal.title | ||
} | ||
} | ||
Comment on lines
+20
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed you don't have a to_dict method for your Goal model. This code seems like it will probably be repeated, so a |
||
|
||
return response, 201 | ||
|
||
@goals_bp.get("") | ||
def get_all_goals(): | ||
|
||
query = db.select(Goal) | ||
|
||
goals = db.session.scalars(query) | ||
|
||
goals_response = [] | ||
for goal in goals: | ||
goals_response.append( | ||
{ | ||
"id": goal.id, | ||
"title": goal.title, | ||
} | ||
) | ||
Comment on lines
+37
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This all looks great! Always feel free to see if you can build this list using comprehension! |
||
|
||
return goals_response | ||
|
||
@goals_bp.get("/<goals_id>") | ||
def get_one_goal(goals_id): | ||
goal = validate_model(Goal, goals_id) | ||
query = db.select(Goal).where(Goal.id == goals_id) | ||
goal = db.session.scalar(query) | ||
Comment on lines
+50
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate_model makes these calls for us, so we don't need to make them again! |
||
return { | ||
"goal": { | ||
"id": goal.id, | ||
"title": goal.title, | ||
} | ||
} | ||
Comment on lines
+52
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is another place where the to_dict may come in handy! |
||
|
||
@goals_bp.put("/<goals_id>") | ||
def update_goal(goals_id): | ||
goal = validate_model(Goal, goals_id) | ||
request_body = request.get_json() | ||
|
||
goal.title = request_body['title'] | ||
|
||
db.session.commit() | ||
|
||
return { | ||
"goal": { | ||
"id": goal.id, | ||
"title": goal.title, | ||
} | ||
Comment on lines
+69
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above! The to_dict for the goal model could have come in handy here. |
||
} | ||
|
||
@goals_bp.delete("/<goals_id>") | ||
def delete_goal(goals_id): | ||
goal = validate_model(Goal, goals_id) | ||
db.session.delete(goal) | ||
db.session.commit() | ||
|
||
return {"details": f"Goal {goals_id} \"{goal.title}\" successfully deleted"} | ||
|
||
@goals_bp.post("/<goals_id>/tasks") | ||
def create_task_list_for_goal(goals_id): | ||
|
||
goal = validate_model(Goal, goals_id) | ||
|
||
request_body = request.get_json() | ||
task_ids = request_body.get("task_ids", []) | ||
|
||
tasks = Task.query.filter(Task.id.in_(task_ids)).all() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks good. We could also use the validate_model function to get each Task from its id and append them to the goal's list of Tasks! |
||
|
||
goal.tasks = tasks | ||
db.session.commit() | ||
|
||
return { | ||
"id": goal.id, | ||
"task_ids": task_ids | ||
} | ||
|
||
@goals_bp.get("/<goals_id>/tasks") | ||
def get_tasks_by_goal(goals_id): | ||
goal = validate_model(Goal, goals_id) | ||
tasks = [task.to_dict() for task in goal.tasks] | ||
return { | ||
"id": goal.id, | ||
"title": goal.title, | ||
"tasks": tasks | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perfect! |
||
|
||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
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} not found"} | ||
abort(make_response(response, 404)) | ||
|
||
return model | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks great! |
||
|
||
def check_complete(completed_at): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the idea behind this route utility! |
||
if not completed_at: | ||
return False | ||
else: | ||
return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to get in the habit of naming all our blueprints
bp
within their routes and aliasing them within the init.py