Skip to content
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

Tigers- Lindsey B. #126

Open
wants to merge 7 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()'
8 changes: 8 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from dotenv import load_dotenv

Choose a reason for hiding this comment

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

Not sure how this got added, it's a duplicate of line 6's import statement. I figure it was just an accidental extension add.

from flask import Flask
from flask_sqlalchemy import SQLAlchemy
from flask_migrate import Migrate
Expand Down Expand Up @@ -29,6 +30,13 @@ def create_app(test_config=None):
db.init_app(app)
migrate.init_app(app, db)


# Register Blueprints here
from app.models.task import Task
from .routes.routes import task_bp, goals_bp

app.register_blueprint(task_bp)
app.register_blueprint(goals_bp)


return app
33 changes: 32 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,36 @@
from app import db
from flask import Blueprint, make_response, request, jsonify, abort, request



class Goal(db.Model):

Choose a reason for hiding this comment

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

Though, I will mention that your base implementation for both your model classes, Task and Goal is very good overall. I like your pattern for writing the update methods, it's a great use of except.

Choose a reason for hiding this comment

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

Aw, I just noticed you didn't get a chance to use the update methods in the routes. Well, just consider this a comment on how you're on the right track!

goal_id = db.Column(db.Integer, primary_key=True)
goal_id = db.Column(db.Integer, primary_key=True, autoincrement=True)
title = db.Column(db.String)
tasks = db.relationship("Task", back_populates="goal")

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

@classmethod
def from_json(cls, req_body):
return cls(
title=req_body['title']
)

def update(self,req_body):
try:
self.title = req_body["title"]
except KeyError as error:
abort(make_response({'message': f"Missing attribute: {error}"}))

def to_new_dict(self):
return {
"title": self.title,
"description": self.description,
"is_complete": self.completed_at != None,
"id": self.task_id,
"goal_id": self.goal_id
}
47 changes: 46 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,50 @@
from app import db
from flask import Blueprint, make_response, request, jsonify, abort, request


class Task(db.Model):
task_id = db.Column(db.Integer, primary_key=True)
task_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, default=None)
goal_id = db.Column(db.Integer, db.ForeignKey('goal.goal_id'))
goal = db.relationship("Goal", back_populates="tasks")

def to_dict(self):
return {
"title": self.title,
"description": self.description,
"is_complete": self.completed_at != None,
"id": self.task_id
}

@classmethod
def from_json(cls, req_body):
return cls(
title=req_body['title'],
description=req_body['description'],
#TODO completed_at=req_body['completed_at']

Choose a reason for hiding this comment

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

Nice use of TODO comments!

)

def update(self,req_body):
try:
self.title = req_body["title"]
self.description = req_body["description"]
self.completed_at = req_body["completed_at"]
except KeyError as error:
abort(make_response({'message': f"Missing attribute: {error}"}))

def __str__(self):
return "Task"

def __repr__(self):
return "Task"

Comment on lines +37 to +42

Choose a reason for hiding this comment

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

This is also good. These type of functions are often overlooked but setting a good value here can be very helpful when debugging in a REPL as well as other cases.

Though, these particular implementation of these aren't the best in that they just print "Task" rather than using the attributes of self to print more information. But even thinking about them at this point is worth praising.

(Also, if you're confused about the difference between __str__ and __repr__, like I was, this GeeksForGeeks page explained it pretty well.)

def to_new_dict(self):
return {
"title": self.title,
"description": self.description,
"is_complete": self.completed_at != None,
"id": self.task_id,
"goal_id": self.goal_id
}
Comment on lines +43 to +50

Choose a reason for hiding this comment

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

It's kind of odd to make a whole separate to_new_dict function here rather than updating to_dict to handle adding the "goal_id" key. I cloned your code to check if there was another reason, but that was the main use of it in your GET /tasks/<task_id> endpoint.

By forcing the call to to_new_dict I did see that a test failed because of the "goal_id" key. But then that's something that can be handled by saving the dictionary to a variable and then adding the"goal_id" key in an if block:

    def to_dict(self):
        new_dict = {
            "title": self.title,
            "description": self.description,
            "is_complete": self.completed_at != None,
            "id": self.task_id,
        }

        if self.goal_id:
            new_dict["goal_id"] = self.goal_id
        
        return new_dict

Plus, this extra function gets copied over to Goal, presumably when you copy-pasted Task to create that class. There's nothing wrong with doing that, I do it constantly, but do make sure to check because the Goal.to_new_dict function does not ever get used, so there is no need for it.

1 change: 0 additions & 1 deletion app/routes.py

This file was deleted.

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

#VALIDATE ID
def validate_id(class_obj,id):
try:
id = int(id)
except:
abort(make_response({"message":f"{id} is an invalid id"}, 400))
query_result = class_obj.query.get(id)
if not query_result:
abort(make_response({"message":f"{id} not found"}, 404))

return query_result

#CREATE TASK
task_bp = Blueprint("Task", __name__, url_prefix="/tasks")
@task_bp.route("", methods=["POST"])
def create_task():
request_body = request.get_json()
if "title" not in request_body or "description" not in request_body:
#TODO later wave will need 'or "completed_at" not in request_body', add above

Choose a reason for hiding this comment

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

It's weird to talk about code you didn't write, but this makes sense assuming we want "completed_at" to be a required parameter. I think that isn't completely necessary because I think most tasks are going to be sent in uncompleted and while we can force users to supply "completed_at": null, if there is no "completed_at" key, we can just treat it like we would that case.

Anyway, mainly wanted to point out this little design choice.

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

new_task = Task.from_json(request_body)

# abort(make_response)

Choose a reason for hiding this comment

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

Writing code is often messy, but in general I'm in favor of leaving commented code out. If it does need to stick around for some reason, I usually add a comment why (usually a TODO comment).

Choose a reason for hiding this comment

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

Won't comment on every instance of this, but there's more of them.

db.session.add(new_task)
db.session.commit()
response_body = {
"task": new_task.to_dict()
}
return make_response(response_body, 201)

# @task_bp.route("", methods=["GET"])
# def read_all_task():
# tasks_response = []
# tasks = Task.query.all()
# for task in tasks:
# tasks_response.append(task.to_dict())
# return jsonify(tasks_response)
##GET ALL TASKS AND SORT TASKS BY ASC & DESC

@task_bp.route("", methods=["GET"])
def read_all_task():
title_sort_query = request.args.get("sort")
if title_sort_query == "asc":
tasks = Task.query.order_by(Task.title.asc())
elif title_sort_query == "desc":
tasks = Task.query.order_by(Task.title.desc())
else:
tasks = Task.query.all()

response = []
for task in tasks:
response.append(task.to_dict())
return jsonify(response)

#GET ONE TASK
@task_bp.route("/<task_id>", methods=["GET"])
def get_one_task(task_id):
task = validate_id(Task, task_id)
if task.goal_id is None:
return {"task": task.to_dict()}
else:
return {"task": task.to_new_dict()}
# response_body = {
# "task": task.to_dict()
# }
# return response_body

#UPDATE TASK
@task_bp.route("/<task_id>", methods=["PUT"])
def update_task(task_id):
task = validate_id(Task, task_id)
request_body = request.get_json()
task.title = request_body["title"]
task.description = request_body["description"]
# TODO task.completed_at = request_body["completed_at"] #include later
# task.update(request_body)
db.session.commit()
response_body = {
"task": task.to_dict()
}
return make_response(response_body, 200)

#DELETE ONE TASK
@task_bp.route("/<task_id>", methods=["DELETE"])
def delete_task(task_id):
task = validate_id(Task, task_id)

task_dict = task.to_dict()

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

return {
"details": f'Task {task_id} "{task_dict["title"]}" successfully deleted'}

#MARK COMPLETE
#MODIFY MARK COMPLETE TO CALL SLACK API

Choose a reason for hiding this comment

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

You did!

(Probably just an overlooked TODO comment.)

@task_bp.route("/<task_id>/mark_complete", methods=["PATCH"]) #custom endpoint mark task complete
def mark_complete(task_id):
task = validate_id(Task, task_id)
task.completed_at = datetime.utcnow()

db.session.commit()
response = {
"task": task.to_dict()
}
slack_key = os.environ.get("SLACKBOT_API_KEY")
path = "https://slack.com/api/chat.postMessage"
data = {
"channel": "task-notifications",
"text": f"Someone just completed the task {task.title}"
}
headers = {
"authorization":f"Bearer {slack_key}"
}
return jsonify(response)


#MARK INCOMPLETE
@task_bp.route("/<task_id>/mark_incomplete", methods=["PATCH"])
def mark_incomplete(task_id):
task = validate_id(Task, task_id)
task.completed_at = None

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

return jsonify(response)

### GOAL ROUTES

Choose a reason for hiding this comment

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

I like the use of multiple #s in these headings. Gets the point across without being too fancy.

#POST/CREATE A GOAL
goals_bp = Blueprint("Goal", __name__, url_prefix="/goals")
@goals_bp.route("", methods=["POST"])
def create_goal():
request_body = request.get_json()
if "title" not in request_body:
return make_response({"details": "Invalid data"}, 400)

new_goal = Goal(title=request_body["title"])

#abort(make_response)
db.session.add(new_goal)
db.session.commit()

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

#POST #
@goals_bp.route("/<goal_id>/tasks", methods=["POST"])
def post_tasks_to_goal(goal_id):
goal = validate_id(Goal, goal_id)

request_body = request.get_json()
task_ids = []

for task_id in request_body["task_ids"]:
task = validate_id(Task, task_id)

goal.tasks.append(task)
task_ids.append(task_id)

db.session.commit()

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

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

response = []

for task in goal.tasks:
response.append(task.to_new_dict())

return {"id": goal.goal_id, "title": goal.title, "tasks": response}

Choose a reason for hiding this comment

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

Oops, some extra blank lines.





## GET ALL GOALS
@goals_bp.route("", methods=["GET"])
def get_all_goals():
goals = Goal.query.all()
response = []

for goal in goals:
response.append(goal.to_dict())
return jsonify(response), 200

#GET ONE GOAL
@goals_bp.route("/<goal_id>", methods=["GET"])
def get_one_goal(goal_id):
goal = validate_id(Goal, goal_id) #id instead of goal_id
response_body = {
"goal": goal.to_dict()
}
return response_body

#UPDATE GOAL
@goals_bp.route("/<goal_id>", methods=["PUT"])
def update_goal(goal_id):
goal = validate_id(Goal, goal_id)
request_body = request.get_json()
goal.title = request_body["title"]

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

#DELETE ONE GOAL
@goals_bp.route("/<goal_id>", methods=["DELETE"])
def delete_goal(goal_id):
goal = validate_id(Goal, goal_id)

goal_dict = goal.to_dict()

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

return {
"details": f'Goal {goal_id} "{goal_dict["title"]}" successfully deleted'}

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