Skip to content

Sea Turtles - Esther Annorzie #96

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 29 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c359154
added Task attributes, registered Blueprint
estherannorzie May 6, 2022
da8b4b9
started working on create_task POST route
estherannorzie May 7, 2022
522cdb1
fixed Task indentation
estherannorzie May 7, 2022
f82647d
passed create_task test
estherannorzie May 7, 2022
ff50e45
created GET route
estherannorzie May 7, 2022
355fbb5
completed GET route
estherannorzie May 9, 2022
1c54fb7
passed test_delete_task
estherannorzie May 10, 2022
807ad61
changed some returns for routes
estherannorzie May 10, 2022
533d1bd
changed wave 1 assertions
estherannorzie May 10, 2022
6631836
fixed instance method and PUT route
estherannorzie May 10, 2022
507e35f
refacrored GET route to pass all tests
estherannorzie May 10, 2022
f00d20d
added query params to GET route
estherannorzie May 11, 2022
c0a44b4
refactored POST route
estherannorzie May 11, 2022
8321582
added message helper function, ins and cls method to Task
estherannorzie May 11, 2022
6d83c97
created starter code for PATCH route
estherannorzie May 11, 2022
84e7a21
created task completion functions
estherannorzie May 12, 2022
f14d1ad
refactored from_dict
estherannorzie May 12, 2022
99e989c
created routes_helper for model
estherannorzie May 12, 2022
a511f84
added goal routes, modified goal attributes
estherannorzie May 12, 2022
c015c33
modified goal routes
estherannorzie May 12, 2022
62c5100
added GET routes for goals
estherannorzie May 12, 2022
77e105c
created assertions for wave 5
estherannorzie May 13, 2022
9250b1b
updated Goal and Task models
estherannorzie May 13, 2022
582d476
refactored create_message
estherannorzie May 13, 2022
15a79a8
completed POST route for adding tasks to goals
estherannorzie May 13, 2022
bf93f05
added assertion for getting goals with no tasks
estherannorzie May 13, 2022
f3a7cf3
finished project
estherannorzie May 13, 2022
2758c5d
added Procfile
estherannorzie May 13, 2022
ce9e220
Refactored Models and routes
estherannorzie May 20, 2022
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
1 change: 1 addition & 0 deletions Procfile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
web: gunicorn 'app:create_app()'
2 changes: 1 addition & 1 deletion ada-project-docs/wave_01.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ As a client, I want to be able to make a `GET` request to `/tasks/1` when there

As a client, I want to be able to make a `PUT` request to `/tasks/1` when there is at least one saved task with this request body:

```json
```jso

Choose a reason for hiding this comment

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

As we build our skills, something to fold into our process should be reviewing our code when we open PRs to make sure we haven't made unintended changes to other files.

