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

Muniba K Zoisite C19 #128

Open
wants to merge 12 commits into
base: main
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
8 changes: 6 additions & 2 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ def create_app(test_config=None):
app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False

if test_config is None:
app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
"SQLALCHEMY_DATABASE_URI")
app.config['SQLALCHEMY_DATABASE_URI'] = os.environ.get("RENDER_DATABASE_URI")
else:
app.config["TESTING"] = True
app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
Expand All @@ -30,5 +29,10 @@ def create_app(test_config=None):
migrate.init_app(app, db)

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

from app.routes.goal_routes import goals_bp
app.register_blueprint(goals_bp)
Comment on lines +32 to +36

Choose a reason for hiding this comment

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

Nice choice to split up the routes into files that are more specific to the resources they work with!


return app
34 changes: 34 additions & 0 deletions app/helper_functions.py

Choose a reason for hiding this comment

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

I recommend placing this file in the routes directory to keep it close to the code that uses it.

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
from flask import abort, make_response, request, jsonify
import requests
from dotenv import load_dotenv
import os
load_dotenv()

def validate_model(cls, model_id):
try:
model_id = int(model_id)
except:
abort(make_response({"message":f"{cls.__name__} {model_id} invalid"}, 400))

model = cls.query.get(model_id)

if not model:
abort(make_response({"message":f"{cls.__name__} {model_id} not found"}, 404))

Choose a reason for hiding this comment

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

A lot happens on this line, to make the code easier to read at a glance I'd suggest splitting this line up.


return model

def slack_mark_complete(self):

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 for the slack message!

API_KEY= os.environ.get("API_KEY")

Choose a reason for hiding this comment

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

API_KEY is a local variable of the slack_mark_complete function, so we'd typically see the name lowercased here. We use all caps for constant values, typically when declared where they're accessible across a file or project.

url = "https://slack.com/api/chat.postMessage"

data = {
"channel": "#task-list-api",
"text": f"Someone just completed the task {self.title}"
}
headers = {
'Authorization': f'{API_KEY}'

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 funky here, the common practice is to indent the contents of a dictionary literal more than the brackets.

    headers = {
        'Authorization': f'{API_KEY}'
    }

}

response = requests.post(url, headers=headers, data=data)

print(response.text)
35 changes: 34 additions & 1 deletion app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,38 @@
from app import db


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


def to_dict(self):

goal_as_dict = {
"id": self.goal_id,
"title": self.title,
}
if self.tasks:
goal_as_dict["task_ids"] = [task.to_dict() for task in self.tasks]

return goal_as_dict


def to_json(self):

json_tasks = []

for task in self.tasks:
json_tasks.append(task.to_json())

return{
"id": self.goal_id,
"title": self.title,
"tasks": json_tasks
}
Comment on lines +9 to +32

Choose a reason for hiding this comment

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

These functions have a lot of code in common, we could combine them if we had some kind of parameter to determine whether the task key needs to be "task_ids" or "tasks". How could we implement that to D.R.Y. up our code?



@classmethod
def from_dict(cls, goal_data):
new_goal = Goal(title=goal_data["title"])
return new_goal
86 changes: 85 additions & 1 deletion app/models/task.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,89 @@
from app import db

from app.models.goal import Goal
from datetime import datetime
import requests
from dotenv import load_dotenv
import os
load_dotenv()

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



Comment on lines +16 to +18

Choose a reason for hiding this comment

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

Typically we would have either 1 or 2 blank lines between the attributes and the methods of a class, the most important thing is to be consistent across all classes, whichever you choose.

def to_dict(self):
task_as_dict = {}
task_as_dict["task_id"] = self.task_id
task_as_dict["title"] = self.title
task_as_dict["description"] = self.description
task_as_dict["completed_at"] = self.completed_at
if self.goal_id:
task_as_dict["goal_id"] =self.goal_id

return task_as_dict

def to_json(self):
if self.completed_at:
is_complete = True
else:
is_complete = False
Comment on lines +31 to +34

Choose a reason for hiding this comment

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

Great choice to derive the value of is_complete from self.completed_at! We could shorten this a little with the python bool function:

is_complete = bool(self.completed_at)


if self.goal_id:
return{
"id": self.task_id,
"title": self.title,
"description": self.description,
"is_complete": is_complete,
"goal_id": self.goal_id
}
else:

return{
"id": self.task_id,
"title": self.title,
"description": self.description,
"is_complete": is_complete
}

# def to_json_tasks(self):


# return{
# "id": self.goal_id,
# "title": self.title
# }
Comment on lines +53 to +59

Choose a reason for hiding this comment

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

Reminder that we want to remove commented out code before opening PRs.



@classmethod
def from_dict(cls, task_data):
new_task = Task(title=task_data["title"],
description=task_data["description"]
)
return new_task

def mark_complete(self, request_body):
self.completed_at = datetime.utcnow()

def mark_incomplete(self, request_body):
self.completed_at = None

def slack_mark_complete(self):
API_KEY = os.environ.get("API_KEY")

Choose a reason for hiding this comment

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

API_KEY is a local variable of the slack_mark_complete function, so we'd typically see the name lowercased here. We use all caps for constant values, typically when declared where they're accessible across a file or project.

url = "https://slack.com/api/chat.postMessage"

data = {
"channel": "#task-list-api",
"text": f"Someone just completed the task {self.title}"
}
headers = {
'Authorization': f'{API_KEY}'
}

response = requests.post(url, headers=headers, data=data)

print(response.text)

Choose a reason for hiding this comment

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

Unless it's required, we should remove print and other debugging statements before opening PRs. Are there any other debugging statements across the project that could be removed?

Comment on lines +75 to +89

Choose a reason for hiding this comment

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

