-
Notifications
You must be signed in to change notification settings - Fork 43
My_task_list_api #19
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
base: main
Are you sure you want to change the base?
My_task_list_api #19
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,7 @@ | ||||||||||
| from flask import Flask | ||||||||||
| from .db import db, migrate | ||||||||||
| from .routes.task_routes import tasks_bp | ||||||||||
| from .routes.goal_routes import goals_bp | ||||||||||
| from .models import task, goal | ||||||||||
| import os | ||||||||||
|
|
||||||||||
|
|
@@ -10,13 +12,11 @@ def create_app(config=None): | |||||||||
| app.config['SQLALCHEMY_DATABASE_URI'] = os.environ.get('SQLALCHEMY_DATABASE_URI') | ||||||||||
|
|
||||||||||
| if config: | ||||||||||
| # Merge `config` into the app's configuration | ||||||||||
| # to override the app's default settings for testing | ||||||||||
| app.config.update(config) | ||||||||||
|
|
||||||||||
| db.init_app(app) | ||||||||||
| migrate.init_app(app, db) | ||||||||||
|
|
||||||||||
| # Register Blueprints here | ||||||||||
|
|
||||||||||
| app.register_blueprint(tasks_bp, url_prefix="/tasks") | ||||||||||
| app.register_blueprint(goals_bp, url_prefix="/goals") | ||||||||||
|
Comment on lines
+20
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
We don't need to add |
||||||||||
| return app | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,27 @@ | ||||||
| from sqlalchemy.orm import Mapped, mapped_column | ||||||
| from sqlalchemy.orm import Mapped, mapped_column, relationship | ||||||
| from sqlalchemy import Integer, String | ||||||
| from ..db import db | ||||||
|
|
||||||
| class Goal(db.Model): | ||||||
| id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) | ||||||
| __tablename__ = "goals" | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless a team has a convention of preferring to explicitly name a table in the plural form, we should be ok to just leave the table name the same as the class name, which means line 6 can be removed. |
||||||
|
|
||||||
| id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: you have extra whitespaces here. Before you open a PR for code review at internship be sure that your code is cleaned up. Code that doesn't follow convention won't be reviewed or allowed to be merged because the team won't want to introduce inconsistencies into a codebase because it makes it more difficult to maintain and read.
Suggested change
|
||||||
| title: Mapped[str] = mapped_column(String, nullable=False) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the current version of SQLAlchemy
Suggested change
|
||||||
|
|
||||||
| tasks: Mapped[list["Task"]] = relationship( | ||||||
| "Task", | ||||||
| back_populates="goal", | ||||||
| cascade="all, delete-orphan" | ||||||
| ) | ||||||
|
|
||||||
| def to_dict(self) -> dict: | ||||||
| return { | ||||||
| "id": self.id, | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: fix up whitespaces here. Please review the PEP8 style guide here on how to use whitespaces in your code if this topic feels unclear.
Suggested change
|
||||||
| "title": self.title | ||||||
| } | ||||||
|
|
||||||
| @classmethod | ||||||
| def from_dict(cls, data: dict): | ||||||
| return cls( | ||||||
| title=data["title"] | ||||||
| ) | ||||||
|
Comment on lines
+17
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you mean to bring in type checking here? It seems inconsistently used in the project.
|
||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happened to the white spacing in this file? 🧐 Is this intentional or did this formatting get brought in with the help of AI? I find the whitespaces like this make it challenging to read the code and it would be even more challenging to maintain a codebase like this. I prefer to not have the extra whitespaces and for the code to follow the guidelines outlined in the PEP8 style guide (linked above) |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,37 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from sqlalchemy.orm import Mapped, mapped_column | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from sqlalchemy.orm import Mapped, mapped_column, relationship | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from sqlalchemy import Integer, String, Text, DateTime, ForeignKey | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ..db import db | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class Task(db.Model): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| __tablename__ = "tasks" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| title: Mapped[str] = mapped_column(String, nullable=False) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: Mapped[str] = mapped_column(Text, nullable=False) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+9
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the current version of SQLAlchemy
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| completed_at: Mapped[DateTime] = mapped_column(DateTime, nullable=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed that there's a mix of syntax in the Task model. We should always prefer to use consistent syntax in our codebase so that it's easy to read and understand, easy to maintain and predictable. Below on line 14, you use If you use To keep your codebase streamlined and consistent, ideally you'd stick to just using one kind of syntax. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another option, which is the one we show in our curriculum and also shown in the SQLAlchemy documentation is To set nullability for a table's column we can use You can review Building an API Part 3 to read about the
Therefore, these two lines of code accomplish the same thing: completed_at: Mapped[DateTime|None]
completed_at: Mapped[Optional[DateTime]] |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # ← NEW: FK to goals.id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| goal_id: Mapped[int|None] = mapped_column( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ForeignKey("goals.id"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nullable=True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+14
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @NataliaRaz How does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since SQLAlchemy is able to infer that this is a nullable field with
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| goal: Mapped["Goal"] = relationship( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Goal", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| back_populates="tasks" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def from_dict(cls, data): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return cls( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| title = data["title"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description = data["description"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| completed_at = data.get("completed_at") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+26
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer to use the same kind of syntax for getting a value from a dictionary for the sake of consistency. If you use
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def to_dict(self): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "id": self.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "title": self.title, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "description": self.description, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "is_complete": bool(self.completed_at) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+33
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your How does this function need to be updated to conditionally set |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove unnecessary white spaces here and throughout this file too.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1 +1,108 @@ | ||||||||||||||||||
| from flask import Blueprint | ||||||||||||||||||
| from flask import Blueprint, abort, make_response, request, Response | ||||||||||||||||||
| from ..models.goal import Goal | ||||||||||||||||||
| from ..models.task import Task | ||||||||||||||||||
| from ..db import db | ||||||||||||||||||
|
|
||||||||||||||||||
| goals_bp = Blueprint("goals_bp", __name__, url_prefix="/goals") | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer blueprint to be named
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| def validate_goal(goal_id): | ||||||||||||||||||
| try: | ||||||||||||||||||
| gid = int(goal_id) | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can reassign the value of I'd prefer reassignment because
Suggested change
We do the same thing here in Flasky. model_id = int(model_id) |
||||||||||||||||||
| except ValueError: | ||||||||||||||||||
| invalid = {"message": f"Goal id '{goal_id}' is invalid. Must be an integer."} | ||||||||||||||||||
| abort(make_response(invalid, 400)) | ||||||||||||||||||
|
|
||||||||||||||||||
| goal = db.session.get(Goal, gid) | ||||||||||||||||||
| if not goal: | ||||||||||||||||||
| not_found = {"message": f"Goal with id '{goal_id}' not found."} | ||||||||||||||||||
| abort(make_response(not_found, 404)) | ||||||||||||||||||
|
|
||||||||||||||||||
| return goal | ||||||||||||||||||
|
Comment on lines
+8
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer Right now the only differences in these two helper functions are that you specify if you're validating a Instead, we can make a very generic Please review how we did this in Flasky here |
||||||||||||||||||
|
|
||||||||||||||||||
| @goals_bp.post("") | ||||||||||||||||||
| def create_goal(): | ||||||||||||||||||
| request_body = request.get_json() or {} | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
If a request body is empty, |
||||||||||||||||||
| 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() | ||||||||||||||||||
|
Comment on lines
+25
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This works 👍 Prefer the changes we introduced in the lesson on Refactoring (Part 7 of Building an API) were incorporated here - specifically the You can review the implementation of that helper here. Writing a helper function and then using it here makes this route @goals_bp.post("")
def create_goal():
request_body = request.get_json()
return create_model(Goal, request_body) |
||||||||||||||||||
|
|
||||||||||||||||||
| return {"goal": new_goal.to_dict()}, 201 | ||||||||||||||||||
|
|
||||||||||||||||||
| @goals_bp.get("") | ||||||||||||||||||
| def get_all_goals(): | ||||||||||||||||||
| query = db.select(Goal).order_by(Goal.id) | ||||||||||||||||||
| goals = db.session.scalars(query) | ||||||||||||||||||
|
|
||||||||||||||||||
| goals_response = [g.to_dict() for g in goals] | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer
Suggested change
|
||||||||||||||||||
| return goals_response, 200 | ||||||||||||||||||
|
|
||||||||||||||||||
| @goals_bp.get("/<id>") | ||||||||||||||||||
| def get_one_goal(id): | ||||||||||||||||||
| goal = validate_goal(id) | ||||||||||||||||||
| return {"goal": goal.to_dict()}, 200 | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the default status code sent back to the client is 200 so we don't need need to specify 200 here.
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| @goals_bp.put("/<id>") | ||||||||||||||||||
| def update_goal(id): | ||||||||||||||||||
| goal = validate_goal(id) | ||||||||||||||||||
| request_body = request.get_json() or {} | ||||||||||||||||||
| if "title" not in request_body: | ||||||||||||||||||
| return make_response({"details": "Invalid data"}, 400) | ||||||||||||||||||
|
|
||||||||||||||||||
| goal.title = request_body["title"] | ||||||||||||||||||
| db.session.commit() | ||||||||||||||||||
|
|
||||||||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||||||||
|
|
||||||||||||||||||
| @goals_bp.delete("/<id>") | ||||||||||||||||||
| def delete_goal(id): | ||||||||||||||||||
| goal = validate_goal(id) | ||||||||||||||||||
| db.session.delete(goal) | ||||||||||||||||||
| db.session.commit() | ||||||||||||||||||
|
|
||||||||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||||||||
| #------------------Wave 6------------------------ | ||||||||||||||||||
| @goals_bp.post("/<goal_id>/tasks") | ||||||||||||||||||
| def assign_tasks_to_goal(goal_id): | ||||||||||||||||||
| goal = validate_goal(goal_id) | ||||||||||||||||||
|
|
||||||||||||||||||
| data = request.get_json() or {} | ||||||||||||||||||
| if "task_ids" not in data or not isinstance(data["task_ids"], list): | ||||||||||||||||||
| return make_response({"details": "Invalid data"}, 400) | ||||||||||||||||||
|
|
||||||||||||||||||
| tasks_to_assign = [] | ||||||||||||||||||
| for tid in data["task_ids"]: | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer Names are hard and getting better at naming variables comes with time. |
||||||||||||||||||
| task = db.session.get(Task, tid) | ||||||||||||||||||
| if not task: | ||||||||||||||||||
| abort(make_response( | ||||||||||||||||||
| {"message": f"Task with id '{tid}' not found."}, | ||||||||||||||||||
| 404 | ||||||||||||||||||
| )) | ||||||||||||||||||
|
Comment on lines
+77
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of writing all this code here in the route, which duplicates code you have written in
Suggested change
|
||||||||||||||||||
| tasks_to_assign.append(task) | ||||||||||||||||||
|
|
||||||||||||||||||
| goal.tasks = tasks_to_assign | ||||||||||||||||||
| db.session.commit() | ||||||||||||||||||
|
|
||||||||||||||||||
| return {"id": goal.id, "task_ids": data["task_ids"]}, 200 | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Also, the default status code sent back to the client is 200 so we don't need need to specify 200 here. |
||||||||||||||||||
|
|
||||||||||||||||||
| @goals_bp.get("/<goal_id>/tasks") | ||||||||||||||||||
| def get_tasks_for_goal(goal_id): | ||||||||||||||||||
| goal = validate_goal(goal_id) | ||||||||||||||||||
|
|
||||||||||||||||||
| tasks_list = [] | ||||||||||||||||||
| for t in goal.tasks: | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This codebase would be more self-documenting and readable if the variable was the entire noun, instead of just the first letter of the noun. It doesn't make the code overly verbose, but actually improves readability, by writing |
||||||||||||||||||
| tasks_list.append({ | ||||||||||||||||||
| "id": t.id, | ||||||||||||||||||
| "goal_id": t.goal_id, | ||||||||||||||||||
| "title": t.title, | ||||||||||||||||||
| "description": t.description, | ||||||||||||||||||
| "is_complete": bool(t.completed_at) | ||||||||||||||||||
| }) | ||||||||||||||||||
|
Comment on lines
+96
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about using the
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| return { | ||||||||||||||||||
| "id": goal.id, | ||||||||||||||||||
| "title": goal.title, | ||||||||||||||||||
| "tasks": tasks_list | ||||||||||||||||||
| }, 200 | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1 +1,132 @@ | ||||||||||||||||||
| from flask import Blueprint | ||||||||||||||||||
| import os | ||||||||||||||||||
| from flask import Blueprint, abort, make_response, request, Response | ||||||||||||||||||
| from app.models.task import Task | ||||||||||||||||||
| from datetime import datetime, timezone | ||||||||||||||||||
| from ..db import db | ||||||||||||||||||
| import requests | ||||||||||||||||||
|
|
||||||||||||||||||
| tasks_bp = Blueprint("tasks_bp", __name__, url_prefix="/tasks") | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| def validate_task(task_id): | ||||||||||||||||||
|
|
||||||||||||||||||
| try: | ||||||||||||||||||
| tid = int(task_id) | ||||||||||||||||||
| except ValueError: | ||||||||||||||||||
| abort(make_response( | ||||||||||||||||||
| {"message": f"Task id '{task_id}' is invalid. Must be an integer."}, | ||||||||||||||||||
| 400 | ||||||||||||||||||
| )) | ||||||||||||||||||
|
|
||||||||||||||||||
| task = db.session.get(Task, tid) | ||||||||||||||||||
| if not task: | ||||||||||||||||||
| not_found = {"message": f"Task with id '{task_id}' not found."} | ||||||||||||||||||
| abort(make_response(not_found, 404)) | ||||||||||||||||||
|
|
||||||||||||||||||
| return task | ||||||||||||||||||
|
Comment on lines
+10
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like my comment in |
||||||||||||||||||
|
|
||||||||||||||||||
| @tasks_bp.post("") | ||||||||||||||||||
| def create_task(): | ||||||||||||||||||
| request_body = request.get_json() or {} | ||||||||||||||||||
| 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() | ||||||||||||||||||
|
Comment on lines
+30
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like I previously mentioned, I'd prefer you implement a helper method like create_model to reduce repetition and make this route more concise.
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| return {"task": new_task.to_dict()}, 201 | ||||||||||||||||||
|
|
||||||||||||||||||
| @tasks_bp.get("") | ||||||||||||||||||
| def get_all_tasks(): | ||||||||||||||||||
|
|
||||||||||||||||||
| query = db.select(Task) | ||||||||||||||||||
|
|
||||||||||||||||||
| sort_param = request.args.get("sort") #Checks the URL for ?sort=asc or ?sort=desc | ||||||||||||||||||
| if sort_param == "asc": | ||||||||||||||||||
| query = query.order_by(Task.title) | ||||||||||||||||||
| elif sort_param == "desc": | ||||||||||||||||||
| query = query.order_by(Task.title.desc()) | ||||||||||||||||||
| else: | ||||||||||||||||||
| query = query.order_by(Task.id) | ||||||||||||||||||
|
|
||||||||||||||||||
| tasks = db.session.scalars(query) #Executes the query | ||||||||||||||||||
|
|
||||||||||||||||||
| tasks_response = [] | ||||||||||||||||||
| for task in tasks: | ||||||||||||||||||
| tasks_response.append({ | ||||||||||||||||||
| "id": task.id, | ||||||||||||||||||
| "title": task.title, | ||||||||||||||||||
| "description": task.description, | ||||||||||||||||||
| "is_complete": bool(task.completed_at) | ||||||||||||||||||
| }) | ||||||||||||||||||
|
Comment on lines
+55
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about your
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| return tasks_response, 200 | ||||||||||||||||||
|
|
||||||||||||||||||
| @tasks_bp.patch("/<id>/mark_complete") | ||||||||||||||||||
| def mark_complete(id): | ||||||||||||||||||
| task = validate_task(id) | ||||||||||||||||||
|
|
||||||||||||||||||
| task.completed_at = datetime.now(timezone.utc) | ||||||||||||||||||
| db.session.commit() | ||||||||||||||||||
| token = os.environ.get('SLACK_BOT_TOKEN', "") | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a constant variable since the string value won't change so we can use all capital letters to indicate this.
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| url = "https://slack.com/api/chat.postMessage" | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a constant variable since the string value won't change so we can use all capital letters to indicate this.
Suggested change
|
||||||||||||||||||
| headers = { | ||||||||||||||||||
| "Authorization": "Bearer " + token, | ||||||||||||||||||
| "Content-Type": "application/json; charset=utf-8" | ||||||||||||||||||
| } | ||||||||||||||||||
| payload = { | ||||||||||||||||||
| "channel": "test-slack-api", | ||||||||||||||||||
| "text": "Someone just completed the task: " + task.title | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| requests.post(url, headers=headers, json=payload) | ||||||||||||||||||
|
Comment on lines
+71
to
+83
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer this logic related to Slack to be encapsulated in a helper function, maybe something like @tasks_bp.patch("/<id>/mark_complete")
def mark_complete(id):
task = validate_task(id)
task.completed_at = datetime.now(timezone.utc)
db.session.commit()
call_slack_api()
return Response(status=204, mimetype="application/json") |
||||||||||||||||||
|
|
||||||||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||||||||
|
|
||||||||||||||||||
| @tasks_bp.patch("/<id>/mark_incomplete") | ||||||||||||||||||
| def mark_incomplete(id): | ||||||||||||||||||
| task = validate_task(id) | ||||||||||||||||||
|
|
||||||||||||||||||
| # clear completed_at | ||||||||||||||||||
| task.completed_at = None | ||||||||||||||||||
| db.session.commit() | ||||||||||||||||||
|
|
||||||||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||||||||
|
|
||||||||||||||||||
| @tasks_bp.get("/<id>") | ||||||||||||||||||
| def get_one_task(id): | ||||||||||||||||||
| task = validate_task(id) | ||||||||||||||||||
|
|
||||||||||||||||||
| task_data = { | ||||||||||||||||||
| "id": task.id, | ||||||||||||||||||
| "title": task.title, | ||||||||||||||||||
| "description": task.description, | ||||||||||||||||||
| "is_complete": bool(task.completed_at) | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+101
to
+106
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use the instance method
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| if task.goal_id is not None: | ||||||||||||||||||
| task_data["goal_id"] = task.goal_id | ||||||||||||||||||
|
Comment on lines
+108
to
+109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice if this logic was part of |
||||||||||||||||||
|
|
||||||||||||||||||
| return {"task": task_data}, 200 | ||||||||||||||||||
|
|
||||||||||||||||||
| @tasks_bp.put("/<id>") | ||||||||||||||||||
| def update_task(id): | ||||||||||||||||||
| task = validate_task(id) | ||||||||||||||||||
| request_body = request.get_json() or {} | ||||||||||||||||||
| if "title" not in request_body or "description" not in request_body: | ||||||||||||||||||
| return make_response({"details": "Invalid data"}, 400) | ||||||||||||||||||
|
|
||||||||||||||||||
| task.title = request_body["title"] | ||||||||||||||||||
| task.description = request_body["description"] | ||||||||||||||||||
|
|
||||||||||||||||||
| db.session.commit() | ||||||||||||||||||
|
|
||||||||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||||||||
|
|
||||||||||||||||||
| @tasks_bp.delete("/<id>") | ||||||||||||||||||
| def delete_task(id): | ||||||||||||||||||
| task = validate_task(id) | ||||||||||||||||||
| db.session.delete(task) | ||||||||||||||||||
| db.session.commit() | ||||||||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Single-database configuration for Flask. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # 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,flask_migrate | ||
|
|
||
| [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 | ||
|
|
||
| [logger_flask_migrate] | ||
| level = INFO | ||
| handlers = | ||
| qualname = flask_migrate | ||
|
|
||
| [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 |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prefer to name the
task_bpandgoal_bpjustbpin their respective route files, and then either do animport ashere to differentiate them, or import each entire route module and access its Blueprint using dot notation.