Skip to content
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()'
7 changes: 7 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?
This endpoint posts a message to a channel.
- 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?
token and channel (string)
- How does this endpoint relate to the Slackbot API key (token) we just created?
The token we just made is one of the arguments we have to pass in to the endpoint

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

Expand All @@ -119,8 +123,11 @@ Press the "Test Method" button!
Scroll down to see the HTTP response. Answer the following questions:

- Did we get a success message? If so, did we see the message in our actual Slack workspace?
The actual text I put in the test did show up as a message in the slack workspace. There was also a response that came back on the test webpage that said the message had been successful.
- Did we get an error emssage? If so, why?
I did not get an error message.
- What is the shape of this JSON? Is it a JSON object or array? What keys and values are there?
The JSON is an object enclosed in {} and has many key value pairs such as "ok" : true and "deleted": false.

### Verify with Postman

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

# Register Blueprints here
from .routes.task_routes import task_bp
from .routes.goal_routes import goal_bp
app.register_blueprint(task_bp)
app.register_blueprint(goal_bp)

return app
15 changes: 15 additions & 0 deletions app/models/goal.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
from app import db
from flask import abort, make_response


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 update(self,request_body):
try:
self.title = request_body["title"]
except KeyError as error:
abort(make_response({'message': f"Missing attribute: {error}"}))

def to_json(self):
return {
"id": self.goal_id,
"title": self.title,
}
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 make_response, abort


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, nullable=True, default=None)
goal = db.relationship("Goal", back_populates="tasks")
goal_id = db.Column(db.Integer, db.ForeignKey("goal.goal_id"), nullable=True)

def to_json(self):
if self.completed_at:
is_complete = True
else:
is_complete = False

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
}
Comment on lines +13 to +33
Copy link

@chimerror chimerror Jan 9, 2023

Choose a reason for hiding this comment

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

This works fine, but you can cut down on a bit of repetition using bool and a local variable for your return value:

   def to_json(self):
       json_dict = {
           "id": self.task_id,
           "title": self.title,
           "description": self.description,
           "is_complete": bool(self.completed_at)
       }

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

       return json_dict


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

@classmethod
def create(cls, request_body):
new_task = cls(
title = request_body["title"],
description = request_body["description"],
)
return new_task


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

This file was deleted.

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

Choose a reason for hiding this comment

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

Good job splitting this into goal routes and task routes!

from app.models.goal import Goal
from app.models.task import Task
from flask import Blueprint, request, jsonify, make_response
from app.routes.task_routes import validate_id

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

# ================================
# Create one goal
# ================================
Comment on lines +9 to +11

Choose a reason for hiding this comment

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

I like how the borders here make these comments pop.


@goal_bp.route("", methods=["POST"])
def create_goal():
request_body = request.get_json()

try:
new_goal = Goal(title=request_body["title"])
except KeyError:
return make_response({"details": "Invalid data"}, 400)

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

return jsonify({"goal": new_goal.to_json()}), 201

Choose a reason for hiding this comment

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

Since you are returning a dictionary, you shouldn't need to use jsonify here, so you could replace this return statement with return {"goal": new_goal.to_json()}, 201.

I wager there's other places where you used jsonify like this, but I won't point them all out to avoid repeating myself.


# ==================================
# Get all goals
# ==================================

@goal_bp.route("", methods=["GET"])
def get_all_goals():
all_goals = Goal.query.all()
results_list = []
for goal in all_goals:
results_list.append(goal.to_json())
return jsonify(results_list), 200

Choose a reason for hiding this comment

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

Here, though, you do have to use jsonify since results_list is a list, not a dictionary.

That being said, if your HTTP status code is 200, you don't need to include it in your return value, so you could just do return jsonify(results_list).

That being said, keeping it can be easily justified as a way to be more explicit, which is often good style IMO.


# ==================================
# Get one goal by id number
# ==================================

@goal_bp.route("/<goal_id>", methods=["GET"])
def get_one_goal(goal_id):
return jsonify({"goal": validate_id(Goal, goal_id).to_json()}), 200
Comment on lines +43 to +45

Choose a reason for hiding this comment

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

This works fine and is succinct, but there is a lot going on that may make this a bit hard to parse for a reader. That is, there's a call to validate_id, to_json, the dictionary constructor, and jsonify. (Which, you don't need the jsonify call as previously mentioned.)

Beyond just being hard to parse, this also has disadvantages when debugging. For example, if you wanted to debug just this call to to_json by putting a breakpoint on it, you'd have to step in and out of validate_id before that calls to_json. Alternatively, you put your breakpoint in to_json, but then all calls to to_json would hit that breakpoint unless you make a conditional breakpoint (which I'm not sure there's an easy condition you could use here...).

I'm probably being nitpicky, but I guess I wanted to point out some of the issues with succinctness!


# ==================================
# Update one goal
# ==================================

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

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 a method off of the Goal to update here instead of doing it directly in the route itself. It's good separation of concerns, leaving what exactly updating means to the Goal model class.

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

# ==================================
# Delete one goal by id
# ==================================

@goal_bp.route("/<goal_id>", methods=["DELETE"])
def delete_one_goal(goal_id):
goal = validate_id(Goal, goal_id)

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

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

Choose a reason for hiding this comment

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

This return statement using make_response ends up being exactly the same as your other return statements that don't use make_response. Which is fine, but it's often a good habit to pick one way to return here and be consistent, and so far, I've generally seen you not use make_response, so I think that would be preferred.


