Skip to content

Snow Leopards | Anika SW #142

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: master
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
5 changes: 5 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,10 @@ def create_app(test_config=None):
migrate.init_app(app, db)

# Register Blueprints here
from .task_routes import tasks_bp
app.register_blueprint(tasks_bp)

from .goal_routes import goals_bp
app.register_blueprint(goals_bp)

return app
156 changes: 156 additions & 0 deletions app/goal_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
from flask import Blueprint, jsonify, abort, make_response, request
from app import db
from app.models.goal import Goal
from app.models.task import Task

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

Choose a reason for hiding this comment

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

This can just be called bp since you've only got one blueprint in this file and you have routes for each model in their own file.


def validate_complete_request(request_body):

Choose a reason for hiding this comment

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

Nice error handling here, we can't expect users to always give us valid data and need to handle those scenarios afccordingly

try:
if request_body["title"]:
return request_body

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


def validate_model_id(cls, model_id):
try:
model_id = int(model_id)
except:
abort(make_response({"details": "Invalid data"}, 404))

model = cls.query.get(model_id)

if not model:
abort(make_response({"details": "Invalid data"}, 404))

return model


@goals_bp.route("", methods=["POST"])
def create_goal():
request_body = request.get_json()
valid_data = validate_complete_request(request_body)
new_goal = Goal.from_dict(valid_data)

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

goal_response = {
"goal": new_goal.to_dict()
}
return make_response(jsonify(goal_response), 201)


@goals_bp.route("", methods=["GET"])
def get_all_goals_sort_asc():
goal_query = Goal.query.all()
title_query = request.args.get("title")
if title_query:
goal_query = Goal.query.filter_by(title=title_query)
Comment on lines +50 to +51

Choose a reason for hiding this comment

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

👍


goals = goal_query

goals_response = [goal.to_dict() for goal in goals]

return make_response(jsonify(goals_response), 200)


@goals_bp.route("/<goal_id>", methods=["GET"])
def get_one_goal(goal_id):
goal = validate_model_id(Goal, goal_id)

goal_response = {
"goal": goal.to_dict()
}

return make_response(jsonify(goal_response), 200)

@goals_bp.route("/<goal_id>", methods=["PUT"])
def goal_update_entire_entry(goal_id):
goal = validate_model_id(Goal, goal_id)
request_body = request.get_json()
goal.title = request_body["title"]
Comment on lines +73 to +74

Choose a reason for hiding this comment

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

You could run into an error on line 74 if a request was sent without a key "title". You'll want to have logic in PUT or PATCH methods (any route that takes in a request body) to guard against invalid data like you did in your POST request on line 34


db.session.commit()

goal_response = {
"goal": goal.to_dict()
}

return make_response((goal_response), 200)


@goals_bp.route("/<goal_id>", methods=["DELETE"])
def goal_delete(goal_id):
goal = validate_model_id(Goal, goal_id)

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

return make_response({'details': f'Goal {goal.id} "{goal.title}" successfully deleted'}, 200)

@goals_bp.route("/<goal_id>/tasks", methods=["POST"])
def add_tasks_to_goal(goal_id):
goal = validate_model_id(Goal, goal_id)
request_body = request.get_json()

for task_id in request_body["task_ids"]:
task = validate_model_id(Task, task_id)
goal.tasks.append(task)
db.session.commit()

goal_response = {
"id": goal.id,
"task_ids": request_body["task_ids"]
}


return make_response(jsonify(goal_response), 200)


@goals_bp.route("/<goal_id>/tasks", methods=["GET"])
def get_goal_with_tasks(goal_id):
goal = validate_model_id(Goal, goal_id)

tasks = []
for task in goal.tasks:
tasks.append(

Choose a reason for hiding this comment

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

Consider validating task before appending it to tasks, like:

for task in goal.tasks:
        task = validate_model_id(Task, task.id)
        tasks.append(task)

then you don't need to write out the dictionary literal that represents a task because validate_model_id returns an instance of task.

{
"id": task.id,
"goal_id": task.goal_id,
"title": task.title,
"description": task.description,
"is_complete": False

Choose a reason for hiding this comment

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

If we're getting all tasks associated with an goal, do we need to hard code is_complete to False? Could a goal have a list of tasks where some are completed and some are not?

}
)

goal_response = {
"id": goal.id,
"title": goal.title,
"tasks": tasks
}

return make_response(jsonify(goal_response), 200)

@goals_bp.route("<goal_id>/tasks/<task_id>", methods=["GET"])
def get_one_task(goal_id, task_id):
goal = validate_model_id(Goal, goal_id)
task = validate_model_id(Task, task_id)

task_response = []
for task in goal.tasks:
task_response.append(
Comment on lines +143 to +144

Choose a reason for hiding this comment

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

If we just want to get a specific task by id, then we don't want to go through all the tasks in goal.tasks because then we'd end up with a list of all the tasks associated with the goals.

This route repeats the logic in task_routes.py on lines 72-79 where you get a task by id. Here you nest the route under goals. It's up to the developer to decide if you want to have the request to get a single task by ID nested under goals/goal_id/tasks/task_id or if you can just a route that is /tasks/task_id

{
"task": {
"id": task.id,
"goal_id": task.goal_id,
"title": task.title,
"description": task.description,
"is_complete": False
}
}
)

return make_response(jsonify(task_response), 200)
18 changes: 17 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,20 @@


class Goal(db.Model):
goal_id = db.Column(db.Integer, primary_key=True)
id = db.Column(db.Integer, primary_key=True, autoincrement=True)
title = db.Column(db.String)
tasks = db.relationship("Task", back_populates="goal", lazy=True)


def to_dict(self):
return {
"id": self.id,
"title": self.title,
}

@classmethod
def from_dict(cls, goal_data):
new_goal = Goal(
title=goal_data["title"],
)
return new_goal
Comment on lines +18 to +21

Choose a reason for hiding this comment

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

2 notes:

  • You can use the cls keywork here instead of Goal
  • You can also directly return a dictionary literal without creating the new_goal variable:
return cls(
    title = dict_data["title"],
)

59 changes: 58 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,61 @@


class Task(db.Model):
task_id = db.Column(db.Integer, primary_key=True)
id = db.Column(db.Integer, primary_key=True, autoincrement=True)
title = db.Column(db.String)
description = db.Column(db.String)
completed_at = db.Column(db.DateTime, nullable=True)
goal_id = db.Column(db.Integer, db.ForeignKey('goal.id'), nullable=True)
goal = db.relationship("Goal", back_populates="tasks")


def to_dict_post_put(self):
return {
"id": self.id,
"title": self.title,
"description": self.description,
"is_complete": False
}

def to_dict_get_patch(self):
if not self.completed_at:
if self.goal_id:
return {
"id": self.id,
"goal_id": self.goal_id,
"title": self.title,
"description": self.description,
"is_complete": False
}
else:
return {
"id": self.id,
"title": self.title,
"description": self.description,
"is_complete": False
}
else:
if self.goal_id:
return {
"id": self.id,
"goal_id": self.goal_id,
"title": self.title,
"description": self.description,
"is_complete": False
}
else:
return {
"id": self.id,
"title": self.title,
"description": self.description,
"is_complete": True
}

Choose a reason for hiding this comment

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

Notice in to_dict_get_patch, you have several lines of code repeated in each dictionary literal you're returning. What if you used a variable and built up a dictionary using conditional logic and return the dictionary at the end of the function to avoid repetition?

This example flips the logic a little bit but you can see how we build up updated_task so we don't need to repeat a bunch of dictionary literals.

updated_task = {
    "id": self.id,
    "title": self.title,
    "description": self.description
}

if not self.completed_at:
    updated_task["is_complete"]: False
    if self.goal_id:    
        updated_task["goal_id"]: self.goal_id
 
