Skip to content

Sphinx C22 - Aleida V#33

Open
aleidavi wants to merge 14 commits intoAda-C22:mainfrom
aleidavi:main
Open

Sphinx C22 - Aleida V#33
aleidavi wants to merge 14 commits intoAda-C22:mainfrom
aleidavi:main

Conversation

@aleidavi
Copy link

@aleidavi aleidavi commented Nov 8, 2024

No description provided.

Copy link

@mikellewade mikellewade left a comment

Choose a reason for hiding this comment

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

Great work, Aleida! If you have any questions about the comments left please do not hesitate to reach out. ⭐️

Comment on lines +29 to +38
def to_nested_dict(self):
goal_to_dict = {}
goal_to_dict["id"] = self.id
goal_to_dict["title"] = self.title
if not self.check_goal_tasks():
goal_to_dict["tasks"] = []
else:
goal_to_dict["tasks"] = self.check_goal_tasks()

return goal_to_dict

Choose a reason for hiding this comment

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

Notice that this method and the Task method of the same name are virtually the same. We could possibly take this logic and place it in a helper or placed it on the Base class to be inherited by the these children models.

Comment on lines +18 to +19
goal_id: Mapped[Optional[int]] = mapped_column(ForeignKey("goal.id"))
goal: Mapped[Optional["Goal"]] = relationship(back_populates="tasks")

Choose a reason for hiding this comment

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

Nice work establishing this relationship and attribute of the Goal this tasks belongs to!

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")

Choose a reason for hiding this comment

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

⭐️

tasks_assigned = []
if self.tasks:
for task in self.tasks:
task = validate_task(task.id)

Choose a reason for hiding this comment

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

If a task is found on self.tasks then we already know that it is a valid task and therefore we don't need to validate it.

Comment on lines +29 to +36
def to_nested_dict(self):
goal_to_dict = {}
goal_to_dict["id"] = self.id
goal_to_dict["title"] = self.title
if not self.check_goal_tasks():
goal_to_dict["tasks"] = []
else:
goal_to_dict["tasks"] = self.check_goal_tasks()

Choose a reason for hiding this comment

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

We have an endpoint that will provide us with the same data that goal_to_dict["tasks"] will give us so we don't need to put it on the dictionary representation of the object. If I client has the information to retrieve a Goal (i.e. goal.id) then they have the information to get the same information from our endpoint.

Comment on lines +74 to +87
@bp.patch("/<task_id>/mark_incomplete")
def mark_task_incomplete(task_id):

current_task = validate_task(task_id)

if current_task.completed_at:
current_task.completed_at = None

db.session.add(current_task)
db.session.commit()

response = {"task": current_task.to_dict()}

return jsonify(response)

Choose a reason for hiding this comment

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

Nice work with this function, remember we don't necessarily need to use jsonify here since Flask will know how to adapt dictionaries and lists into json.

Comment on lines +93 to +104
query = db.select(Task)

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

elif title_sort and title_sort == "desc":
query = query.order_by(Task.title.desc())

tasks = db.session.scalars(query)
tasks_response = [task.to_dict() for task in tasks]

Choose a reason for hiding this comment

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

Reference the refactoring we did for Flasky where we looked at a helper function that would make it to where we could filter the search for records on any table.

Comment on lines +66 to +67
assert "details" in response_body
assert response_body["details"] == "Task 1 not found"

Choose a reason for hiding this comment

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

👍🏿

Comment on lines +101 to +111
assert response.status_code == 200
assert "goal" in response_body
# assertion 2 goes here
goal = Goal.query.get(1)
assert response_body == {"goal": {
"id": 1,
"title": "Updated Goal Title"
} }
# assertion 3 goes here
assert goal.id == 1
assert goal.title == "Updated Goal Title"

Choose a reason for hiding this comment

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

⭐️

assert response.status_code == 404

raise Exception("Complete test with assertion about response body")
# raise Exception("Complete test with assertion about response body")

Choose a reason for hiding this comment

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

Think you forgot to complete this test.

aleidavi and others added 4 commits December 20, 2024 11:50
Adding comments to understand the dunder init file
updating the config lines in the dunder init file for the task-list-api project
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants