Possom - Anaiahm by Anaiahm · Pull Request #35 · Ada-C24/task-list-api

Anaiahm · 2025-11-07T17:54:41Z

No description provided.

…est_wave_01

…e is found

…ed to a goal

yangashley · 2025-12-09T23:32:29Z

+    response = client.put("/goals/1", json={
+        "title": "Updated Goal Title"
+    })
+    goal = Goal.query.get(1)


❗️get is a legacy method that is deprecated and shouldn't be used (unless you're maintaining an older project that has this syntax).

Our tests should use syntax similar to what we have in our route_utilities functions: something like db.select with db.sessions.scalar

yangashley · 2025-12-09T23:36:01Z

@@ -1 +1,76 @@
-from flask import Blueprint
+from flask import Blueprint, make_response, request, Response, abort


make_response and abort are imported but not accessed and should be removed from the imports here to keep your project neat and free from unnecessary clutter.

yangashley · 2025-12-09T23:36:24Z

@@ -1 +1,66 @@
-from flask import Blueprint
+from flask import Blueprint, request, Response, session


session is imported but not accessed and should be removed.

yangashley · 2025-12-09T23:42:29Z

+    def to_dict(self):
+        return {
+            "id": self.id,
+            "title": self.title,
+        }
+
+    def to_dict_with_tasks(self):
+        return {
+            "id": self.id,
+            "task_ids": [task.id for task in self.tasks]
+        }


We can include the logic related to task_ids in to_dict by using conditional logic so that we don't need to create a second function.

Suggested change

def to_dict(self):

return {

"id": self.id,

"title": self.title,

}

def to_dict_with_tasks(self):

return {

"id": self.id,

"task_ids": [task.id for task in self.tasks]

}

def to_dict(self):

goal = {

"id": self.id,

"title": self.title,

}

if self.tasks:

goal["task_ids"] = [task.id for task in self.tasks]

return goal

yangashley · 2025-12-09T23:45:04Z

+    # def to_dict_with_goal(self):
+    #     task_dict = self.to_dict()
+    #     task_dict["goal_id"] = self.goal_id
+    #     return task_dict


Remove unnecessary commented out code to keep your codebase concise and to prevent bugs from being introduced.

Suggested change

# def to_dict_with_goal(self):

# task_dict = self.to_dict()

# task_dict["goal_id"] = self.goal_id

# return task_dict

yangashley · 2025-12-10T00:04:23Z

+    tasks_response = [task.to_dict() for task in goal.tasks]
+
+
+    return {
+        "id": goal.id,
+        "title": goal.title,
+        "tasks": tasks_response
+    }, 200


I'd prefer this logic related to creating a dictionary that represents a goal and its tasks to be in the to_dict instance method for goal so that this route can be more single
responsibility and tidy.

Your to_dict could check to see that a goal has tasks and if it does, then it can handle creating a dictionary that has the key tasks and value as a list of dictionaries representing each task.

With that refactor, this route would only need to be:

Suggested change

tasks_response = [task.to_dict() for task in goal.tasks]

return {

"id": goal.id,

"title": goal.title,

"tasks": tasks_response

}, 200

return goal.to_dict()

And no need to explicitly return 200 status code since Flask will do that for us.

yangashley · 2025-12-10T00:06:25Z

+    response_body = task.to_dict()
+    return response_body


You initialize the variable response_body but don't do anything with the variable besides return it on the next line. You can directly return the task dictionary, like:

Suggested change

response_body = task.to_dict()

return response_body

return task.to_dict()

yangashley · 2025-12-10T00:09:12Z

+def mark_task_incomplete(id):
+    task = validate_model(Task, id)
+    task.completed_at = None
+    task.is_complete = False


We don't need to set is_complete because this is not an attribute on the model. is_complete is a key in the dictionary that represents a task and this value is computed from completed_at.

Suggested change

task.is_complete = False

yangashley · 2025-12-10T00:15:35Z

+    task = validate_model(Task, id)
+    task.completed_at = datetime.now()
+
+
+    db.session.commit()
+    return Response(status=204, mimetype="application/json")


