-
Notifications
You must be signed in to change notification settings - Fork 36
Crow-BCaprirolo-Task-List-API #28
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?
Changes from all commits
04b2817
d8c2173
04e6bc3
5f50661
e28ce9e
87082c7
179c464
e600e2a
ec28431
e3563fc
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,25 @@ | ||||||||||||||||||||||||||||||||||
| from sqlalchemy.orm import Mapped, mapped_column | ||||||||||||||||||||||||||||||||||
| from sqlalchemy.orm import Mapped, mapped_column, relationship | ||||||||||||||||||||||||||||||||||
| from ..db import db | ||||||||||||||||||||||||||||||||||
| from typing import TYPE_CHECKING | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if TYPE_CHECKING: | ||||||||||||||||||||||||||||||||||
| from .task import Task | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
Comment on lines
+5
to
7
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. While TYPE_CHECKING isn't necessarily required here because we have a one-to-many relationship, it is best practice and is necessary to avoid circular imports in a many-to-many relationship, so great job using it here. Also, as stated in our Learn lessons, it does allow us to get rid of the yellow underline on line 11! |
||||||||||||||||||||||||||||||||||
| class Goal(db.Model): | ||||||||||||||||||||||||||||||||||
| id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) | ||||||||||||||||||||||||||||||||||
| title: Mapped[str] | ||||||||||||||||||||||||||||||||||
| tasks: Mapped[list["Task"]] = relationship(back_populates="goal") | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def to_dict(self): | ||||||||||||||||||||||||||||||||||
| goal_as_dict = { | ||||||||||||||||||||||||||||||||||
| "id": self.id, | ||||||||||||||||||||||||||||||||||
| "title": self.title | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return goal_as_dict | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||||||
| def from_dict(cls, task_data): | ||||||||||||||||||||||||||||||||||
| new_task = cls(title=task_data['title']) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return new_task | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
+25
|
||||||||||||||||||||||||||||||||||
| def from_dict(cls, task_data): | |
| new_task = cls(title=task_data['title']) | |
| return new_task | |
| def from_dict(cls, goal_data): | |
| new_goal = cls(title=goal_data['title']) | |
| return new_goal |
Copilot
AI
Dec 21, 2025
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.
Inconsistent naming: the variable is named 'new_task' but it's actually a Goal object. It should be renamed to 'new_goal' for clarity.
| def from_dict(cls, task_data): | |
| new_task = cls(title=task_data['title']) | |
| return new_task | |
| def from_dict(cls, goal_data): | |
| new_goal = cls(title=goal_data['title']) | |
| return new_goal |
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.
You are currently in the Goal model but refer to task_data here. It would appear you copied and pasted from the Task model which is perfectly fine, but do make sure to make the appropriate changes!
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,50 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| from sqlalchemy.orm import Mapped, mapped_column | ||||||||||||||||||||||||||||||||||||||||||||||
| from sqlalchemy.orm import Mapped, mapped_column, relationship | ||||||||||||||||||||||||||||||||||||||||||||||
| from sqlalchemy import ForeignKey | ||||||||||||||||||||||||||||||||||||||||||||||
| from ..db import db | ||||||||||||||||||||||||||||||||||||||||||||||
| from datetime import datetime | ||||||||||||||||||||||||||||||||||||||||||||||
| from typing import Optional, TYPE_CHECKING | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if TYPE_CHECKING: | ||||||||||||||||||||||||||||||||||||||||||||||
| from .goal import Goal | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| class Task(db.Model): | ||||||||||||||||||||||||||||||||||||||||||||||
| id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) | ||||||||||||||||||||||||||||||||||||||||||||||
| title: Mapped[str] | ||||||||||||||||||||||||||||||||||||||||||||||
| description: Mapped[str] | ||||||||||||||||||||||||||||||||||||||||||||||
| goal_id: Mapped[Optional[int]] = mapped_column(ForeignKey("goal.id")) | ||||||||||||||||||||||||||||||||||||||||||||||
| goal: Mapped[Optional["Goal"]] = relationship(back_populates="tasks") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| completed_at: Mapped[Optional[datetime]] | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| #converts task instance into dictionary | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
| def to_dict(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| task_as_dict = {} | ||||||||||||||||||||||||||||||||||||||||||||||
| task_as_dict["id"]= self.id | ||||||||||||||||||||||||||||||||||||||||||||||
| task_as_dict["title"]= self.title | ||||||||||||||||||||||||||||||||||||||||||||||
| task_as_dict["description"]= self.description | ||||||||||||||||||||||||||||||||||||||||||||||
| task_as_dict["is_complete"]= self.completed_at is not None | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
+26
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 nice start, but it does start to get a little clunky. Feel free to favor building a dictionary directly:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if self.goal_id: | ||||||||||||||||||||||||||||||||||||||||||||||
| task_as_dict["goal_id"] = self.goal_id | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if self.completed_at: | ||||||||||||||||||||||||||||||||||||||||||||||
| task_as_dict["completed_at"]= self.completed_at.isoformat() | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+23
to
+32
|
||||||||||||||||||||||||||||||||||||||||||||||
| task_as_dict["id"]= self.id | |
| task_as_dict["title"]= self.title | |
| task_as_dict["description"]= self.description | |
| task_as_dict["is_complete"]= self.completed_at is not None | |
| if self.goal_id: | |
| task_as_dict["goal_id"] = self.goal_id | |
| if self.completed_at: | |
| task_as_dict["completed_at"]= self.completed_at.isoformat() | |
| task_as_dict["id"] = self.id | |
| task_as_dict["title"] = self.title | |
| task_as_dict["description"] = self.description | |
| task_as_dict["is_complete"] = self.completed_at is not None | |
| if self.goal_id: | |
| task_as_dict["goal_id"] = self.goal_id | |
| if self.completed_at: | |
| task_as_dict["completed_at"] = self.completed_at.isoformat() |
Copilot
AI
Dec 21, 2025
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.
Missing space after the equals sign. Should be 'task_as_dict["completed_at"] = self.completed_at.isoformat()' for consistent spacing with PEP 8 style guidelines.
| task_as_dict["completed_at"]= self.completed_at.isoformat() | |
| task_as_dict["completed_at"] = self.completed_at.isoformat() |
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.
This else statement currently does not do anything, so it can be removed completely.
Copilot
AI
Dec 21, 2025
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.
Comment is missing a space after the hash symbol. Should be '# creates a task from a dictionary' for consistency with Python style guidelines.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1 +1,89 @@ | ||||||||||||||||||||||||||||
| from flask import Blueprint | ||||||||||||||||||||||||||||
| from flask import Blueprint, request | ||||||||||||||||||||||||||||
| from app.models.goal import Goal | ||||||||||||||||||||||||||||
| from app.models.task import Task | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| from .route_utilities import * | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
Comment on lines
+5
to
+6
|
||||||||||||||||||||||||||||
| from .route_utilities import * | |
| from .route_utilities import ( | |
| validate_model, | |
| db, | |
| create_model, | |
| get_all_items, | |
| get_one_item, | |
| update_entire_item, | |
| update_partial_item, | |
| mark_item_complete, | |
| mark_item_incomplete, | |
| delete_item, | |
| ) |
Copilot
AI
Dec 21, 2025
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.
Comment is missing a space after the hash symbol. Should be '# add new tasks' for consistency with Python style guidelines.
| #add new tasks | |
| # add new tasks |
Copilot
AI
Dec 21, 2025
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.
Missing parentheses for db.session.commit(). This will not actually commit the transaction to the database, causing the new task to not be persisted.
| db.session.commit | |
| db.session.commit() |
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.
Flask returns status code 200 by default so we don't need to explicitly return it! We can go ahead and just return the new task's dictionary.
Copilot
AI
Dec 21, 2025
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.
This line returns the function object itself rather than calling it. It should be 'return create_new_task_for_goal(goal, request_body)' to actually invoke the function with the necessary arguments.
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.
Overall, this view function is simply associating a goal with tasks and we're not really creating new Tasks at any point. A couple of things you could change:
-
No need for an
associate_tasks_with_goalshelper function. Feel free to move the logic in that helper function here because it will still follow the single responsibility principle. -
As Copilot mentioned, you are returning a function object as opposed to what gets returned from a function call. To me, that means the
create_new_task_for_goalhelper function is never actually called which means it isn't really necessary.
Copilot
AI
Dec 21, 2025
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.
Missing space between 'return' and the opening brace. Should be 'return {' for consistency with the rest of the codebase.
| return{ | |
| return { |
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.
This looks great! One small thing you could tweak would be to tweak your Goal model's to_dict() function to do all of this for you!
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.
Nice job with these view functions! I know that in the past I have mentioned that single line functions are not always ideal. What sets these apart is that we need a view function for each of our CRUD routes, but because our routes are so similar across models, we end up repeating a lot of code. These single line functions give us a lot more flexibility across functions!
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,138 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from flask import abort, make_response, request, Response | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ..db import db | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from datetime import datetime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import requests | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def validate_model(cls, id): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id = int(id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except ValueError: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = {"message": f"{cls.__name__} id {id} is invalid"} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| abort(make_response(response, 400)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| query = db.select(cls).where(cls.id == id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item = db.session.scalar(query) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not item: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = {"message": f"{cls.__name__} {id} not found"} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| abort(make_response(response, 404)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return item | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def send_slack_notification(item_title): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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 was a smart move to go ahead and include the slack API request as a helper function! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| slack_token = os.environ.get("SLACK_TOKEN") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if slack_token: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| requests.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "https://slack.com/api/chat.postMessage", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers={"Authorization": f"Bearer {slack_token}"}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| json={ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "channel": "slack-api-testing", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "text": f"Someone just completed {item_title}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Slack notification failed: {e}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '''none of these probably go here, but it seemed easier at the time... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| & while my code is solid, my brain is delicate & easy to break.''' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+40
to
+41
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 beg to differ! I think this is a great spot for all of these! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def create_model(cls, request_body): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| new_item = cls.from_dict(request_body) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except KeyError: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = {"details": "Invalid data"} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| abort(make_response(response, 400)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.session.add(new_item) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.session.commit() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new_item.to_dict(), 201 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def get_all_items(cls): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| query = db.select(cls) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| title_param = request.args.get('title') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if title_param: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| query = query.where(cls.title.ilike(f"{title_param}%")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sort_param = request.args.get('sort', 'asc') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if sort_param == "desc": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| query = query.order_by(cls.title.desc()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| query = query.order_by(cls.title.asc()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| items = db.session.scalars(query) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| items_response = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for item in items: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| items_response.append(item.to_dict()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return items_response | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+55
to
+74
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. Overall, this is great! I specifically love that you have included both the filtering and the sorting in this helper! One big tweak that you could make comes from the fact that there are a variety of potential filters from model to model. As a result, you could use something similar to the following code to work with any filter as opposed to just title:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def get_one_item(cls, id): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item = validate_model(cls, id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return item.to_dict() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+76
to
+79
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 function feels redundant, but because retrieving a single item will look similar from model to model, I think it's a good idea to have it here! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def update_entire_item(cls, id, goal_update=False): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item = validate_model(cls, id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request_body = request.get_json() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item.title = request_body["title"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not goal_update: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item.description = request_body["description"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.session.commit() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return item | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+81
to
+90
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. Honestly, I'm consistently impressed by how your brain works with these types of problems. I was initially unsure of how this particular helper function would work, but you made it pretty flexible. That being said, if we were to add more and more models, this would become less stable and would need to be tweaked in order to make sure it can apply to more models. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def update_partial_item(cls, id): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item = validate_model(cls, id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request_body = request.get_json() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if 'title' in request_body: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item.title = request_body['title'] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if 'description' in request_body: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item.description = request_body['description'] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if 'is_complete' in request_body: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if request_body['is_complete']: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item.completed_at = datetime.now() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item.completed_at = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+96
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. The small issue I see here again is its scalability. If we had more models with different attributes, then we'll have to add more if statements which isn't entirely feasible. In its current form, it also lends itself to the issue of trying to access attributes that don't exist to different models. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.session.commit() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def mark_item_complete(cls, id): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item = validate_model(cls, id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item.completed_at = datetime.now() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.session.commit() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| send_slack_notification(item.title) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def mark_item_incomplete(cls, id): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item = validate_model(cls, id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item.completed_at = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.session.commit() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def delete_item(cls, id): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| item = validate_model(cls, id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.session.delete(item) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.session.commit() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Response(status=204, mimetype="application/json") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+112
to
+138
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. These three look great! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1 +1,44 @@ | ||||||||||||||||||||||||||||||||
| from flask import Blueprint | ||||||||||||||||||||||||||||||||
| from flask import Blueprint | ||||||||||||||||||||||||||||||||
| from app.models.task import Task | ||||||||||||||||||||||||||||||||
| from .route_utilities import * | ||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+3
|
||||||||||||||||||||||||||||||||
| from flask import Blueprint | |
| from app.models.task import Task | |
| from .route_utilities import * | |
| from flask import Blueprint, request, Response | |
| from app.models.task import Task | |
| from .route_utilities import ( | |
| create_model, | |
| get_all_items, | |
| get_one_item, | |
| update_entire_item, | |
| update_partial_item, | |
| mark_item_complete, | |
| mark_item_incomplete, | |
| delete_item, | |
| ) |
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.
Extra whitespace! We can get rid of this!
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Single-database configuration for Flask. |
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.
Nice job aliasing your blueprints!