# ======================================
# Get all tasks that are under one goal
# ======================================
@goal_bp.route("/<goal_id>/tasks", methods=["GET"])
def get_all_tasks_for_one_goal(goal_id):
goal = validate_id(Goal, goal_id)
all_goal_tasks = []
for task in goal.tasks:
all_goal_tasks.append(Task.to_json(task))
return make_response({"id": goal.goal_id, "title": goal.title, "tasks": all_goal_tasks}, 200)

# ======================================
# Post tasks to one goal
# ======================================
@goal_bp.route("/<goal_id>/tasks", methods=["POST"])
def associate_tasks_with_one_goal(goal_id):
request_body = request.get_json()
goal = validate_id(Goal,goal_id)
task_list = request_body["task_ids"]
for task_number in task_list:
task = validate_id(Task, task_number)
if task not in goal.tasks:
goal.tasks.append(task)
db.session.commit()
return make_response({"id": goal.goal_id, "task_ids": task_list}, 200)
135 changes: 135 additions & 0 deletions app/routes/task_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
from flask import Blueprint, jsonify, abort, make_response, request
from app import db
from app.models.task import Task
from datetime import datetime
import os
import requests

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

# ================================
# Create one task
# ================================
@task_bp.route("", methods=["POST"])
def create_task():
request_body = request.get_json()

try:
new_task = Task.create(request_body)
except KeyError:
return make_response({"details": "Invalid data"}, 400)

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

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

# ==================================
# Get all tasks
# ==================================
@task_bp.route("", methods=["GET"])
def get_all_tasks():
sort_query = request.args.get("sort")
if sort_query == 'asc':
all_tasks = Task.query.order_by(Task.title.asc())
elif sort_query == 'desc':
all_tasks = Task.query.order_by(Task.title.desc())
else:
all_tasks = Task.query.all()

results_list = []
for task in all_tasks:
results_list.append(task.to_json())

return jsonify(results_list), 200

# ==================================
# Get one task by id number
# ==================================
@task_bp.route("/<task_id>", methods=["GET"])
def get_one_task(task_id):
return jsonify({"task": validate_id(Task, task_id).to_json()}), 200

# ==================================
# Update one task
# ==================================
@task_bp.route("/<task_id>", methods=["PUT"])
def update_one_task(task_id):
request_body = request.get_json()
task = validate_id(Task, task_id)
task.update(request_body)

db.session.commit()

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

# ==================================
# Delete one task by id
# ==================================
@task_bp.route("/<task_id>", methods=["DELETE"])
def delete_one_task(task_id):
task = validate_id(Task, task_id)

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

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

# ==================================
# update one task's completeness
# ==================================
@task_bp.route("/<task_id>/<mark>", methods=["PATCH"])
def mark_tasks_complete_or_incomplete(task_id, mark):
task = validate_id(Task, task_id)
task.completed_at = datetime.now() if mark == "mark_complete" else None

Choose a reason for hiding this comment

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

Nice, I like how succinct this is, and saves having to define two different endpoints.

There's a downside in that I can provide other mark values than are acceptable, so PATCH /tasks/13/mark_foo would be the same as PATCH /tasks/13/mark_incomplete (or as a scarier example, PATCH /tasks/13/mark_complet, where I misspelled "complete").

But this is still a good idea, and there's a pretty easy fix.

send_to_slack(task.title, "task-notifications", mark)
db.session.commit()
return jsonify({"task": task.to_json()}), 200

# ==================================
# Helper function to validate id
# ==================================
def validate_id(class_name,id):
try:
id = int(id)
except:
abort(make_response({"message":f"Id {id} is an invalid id"}, 400))

query_result = class_name.query.get(id)
if not query_result:
abort(make_response({"message":f"Id {id} not found"}, 404))

return query_result
Comment on lines +92 to +102

Choose a reason for hiding this comment

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

Two minor things:

  1. Since this function is used by both the goal_routes and task_routes modules, I would recommend pulling it out to a shared utils module or something like that. The idea being to signal to a reader where they could find the validate_id module that is shared between them both.
  2. While your messages here are pretty good, I think they could be even more informative if you used the class_name parameter in them. So for example, f"{class_name} id {id} has an invalid id" would produce "Task id foo has an invalid id". This would be helpful for the POST /goals/<goal_id>/tasks endpoint where you have both Goals and Tasks you're working with.


# ==================================
# Helper function to send message to slack
# ==================================
def send_to_slack(title, channel_name, mark):
header_data = {'Authorization': f"Bearer {os.environ.get('SLACK_BOT_TOKEN')}"}
message_data = {'channel': channel_name, 'text': f"Someone just completed the task {title}"}
if mark == 'mark_complete':
requests.post('https://slack.com/api/chat.postMessage', data=message_data, headers=header_data)
Comment on lines +107 to +111

Choose a reason for hiding this comment

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

While I made a comment about the validate_id function, this send_to_slack function is only used by the task_routes so it's fine to remain here.

If you began expanding the Slack integration to goal_routes, then it may make sense to move this to its own module, perhaps something like slack_utils...





# original implementation using slack_sdk to make calls to slack API

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.

Good job at explaining why this commented code was here! In general, I recommend getting rid of commented code, especially if it's been committed already and thus is available from the source control history, but if that doesn't work for some reason, this is exactly what you should do!

# import logging
# from slack_sdk import WebClient
# from slack_sdk.errors import SlackApiError

# def send_to_slack(title, channel_name, mark):
# if mark == "mark_complete":
# client = WebClient(token=os.environ.get("SLACK_BOT_TOKEN"))
# logger = logging.getLogger(__name__)
# try:
# # Call the chat.postMessage method using the WebClient
# result = client.chat_postMessage(
# channel=channel_name,
# text= f"Someone just completed the task '{title}'"
# )
# logger.info(result)

# except SlackApiError as e:
# logger.error(f"Error posting message: {e}")
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