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

Say Zoisite #109

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
22 changes: 17 additions & 5 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,43 @@
import os
from dotenv import load_dotenv

token = os.environ.get('SLACK_TOKEN')

db = SQLAlchemy()
migrate = Migrate()
load_dotenv()


# client = slack.WebClient(token=os.environ['SLACK_TOKEN'])
def create_app(test_config=None):
app = Flask(__name__)
app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False

if test_config is None:
app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
"SQLALCHEMY_DATABASE_URI")
"RENDER_DATABASE_URI")
# app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
# "SQLALCHEMY_DATABASE_URI")
else:
app.config["TESTING"] = True
app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
"SQLALCHEMY_TEST_DATABASE_URI")

db.init_app(app)
migrate.init_app(app, db)



# Import models here for Alembic setup
from app.models.task import Task
from app.models.goal import Goal

db.init_app(app)
migrate.init_app(app, db)


from app.routes.task_routes import tasks_bp
from app.routes.goal_routes import goals_bp
Comment on lines +38 to +39

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.

# Register Blueprints here
app.register_blueprint(tasks_bp)
app.register_blueprint(goals_bp)

# client = slack.WebClient(token=os.environ['SLACK_TOKEN'])

Choose a reason for hiding this comment

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

I see this commented line duplicated on line 13, we should remove commented code like this and rely on our repo's commit history if we need to look up past code.

return app

57 changes: 57 additions & 0 deletions app/helper.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,57 @@
from app.models.task import Task
from flask import jsonify, abort, make_response
from app import token
import requests

def filter_by_params(cls, query_params):
sort_by = query_params.get("sort")

if sort_by:
return get_sorted_items_by_params(cls, query_params)
Comment on lines +7 to +10

Choose a reason for hiding this comment

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

I would consider separating out the sort into its own function or renaming filter_by_params to reflect that it results in both sorting and filtering.


if query_params:
query_params = {k.lower(): v.title() for k, v in query_params.items()}
items = cls.query.filter_by(**query_params).all()
else:
items = cls.query.all()
Comment on lines +12 to +16

Choose a reason for hiding this comment

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

There's some similar code repeated depending on if we want to sort and filter or only filter. If we want to D.R.Y. up the code, we could get a reference to the class's query object that we keep updating. One possibility might look like:

model_query = cls.query

if query_params:
    query_params = {k.lower(): v.title() for k, v in query_params.items()}
    model_query = model_query.filter_by(**query_params).all()

if sort_by and sort_by == "asc":
    model_query = model_query.order_by(cls.title.asc())
elif sort_by and sort_by == "desc":
    model_query = model_query.order_by(cls.title.desc())

items = model_query.all()


return items


def get_sorted_items_by_params(cls, query_params):
sort_param = query_params.pop('sort', None)

if sort_param == 'asc':
return cls.query.filter_by(**query_params).order_by(cls.title.asc()).all()
elif sort_param == 'desc':
return cls.query.filter_by(**query_params).order_by(cls.title.desc()).all()
else:
return cls.query.filter_by(**query_params).order_by(cls.id.asc()).all()


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

obj = cls.query.get(id)
if not obj:
abort(make_response(jsonify(message=f"{cls.__name__} not found"), 404))

return obj

def slack_post_message(task):

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_url = 'http://slack.com/api/chat.postMessage'

payload = {
"channel": "task-notifications",
"text":f"Someone just completed the task {task.title}"
}
headers = {
"Authorization": f"Bearer {token}"
}
response = requests.post(api_url, headers=headers, data=payload)

print(response.text)
32 changes: 32 additions & 0 deletions app/models/goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,35 @@

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", lazy=True)

def goal_to_dict(self):
return {
"id": self.goal_id,
"title": self.title
}
@classmethod

Choose a reason for hiding this comment

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

Style best practice note: We should leave a blank line between functions to make it visually clear where one ends and another begins.

def create_new_goal(cls, request_data):
if "title" not in request_data:
raise KeyError

return cls(
title=request_data["title"].title()
)

def __str__(self):
return {
self.__class__.__name__.lower(): {
"id": self.goal_id,
"title": self.title
}
}
Comment on lines +23 to +29