I would strongly suggest placing this function in a routes helper or the routes file that uses it. By including slack_mark_complete in the Task class, it now needs to have knowledge of how to make requests and use environment variables, instead of having a single purpose of representing a database model and transforming that model's data.

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

This file was deleted.

Empty file added app/routes/__init__.py
Empty file.
96 changes: 96 additions & 0 deletions app/routes/goal_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
from app import db
from flask import Blueprint, jsonify, make_response, request
from app.models.goal import Goal
from app.models.task import Task
from app.helper_functions import validate_model

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

@goals_bp.route("", methods=["GET"])
def read_all_goals():

title_query = request.args.get("title")
if title_query:
goals = Goal.query.filter_by(title=title_query)

if request.args.get("sort") == "asc":
goals = Goal.query.order_by(Goal.title.asc()).all()

elif request.args.get("sort") == "desc":
goals = Goal.query.order_by(Goal.title.desc()).all()

else:
goals = Goal.query.all()

goals_response = []
for goal in goals:
goals_response.append(goal.to_dict())
return jsonify(goals_response), 200
Comment on lines +12 to +28

Choose a reason for hiding this comment

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

We can either sort or filter with this implementation. If we want to do both, we can get a reference to the class's query object that we keep updating:

model_query = Goal.query

title_query = request.args.get("title")
if title_query:
    model_query = model_query.filter(Goal.title == request.args.get("title"))

if request.args.get("sort") == "asc":
    model_query = model_query.order_by(Goal.title.asc())
elif request.args.get("sort") == "desc":
    model_query = model_query.order_by(Goal.title.desc())

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


#GET route to read one task
@goals_bp.route("/<goal_id>", methods=["GET"])
def read_one_task(goal_id):
goal = validate_model(Goal,goal_id)
return jsonify({"goal": goal.to_dict()}), 200

@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.from_dict(request_body)

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

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

#UPDATE route to edit a goal
@goals_bp.route("/<goal_id>", methods=["PUT"])
def update_goal(goal_id):
goal = validate_model(Goal, goal_id)

request_body = request.get_json()

goal.title = request_body["title"]

Choose a reason for hiding this comment

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

There's some error handling in create_goal that we could use here to alert the user if the request was missing a title key. How could we share the behavior with both functions?


db.session.commit()

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

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

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

return make_response({"details": f"Goal {goal.goal_id} \"{goal.title}\" successfully deleted"})


#POST ROUTE
@goals_bp.route("/<goal_id>/tasks", methods=["POST"])
def link_tasks_to_goal(goal_id):

goal = validate_model(Goal, goal_id)
request_body = request.get_json()

for task_id in request_body["task_ids"]:
task = validate_model(Task, task_id)
task.goal_id = goal_id
db.session.add(task)

Choose a reason for hiding this comment

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

task already exists in the database so we could remove this line, the commit below will save our changes.


db.session.commit()

return make_response({"id": goal.goal_id, "task_ids":request_body["task_ids"]}), 200


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

goal = validate_model(Goal, goal_id)

return make_response(goal.to_json()), 200
110 changes: 110 additions & 0 deletions app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
from app import db
from flask import Blueprint, jsonify,make_response, request
from app.models.task import Task
from app.models.goal import Goal
from app.helper_functions import slack_mark_complete, validate_model

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

# POST route for creating a task
@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:
return make_response({"details": "Invalid data"}, 400)

new_task = Task.from_dict(request_body)

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

return make_response({"task":new_task.to_json()}), 201

# GET route for reading all tasks
@tasks_bp.route("", methods=["GET"])
def read_all_tasks():

title_query = request.args.get("title")
if title_query:
tasks = Task.query.filter_by(title=title_query)

description_query = request.args.get("description")
if description_query:
tasks = Task.query.filter_by(description=description_query)

completed_query = request.args.get("is_complete")
if completed_query:
tasks = Task.query.filter_by(is_completed = completed_query)

if request.args.get("sort") == "asc":
tasks = Task.query.order_by(Task.title.asc()).all()

elif request.args.get("sort") == "desc":
tasks = Task.query.order_by(Task.title.desc()).all()

else:
tasks = Task.query.all()


tasks_response = []
for task in tasks:
tasks_response.append(task.to_json())
return jsonify(tasks_response), 200

#GET route to read one task
@tasks_bp.route("/<task_id>", methods=["GET"])
def read_one_task(task_id):
task = validate_model(Task,task_id)
return jsonify({"task": task.to_json()}), 200

#UPDATE route to edit a task
@tasks_bp.route("/<task_id>", methods=["PUT"])
def update_task(task_id):
task = validate_model(Task, task_id)

request_body = request.get_json()

task.title = request_body["title"]
task.description = request_body["description"]

db.session.commit()

return make_response({"task": task.to_json()}), 200

#DELETE existing task
@tasks_bp.route("/<task_id>", methods=["DELETE"])
def delete_task(task_id):
task = validate_model(Task, task_id)

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

return make_response({"details": f"Task {task.task_id} \"{task.title}\" successfully deleted"})

#PATCH REQUEST marking a test complete
@tasks_bp.route("/<task_id>/mark_complete", methods=["PATCH"])
def update_complete_status(task_id):
task_completed = validate_model(Task, task_id)

request_body = request.get_json()

task_completed.mark_complete(request_body)

db.session.commit()

slack_mark_complete(task_completed)

return jsonify({"task":task_completed.to_json()}), 200


@tasks_bp.route("/<task_id>/mark_incomplete", methods=["PATCH"])
def update_incomplete_status(task_id):
task_to_update = validate_model(Task, task_id)
request_body = request.get_json()

task_to_update.mark_incomplete(request_body)

db.session.commit()
return jsonify({"task":task_to_update.to_json()}), 200

Loading