Like I had mentioned earlier in our Slack conversation, this route needs to implement a call to Slack so that when a task is completed then a message is posted to Slack.

👀❗️ You can review the Wave 4 documentation here for details about this.

yangashley · 2025-12-10T00:20:41Z

-    # assertion 2 goes here
-    # assertion 3 goes here
+    assert response.status_code == 204
+    assert goal.title == "Updated Goal Title"


We could also check that the response's has json format:

Suggested change

assert goal.title == "Updated Goal Title"

assert goal.title == "Updated Goal Title"

assert response.mimetype == "application/json"

yangashley · 2025-12-22T20:30:49Z

+    SLACK_BOT_TOKEN = os.environ.get("SLACK_BOT_TOKEN")
+    slack_message_text = f"Someone just completed the task {task.title}"
+    slack_channel = "task-notifications"
+    slack_url = "https://slack.com/api/chat.postMessage"
+    headers = {
+        "Authorization": f"Bearer {SLACK_BOT_TOKEN}",
+        "Content-Type": "application/json"
+    }
+    json = {
+        "channel": slack_channel,
+        "text": slack_message_text
+    }
+
+    response = requests.post(slack_url, headers=headers, json=json)
+    try:
+        requests.post(slack_url, headers=headers, json=json)
+        print(response.status_code, response.text)
+    except Exception as e:
+        print("Slack request failed:", e)


I'd prefer all the logic related to making a request to the Slack API to be in a separate function, maybe something called like make_slack_request. Doing so would keep this route more concise and concerned with fewer responsibilities.

If you invoke a helper function for making a call to Slack, then your whole route would look like:

@task_list_bp.patch("/<id>/mark_complete") def mark_task_complete(id): task = validate_model(Task, id) task.completed_at = datetime.now() make_slack_request() return Response(status=204, mimetype="application/json")

yangashley · 2025-12-22T20:31:39Z

+    slack_channel = "task-notifications"
+    slack_url = "https://slack.com/api/chat.postMessage"


Nice job using variables to refer these string values so that they don't magically appear in your code.

Since they are constant variables, naming them with all caps would convey to other devs that these values should not be changed.

Suggested change

slack_channel = "task-notifications"

slack_url = "https://slack.com/api/chat.postMessage"

SLACK_CHANNEL = "task-notifications"

SLACK_URL = "https://slack.com/api/chat.postMessage"

yangashley · 2025-12-22T20:35:30Z

+    response = requests.post(slack_url, headers=headers, json=json)
+    try:
+        requests.post(slack_url, headers=headers, json=json)
+        print(response.status_code, response.text)


We should remove debugging print statements when we no longer need them so they're not part of the codebase.

Suggested change

print(response.status_code, response.text)

Anaiahm added 13 commits October 30, 2025 14:30

added .env, added initial migrations

171c279

added Task class, ran migrations

4403b27

Added model helper method to Task class and two additional tests to t…

11a9105

…est_wave_01

Added routes to CRUD tasks

f0effca

Added query sort by title asc/desc functionality

d9b1929

added Patch routes for Wave_03

04f960d

added goal class

5048d2d

added goal-task relationship

cf06d24

changed 'title' lookup in from_dict() to raise a key error if no titl…

7e080ba

…e is found

completed tests to assert deleting & updating goals

c32d0c0

added one-to-many relationship between goals and tasks

6d0290b

updated routes so that goal_id is included only when tasks are assign…

5884d97

…ed to a goal

completed wave 7 tests to assert expected response body and code

37ba3cb

yangashley reviewed Dec 10, 2025

View reviewed changes

added functionality to post in slack chanel when a task is completed

19f055e

yangashley reviewed Dec 22, 2025

View reviewed changes

		@@ -1 +1,76 @@
		from flask import Blueprint No newline at end of file
		from flask import Blueprint, make_response, request, Response, abort

		@@ -1 +1,66 @@
		from flask import Blueprint No newline at end of file
		from flask import Blueprint, request, Response, session

	response_body = task.to_dict()
	return response_body
	return task.to_dict()

		slack_channel = "task-notifications"
		slack_url = "https://slack.com/api/chat.postMessage"

Conversation

Anaiahm commented Nov 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants