Skip to content

Task List, C17 Sea Turtles, Shelby Faulconer #97

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 12 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
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()'
4 changes: 4 additions & 0 deletions ada-project-docs/wave_04.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,13 @@ Visit https://api.slack.com/methods/chat.postMessage to read about the Slack API
Answer the following questions. These questions will help you become familiar with the API, and make working with it easier.

- What is the responsibility of this endpoint?
## post to slack chat
- What is the URL and HTTP method for this endpoint?
## POST https://slack.com/api/chat.postMessage
- What are the _two_ _required_ arguments for this endpoint?
## api token and channel and at least onen: attachment, block, or text
- How does this endpoint relate to the Slackbot API key (token) we just created?
## slackbot api key is required to authorize the post request

Now, visit https://api.slack.com/methods/chat.postMessage/test.

Expand Down
4 changes: 4 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ def create_app(test_config=None):
db.init_app(app)
migrate.init_app(app, db)

from .routes import tasks_bp

Choose a reason for hiding this comment

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

We would probably want to rename routes.py to something more descriptive like task_routes.py. As our projects grow in size, specificity becomes very important to helping us find resources.

from .goal_routes import goals_bp
app.register_blueprint(tasks_bp)
app.register_blueprint(goals_bp)
Comment on lines +34 to +35

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!

# Register Blueprints here

return app
133 changes: 133 additions & 0 deletions app/goal_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
# import json
# import os
# import datetime
# from pandas import json_normalize
# import requests
# from urllib import response
from flask import Blueprint, jsonify, abort, make_response, request
from sqlalchemy import desc

Choose a reason for hiding this comment

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

It looks like the import desc isn't used in goal_routes.py, so we should remove it as part of our code cleanup and refactor steps.

from app import db
from app.models.goal import Goal
from app.models.task import Task
from app.routes import get_task, get_task_by_id, update_task_by_id
from .helper_routes import get_record_by_id


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

##CRUD
##CREATE : POST
@goals_bp.route("", methods=["POST"])
def manage_post_goals():

Choose a reason for hiding this comment

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

To me, manage_post_goals doesn't feel very specific about what work this function performs. What's a name that reflects the effects of the function, so someone wouldn't need to read the function internals to know what it does?

request_body = request.get_json()
title = request_body.get("title")

if title is None:
response_body = make_response(jsonify({"details": "Invalid data"}), 400)
return response_body
Comment on lines +26 to +27

Choose a reason for hiding this comment

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

Since we don't perform actions with response_body before returning it, I'd recommend returning on the same line rather than creating a variable. If the goal is to break things up for readability, I'd consider breaking out the message to a variable:

response_body = {"details": "Invalid data"}
return make_response(jsonify(response_body), 400)

We might also want to give the user more information to help them debug. It could be as little as adding what key was missing:

response_body = {"details": "Invalid data: missing 'title'"}



goals = Goal.from_dict(request_body)


Comment on lines +31 to +32

Choose a reason for hiding this comment

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

To be consistent with whitespace inside functions, I recommend removing one of the new lines both here and above goal.

db.session.add(goals)
db.session.commit()

return jsonify({"goal":goals.to_dictionary()}), 201

#READ : GET
@goals_bp.route("", methods=["GET"])
def manage_get_goals():
goals = Goal.query.all()
goal_dictionary = [goal.to_dictionary() for goal in goals]

return jsonify(goal_dictionary)



Comment on lines +45 to +47

Choose a reason for hiding this comment

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

One of the things to look at when we are in our code clean up phase is whitespace across a file. The general guideline is one newline between functions inside of a class, but what's most important is to be consistent across a codebase. A little extra whitespace is often used as a visual separation between groups of related functions, or to make it clear where imports end and implementation code begins. We lose the ability to have a visual separation have meaning if we are not consistent with how they are applied. The PEP 8 style guide has a little more info on their recommendations: https://peps.python.org/pep-0008/#blank-lines

@goals_bp.route("/<id>", methods=["GET"])
def get_goal_by_id(id):
goal = get_goal(id)
# response_body = dict()
# response_body = goal.to_dictionary()
Comment on lines +51 to +52

Choose a reason for hiding this comment

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

We want to make sure to delete commented out code as part of clean up. When we are sharing a codebase with other folks, it's unclear what the intention of commented out code is, especially without extra comments explaining why it's there. We use versioning tools like git so we can confidently delete code and view our commit history to recover code if we ever need to revert back to something we had prior.

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


#UPDATE : PUT
@goals_bp.route("/<id>", methods=["PUT"])
def update_goal_by_id(id):
goal = get_goal(id)

request_body = request.get_json()

goal.title = request_body["title"]

db.session.commit()

return get_goal_by_id(id)


#DELETE : DELETE
@goals_bp.route("/<id>", methods=["DELETE"])
def delete_goal_by_id(id):
goal = get_goal(id)

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

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



#### FRIENDSHIP ROUTEs

Choose a reason for hiding this comment

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

I love friendship routes as a name for nested routes 😄

@goals_bp.route("/<id>/tasks", methods=["POST"])
def post_tasks_for_goal(id):
goal = get_goal(id)
request_body = request.get_json()

# print("~~~RQ_BOD", request_body)
task_ids = request_body["task_ids"]
# print(task_ids)

for task_id in task_ids:
task = get_task(task_id)
task.goal = goal
update_task_by_id(task_id)

db.session.commit()

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


@goals_bp.route("/<id>/tasks", methods=["GET"])
def get_tasks_for_goal(id):
goal = get_record_by_id(Goal, id)
tasks = Task.query.filter_by(id=goal.id)
# print("*******TASKS******",tasks)

Choose a reason for hiding this comment

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

We should remove debugging print statements from our code as part of our clean up. This feedback applies to lines 88 & 90 as well.

tasks_info = [task.to_dictionary_with_goal() for task in tasks]
response_body = {
"id" : goal.id,
"title" : goal.title,
"tasks" : tasks_info
}

return jsonify(response_body)

## no request body ?
## response body returns {goal{tasks}}
Comment on lines +116 to +117

Choose a reason for hiding this comment

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

It's a little unclear what these comments relate to, we probably would want to remove them as part of our clean up steps or relocate the comments so they are with the code they reference.




## HELPER FUNCTIONS:
def get_goal(id):

Choose a reason for hiding this comment

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

Since you have the function get_record_by_id below, we should be able to remove this function get_goal and replace the calls to it with calls to get_record_by_id. This would also let us remove the abort import from this file, since we wouldn't reference it here anymore.

try:
goal_id = int(id)
except ValueError:
abort(make_response(jsonify({"message": f"goal {id} invalid"}), 400))

goal = Goal.query.get(goal_id)

if not goal:
abort(make_response(jsonify({"message": f"goal {id} not found"}), 404))
else:
return goal
14 changes: 14 additions & 0 deletions app/helper_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from flask import jsonify, abort, make_response

def get_record_by_id(cls, id):

Choose a reason for hiding this comment

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

Great use of a helper function that works with multiple classes!

try:
id = int(id)
except ValueError:
abort(make_response(jsonify({"message": f"Invalid id: {id} "}), 400))

model = cls.query.get(id)

if not model:
abort(make_response(jsonify({"message": f"task {id} not found"}), 404))
else:
return model
23 changes: 22 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,25 @@


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, nullable=False)
tasks = db.relationship("Task", back_populates="goal")

def to_dictionary(self):
# if self.completed_at:
# completed = True
# else:
# completed = False
response = {
"id" : self.id,
"title" : self.title
# "tasks" : self.tasks if self.tasks else None
}
if self.tasks:
response["tasks"] = self.tasks
Comment on lines +19 to +20

Choose a reason for hiding this comment

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

Nice way to add the tasks if they exist without duplicating code!


return response

@classmethod
def from_dict(cls, data_dict):
return Goal(title=data_dict["title"])
48 changes: 47 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,51 @@
# from urllib import response
# from pandas import describe_option
# from sqlalchemy import true
from app import db


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, nullable=False)
description = db.Column(db.String, nullable=False)
completed_at = db.Column(db.DateTime, default=None)
goal_id = db.Column(db.Integer, db.ForeignKey('goal.id'))
goal = db.relationship("Goal", back_populates="tasks")

def to_dictionary(self):
# if self.completed_at:
# completed = True
# else:
# completed = False
response = {
"id" : self.id,
"title" : self.title,
"description" : self.description,
"is_complete" : True if self.completed_at else False
}
return response

def to_dictionary_with_goal(self):
if self.goal_id is None:
response = {
"id" : self.id,
"title" : self.title,
"description" : self.description,
"is_complete" : True if self.completed_at else False
}
else:
response = {
"id" : self.id,
"title" : self.title,
"goal_id" : self.goal_id,
"description" : self.description,
"is_complete" : True if self.completed_at else False
}
Comment on lines +37 to +43

Choose a reason for hiding this comment

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

The indentation is a little off on the dictionary here. We'd want everything inside the brackets indented one more, and the closing brace should line up with response = { since that's the opening brace associated with it:

else:
    response = {
        "id" : self.id,
        "title" : self.title,
        "goal_id" : self.goal_id,
        "description" : self.description,
        "is_complete" : True if self.completed_at else False
    }

return response
Comment on lines +29 to +44

Choose a reason for hiding this comment

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

You could follow the same pattern used in to_dictionary on the Goal class to remove the redundancy between the if/else blocks:

        response = {
            "id" : self.id,
            "title" : self.title,
            "description" : self.description,
            "is_complete" : True if self.completed_at else False
        }

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

        return response

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