# more logic here to add whatever key/value pairs are needed for updated_task

    return updated_task


@classmethod
def from_dict(cls, task_data):
new_task = Task(
title=task_data["title"],
description=task_data["description"],
completed_at=None
)
return new_task
1 change: 0 additions & 1 deletion app/routes.py

This file was deleted.

130 changes: 130 additions & 0 deletions app/task_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
from flask import Blueprint, jsonify, abort, make_response, request
from datetime import datetime
from app import db
from app.models.task import Task

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

def validate_complete_request(request_body):
try:
if request_body["title"] and request_body["description"]:
return request_body

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


def validate_model_id(cls, model_id):
try:
model_id = int(model_id)
except:
abort(make_response({"details": "Invalid data"}, 404))

model = cls.query.get(model_id)

if not model:
abort(make_response({"details": "Invalid data"}, 404))

return model


@tasks_bp.route("", methods=["POST"])
def create_task():
request_body = request.get_json()
valid_data = validate_complete_request(request_body)
new_task = Task.from_dict(valid_data)

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

task_response = {
"task": new_task.to_dict_post_put()
}
return make_response(jsonify(task_response), 201)


@tasks_bp.route("", methods=["GET"])
def get_all_tasks_sort_asc():
task_query = Task.query.all()

Choose a reason for hiding this comment

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

Here, we should have task_query = Task.query without all() because we dont actually want to go query everything yet. We want to see which request params were provided to the request and then filter or order accordingly. Finally if no request params were provided then we'd want to query for all tasks near the end of this method.

title_query = request.args.get("title")
description_query = request.args.get("description")
completed_at_query = request.args.get("completed at")
sort_query = request.args.get("sort")
if title_query:
task_query = Task.query.filter_by(title=title_query)
if description_query:
task_query = Task.query.filter_by(description=description_query)
if completed_at_query:
task_query = Task.query.filter_by(completed_at=completed_at_query)
if sort_query == "asc":
task_query = Task.query.order_by(Task.title.asc())
if sort_query == "desc":
task_query = Task.query.order_by(Task.title.desc())

tasks = task_query

Choose a reason for hiding this comment

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

Here is where we'd want to query for all tasks if not query params were provided so line 64 should be

tasks = task_query.all()
# rest of logic 


tasks_response = [task.to_dict_get_patch() for task in tasks]

return make_response(jsonify(tasks_response), 200)


@tasks_bp.route("/<task_id>", methods=["GET"])
def get_one_task(task_id):
task = validate_model_id(Task, task_id)

task_response = {
"task": task.to_dict_get_patch()
}

return make_response(jsonify(task_response), 200)

@tasks_bp.route("/<task_id>", methods=["PUT"])
def task_update_entire_entry(task_id):
task = validate_model_id(Task, task_id)
request_body = request.get_json()
task.title = request_body["title"]
task.description = request_body["description"]

db.session.commit()

task_response = {
"task": task.to_dict_post_put()
}

return make_response((task_response), 200)

@tasks_bp.route("/<task_id>/mark_complete", methods=["PATCH"])
def task_mark_complete(task_id):
task = validate_model_id(Task, task_id)
task.completed_at = datetime.now()
db.session.commit()

task_response = {
"task": task.to_dict_get_patch()
}

return make_response((task_response), 200)


@tasks_bp.route("/<task_id>/mark_incomplete", methods=["PATCH"])
def task_mark_incomplete(task_id):
task = validate_model_id(Task, task_id)
task.completed_at = None

db.session.commit()

task_response = {
"task": task.to_dict_get_patch()
}

return make_response((task_response), 200)


@tasks_bp.route("/<task_id>", methods=["DELETE"])
def task_delete(task_id):
task = validate_model_id(Task, task_id)

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

return make_response({'details': f'Task {task.id} "{task.title}" successfully deleted'}, 200)
1 change: 1 addition & 0 deletions migrations/README
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Generic single-database configuration.
Loading