{
"title": "Updated Task Title",
"description": "Updated Test Description",
Expand Down
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 bp
app.register_blueprint(bp)

from .goal_routes import bp
app.register_blueprint(bp)
Comment on lines +33 to +37

Choose a reason for hiding this comment

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

Great choice to divide the routes into their own files!


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


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


@bp.route("", methods=("POST",))
def create_goal():
request_body = request.get_json()
if "title" not in request_body:
abort_message("Missing title", 400)

goal = Goal.from_dict(request_body)
db.session.add(goal)
db.session.commit()

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


@bp.route("/<goal_id>/tasks", methods=("POST",))
def add_tasks_to_goal(goal_id):
goal = get_record_by_id(goal_id)
request_body = request.get_json()

tasks_list = []
for task_id in request_body["task_ids"]:
task = get_record_by_id(task_id)
task.goal = goal
tasks_list.append(task.task_id)

db.session.commit()
return make_response(jsonify({"id": goal.goal_id,
"task_ids": tasks_list}))
Comment on lines +36 to +37
Copy link

@kelsey-steven-ada kelsey-steven-ada May 16, 2022

Choose a reason for hiding this comment

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

Nice line wrapping! Though it takes more lines, another way we could format this to help the structure be really clear at a quick glance might be:

    return make_response(
        jsonify({
            "id": goal.goal_id, 
            "task_ids": tasks_list
        })
    )



@bp.route("/<goal_id>", methods=("GET",))
def read_goal(goal_id):
goal = get_record_by_id(goal_id)
return make_response(jsonify({"goal": goal.to_dict()}))

Choose a reason for hiding this comment

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

Notice the pattern of calling make_response(jsonify(...) used by many of the functions in goal_routes.py and task_routes.py. We could make another helper function in routes_helper.py that takes an optional response code and use it across all these functions, similar to the create_message helper function that handles abort(...) calls.

def get_success_response(response_data, response_code = 200):
    return make_response(jsonify(response_data), response_code)



@bp.route("", methods=("GET",))
def read_all_goals():
goals = Goal.query.all()
goals_response = [goal.to_dict() for goal in goals]
return make_response(jsonify(goals_response))


@bp.route("/<goal_id>/tasks", methods=("GET",))
def read_specific_goal_tasks(goal_id):
goal = get_record_by_id(goal_id)

goal_tasks = [Task.to_dict(task) for task in goal.tasks]

Choose a reason for hiding this comment

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

Great use of list comprehensions across the code!


return make_response(jsonify({
"id": goal.goal_id,
"title": goal.title,
"tasks": goal_tasks})
)


@bp.route("/<goal_id>", methods=("PUT",))
def replace_goal(goal_id):
goal = get_record_by_id(Goal, goal_id)
request_body = request.get_json()
if "title" not in request_body:
abort_message("Title not found", 404)
goal.override_goal(request_body)
Copy link

@kelsey-steven-ada kelsey-steven-ada May 16, 2022

Choose a reason for hiding this comment

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

Right now this function returns a 500 error without a specific error message if the user forgets or misspells the "title" key in the PUT request, because of the KeyError that will be generated by the override_goal function. In a deployed application, we'd probably want to catch the KeyError and return an error message specific to what went wrong, to help the user troubleshoot.

This feedback applies to where the Goal.from_dict function is used as well.

db.session.commit()

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


@bp.route("/<goal_id>", methods=("DELETE",))
def delete_goal(goal_id):
goal = get_record_by_id(goal_id)
db.session.delete(goal)
db.session.commit()

abort_message(f'Goal {goal_id} "{goal.title}" successfully deleted')
20 changes: 20 additions & 0 deletions app/models/goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,23 @@

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


@classmethod
def from_dict(cls, data_dict):
return cls(
title=data_dict["title"]
)


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


def override_goal(self, data_dict):
self.title = data_dict["title"]
32 changes: 32 additions & 0 deletions app/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,35 @@

class Task(db.Model):
task_id = db.Column(db.Integer, primary_key=True)
title = db.Column(db.String)
description = db.Column(db.String)
completed_at = db.Column(db.DateTime, nullable=True, default=None)

Choose a reason for hiding this comment

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

Nice choices to ensure completed_at doesn't have to exist, and if it hasn't been defined we know it will hold None!

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


def to_dict(self):
response_dict = dict(
id=self.task_id,
title=self.title,
description=self.description,
is_complete=self.completed_at is not None
)

if self.goal_id:
response_dict["goal_id"] = self.goal_id

return response_dict

# create similar functions for task and goal, class for creation
@classmethod
def from_dict(cls, data_dict):
return cls(
title=data_dict["title"],
description=data_dict["description"],
completed_at = data_dict.get("completed_at", None)
)

def override_task(self, data_dict):
self.title = data_dict["title"]
self.description = data_dict["description"]
1 change: 0 additions & 1 deletion app/routes.py

This file was deleted.

46 changes: 46 additions & 0 deletions app/routes_helper.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
from flask import make_response, abort
from app.models.task import Task
from app.models.goal import Goal


def get_task_by_id(task_id):
try:
task_id = int(task_id)
except:
abort_message("Invalid data", 400)

task = Task.query.get(task_id)

if not task:
abort_message("Task not found", 404)
return task


def get_goal_by_id(goal_id):
try:
goal_id = int(goal_id)
except:
abort_message("Invalid data", 400)

goal = Goal.query.get(goal_id)

if not goal:
abort_message("Goal not found", 404)
return goal


def get_record_by_id(cls, record_id):
try:
record_id = int(record_id)
except:
abort_message("Invalid data", 400)

record = cls.query.get(record_id)

if not record:
abort_message(f"{type(cls).__name__} not found", 404)
return record


def abort_message(details_info, status_code=200):
abort(make_response({"details": details_info}, status_code))
93 changes: 93 additions & 0 deletions app/task_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
from flask import Blueprint, jsonify, make_response, request
from app.models.task import Task
from .routes_helper import abort_message, get_record_by_id, create_message
from app import db
from datetime import datetime
import requests, os


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


@bp.route("", methods=("POST",))
def create_task():
request_body = request.get_json()
if "title" not in request_body or "description" not in request_body:
create_message("Title or description missing", 400)
task = Task.from_dict(request_body)
db.session.add(task)
db.session.commit()

return make_response(jsonify({"task": task.to_dict()}), 201)


@bp.route("/<task_id>", methods=("GET",))
def read_one_task(task_id):
task = get_record_by_id(task_id)
return make_response(jsonify({"task": task.to_dict()}))


@bp.route("", methods=("GET",))
def read_all_tasks():
title_query = request.args.get("sort")

if title_query == "asc":
tasks = Task.query.order_by(Task.title.asc())
elif title_query == "desc":
tasks = Task.query.order_by(Task.title.desc())
else:
tasks = Task.query.all()

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

return make_response(jsonify(tasks_response))


@bp.route("/<task_id>", methods=("PUT",))
def replace_task(task_id):
task = get_record_by_id(task_id)
request_body = request.get_json()
if "title" not in request_body:
abort_message("Title missing", 404)
if "description" not in request_body:
abort_message("Description missing", 404)

task.override_task(request_body)

Choose a reason for hiding this comment

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

The feedback on line 68 of goal_routes.py around 500 errors and returning more descriptive error messages applies here as well.

db.session.commit()

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


@bp.route("/<task_id>", methods=("DELETE",))
def delete_task(task_id):
task = get_record_by_id(task_id)
db.session.delete(task)
db.session.commit()

create_message(f'Task {task_id} "{task.title}" successfully deleted')


@bp.route("/<task_id>/mark_complete", methods=("PATCH",))
def mark_task_complete(task_id):
task = get_record_by_id(task_id)
task.completed_at = datetime.utcnow()
db.session.commit()

requests.post(
url="https://slack.com/api/chat.postMessage",
data={
"channel": "task-notifications",
"text": f"Someone just completed the task {task.title}"
},
headers={"Authorization": os.environ.get("token")},
)
Comment on lines +76 to +83

Choose a reason for hiding this comment

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

Nicely organized post request!


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


@bp.route("/<task_id>/mark_incomplete", methods=("PATCH",))
def mark_task_incomplete(task_id):
task = get_record_by_id(task_id)
task.completed_at = None
db.session.commit()
return jsonify({"task": task.to_dict()})
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.
45 changes: 45 additions & 0 deletions migrations/alembic.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# 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

[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

[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