Choose a reason for hiding this comment

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

To reduce confusion for users of our code, I suggest following expectations of a function when implementing python's built in methods for a class. __str__ is meant to return a human-readable, or informal, string representation of an object, and it would be unexpected by most users that this function would return a dictionary. A little more info can be found in the python docs: https://docs.python.org/3/reference/datamodel.html?highlight=__str__#object.__str__


def update(self, goal):

Choose a reason for hiding this comment

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

If the parameter goal is not a Goal object, then for clarity I'd suggest updating the name to better reflect the contents. If the parameter holds data to update a goal, what's a descriptive name we could use?

for key, value in goal.items():
if key == "title":
self.title = value
Comment on lines +32 to +34

Choose a reason for hiding this comment

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

The dictionary could contain many extraneous keys that we need to loop through, another option could be to check if the dictionary contains the title key (which can be an O(1) operation) then set the title attribute if the key exists.


return self.goal_to_dict()

51 changes: 51 additions & 0 deletions app/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,54 @@

class Task(db.Model):
task_id = db.Column(db.Integer, primary_key=True)
title = db.Column(db.String(length=255))
description=db.Column(db.String)
completed_at=db.Column(db.DateTime, nullable=True)
is_complete= db.Column(db.Boolean, default=False)
Comment on lines +8 to +9

Choose a reason for hiding this comment

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

is_complete duplicates data we have available by looking at the truthy or falsy value of the completed_at field, and creates a situation where we need to manually keep the values of completed_at and is_complete in sync with each other. I recommend removing is_complete and checking if completed_at is truthy or falsy in those locations.

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

def task_to_dict(self):
return {
"id": self.task_id,
"title": self.title,
"description": self.description,
"is_complete": False if self.completed_at is None else True
if self.completed_at is not None else None

Choose a reason for hiding this comment

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

Is line 19 intended to be there?

}

@classmethod
def create_new_task(cls, request_data):
if "title" not in request_data:
raise KeyError({"details": "Invalid data"})
if "description" not in request_data:
raise KeyError("description")

request_data["completed_at"] = None
is_complete = False if request_data["completed_at"] is None else True
Comment on lines +29 to +30

Choose a reason for hiding this comment

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

In this implementation, we hardcode a value in the input dictionary to None, then use that to set is_complete and completed_at. The result would be the same if we hardcoded is_complete to False and completed_at to None without adding a key to the request_data parameter.


return cls(
title=request_data["title"].title(),
description=request_data["description"],
completed_at=request_data.get("completed_at"),
is_complete=is_complete
)

def update(self, task):
for key, value in task.items():
if key == "title":
self.title = value
if key == "description":
self.description = value

return self.task_to_dict()

def __str__(self):
return {
self.__class__.__name__.lower(): {
"id": self.task_id,
"title": self.title,
"description": self.description,
"is_complete": self.is_complete
}
}
1 change: 0 additions & 1 deletion app/routes.py

This file was deleted.

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


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

@goals_bp.route("", methods = ["POST"])
def create_goals():
request_body = request.get_json()
try:
new_goal = Goal.create_new_goal(request_body)
db.session.add(new_goal)
db.session.commit()

message = new_goal.__str__()

return make_response(jsonify(message), 201)
except KeyError as e:
message = "Invalid data"
return make_response({"details": message}, 400)
Comment on lines +13 to +23

Choose a reason for hiding this comment

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

To make it easier to debug what line caused an exception, we typically want to limit what we put in try blocks to just the code we think might cause an exception. What part of this code could result in a KeyError? I suggest keeping just that code in the try, and moving the rest to after the try/except block.


@goals_bp.route("", methods = ["GET"])
def read_all_goals():
goals = Goal.query.all()
goal_response = [goal.goal_to_dict() for goal in goals]
return jsonify(goal_response)

@goals_bp.route("/<goal_id>", methods=["GET"])
def read_one_goal(goal_id):
goal = validate_model(Goal, goal_id)
message = goal.__str__()

return make_response(jsonify(message), 200)

@goals_bp.route("/<goal_id>", methods=["PUT"])
def update_goal(goal_id):
goal = validate_model(Goal, goal_id)
request_body = request.get_json()
goal.update(request_body)

db.session.commit()
message = goal.__str__()
return make_response(jsonify(message))

@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()

message = {
"details": f'Goal {goal_id} "{goal.title}" successfully deleted'
}
return make_response(jsonify(message), 200)

@goals_bp.route("/<goal_id>/tasks", methods=["POST"])
def assign_tasks_to_goal(goal_id):
goal = validate_model(Goal, goal_id)
request_body = request.get_json()
if not request_body:
return jsonify({'error': 'Invalid request body'}), 400

task_ids = request_body.get('task_ids', [])
for task_id in task_ids:
task = validate_model(Task, task_id)
task.goal_id = goal.goal_id
db.session.commit()

Choose a reason for hiding this comment

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

We could move this line outside of the for-loop to save the changes to all the tasks at once.


return jsonify({'id': goal.goal_id, 'task_ids': task_ids}), 200

@goals_bp.route("/<goal_id>/tasks", methods=["GET"])
def get_task_of_goal(goal_id):
goal = validate_model(Goal, goal_id)
goal = Goal.query.get(goal_id)
task_data = []
for task in goal.tasks:
task_data.append({
"id": task.task_id,
"goal_id": goal.goal_id,
"title": task.title,
"description": task.description,
"is_complete": task.is_complete
})
Comment on lines +80 to +86

Choose a reason for hiding this comment

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

Could we use the task's task_to_dict function?

task_data

Choose a reason for hiding this comment

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

We should remove this line since we aren't performing an operation.

response = {
"id": goal.goal_id,
"title": goal.title,
"tasks": task_data
}
return jsonify(response), 200


77 changes: 77 additions & 0 deletions app/routes/task_routes.py

Choose a reason for hiding this comment

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

Great looking code across the route files as far as line length, clear variable names, spacing, and overall readability 😊

Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
from flask import Blueprint, jsonify, abort, make_response, request
from app import db
from app.models.task import Task
from app.helper import filter_by_params, validate_model, slack_post_message
from datetime import datetime


tasks_bp = Blueprint("tasks", __name__, url_prefix="/tasks")
@tasks_bp.route("", methods = ["POST"])
def create_tasks():
request_body = request.get_json()
try:
new_task = Task.create_new_task(request_body)
db.session.add(new_task)
db.session.commit()

message = new_task.__str__()

return make_response(jsonify(message), 201)
except KeyError as e:
message = "Invalid data"
return make_response({"details": message}, 400)

@tasks_bp.route("", methods = ["GET"])
def read_all_tasks():
query_params = request.args.to_dict()
tasks = filter_by_params(Task, query_params)
tasks_response = [task.task_to_dict() for task in tasks]
return jsonify(tasks_response)

@tasks_bp.route("/<task_id>", methods=["GET"])
def read_one_task(task_id):
task = validate_model(Task, task_id)
task = Task.query.get(task_id)
task = task.__str__()
return jsonify(task), 200

@tasks_bp.route("/<task_id>", methods=["PUT"])
def update_task(task_id):
task = validate_model(Task, task_id)
request_body = request.get_json()
task.update(request_body)

db.session.commit()
message = task.__str__()
return make_response(jsonify(message))

@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()

message = {
"details": f'Task {task_id} "{task.title}" successfully deleted'
}
return make_response(jsonify(message), 200)

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

slack_post_message(task)
message = task.__str__()
return make_response(jsonify(message), 200)

@tasks_bp.route("/<task_id>/mark_incomplete", methods=["PATCH"])
def mark_task_incomplete(task_id):
task = validate_model(Task, task_id)
task.is_complete = False
task.completed_at = None
db.session.commit()
message = task.__str__()
return make_response(jsonify(message), 200)
8 changes: 8 additions & 0 deletions app/say_bot.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# import os
# from pathlib import Path
# from dotenv import load_dotenv

# env_path = Path('.') / '.env'
# load_dotenv(dotenv_path=env_path)

# token = os.environ.get('SLACK_TOKEN')
Comment on lines +1 to +8

Choose a reason for hiding this comment

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

Was this file intended to be committed?

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