Sea Turtles - Tiffini H. #99
Conversation
…tionality, passes wave 03
| from .task_routes import tasks_bp | ||
| app.register_blueprint(tasks_bp) | ||
|
|
||
| from .goal_routes import goals_bp | ||
| app.register_blueprint(goals_bp) |
There was a problem hiding this comment.
Nice, I like the choice to divide the routes into their own files!
| @@ -0,0 +1,90 @@ | |||
| from flask import Blueprint, request, jsonify, make_response, abort | |||
There was a problem hiding this comment.
It looks like the abort function isn't directly used in this file anymore, so we should remove the import as part of our clean up & refactor steps.
There was a problem hiding this comment.
Oh man, I thought I caught all of those! Thanks, Kelsey!
| if self.tasks: | ||
| goal_dict["tasks"] = [task.task_id for task in self.tasks] |
There was a problem hiding this comment.
Nice way to add the attribute that won't always exist!
|
|
||
| if self.tasks: | ||
| goal_dict["tasks"] = [task.task_id for task in self.tasks] | ||
| # goal_dict["tasks"] = [] |
There was a problem hiding this comment.
We want to make sure to delete commented out code as part of clean up. When we are sharing a codebase with other folks, it's unclear what the intention of commented out code is, especially without extra comments explaining why it's there. We use versioning tools like git so we can confidently delete code and view our commit history if we need to recover something we wrote prior.
There was a problem hiding this comment.
Noted--thank you for catching this!
| if not self.completed_at: | ||
| task_dict["is_complete"] = False | ||
| else: | ||
| task_dict["is_complete"] = True |
There was a problem hiding this comment.
It isn't necessary, but we could use a ternary operator here:
task_dict["is_complete"] = True if self.completed_at else Falseor the bool function:
task_dict["is_complete"] = bool(self.completed_at)| @@ -0,0 +1,101 @@ | |||
| from flask import Blueprint, request, jsonify, make_response, abort | |||
There was a problem hiding this comment.
The feedback about abort in goal_routes.py applies here as well.
| db.session.delete(task) | ||
| db.session.commit() | ||
|
|
||
| return make_response({"details": f'Task {task_id} "{task.title}" successfully deleted'}) |
There was a problem hiding this comment.
Nice, I like the informative messages.
| def test_get_task_not_found(client): | ||
| # Act | ||
| response = client.get("/tasks/1") | ||
| response_body = response.get_json() | ||
|
|
||
| # Assert | ||
| assert response.status_code == 404 | ||
| assert response_body == {"details": f"Task #1 not found"} |
There was a problem hiding this comment.
To ensure we're checking all the relevant data, it might be good to confirm that the database has no records, or that there is no record in the DB with an ID of 1. (This feedback around checking the DB applies to other tests as well.)
| # assertion 2 goes here | ||
| # assertion 3 goes here | ||
| assert response.status_code == 200 | ||
| assert "goal" in response_body |
There was a problem hiding this comment.
Since we check the whole structure of response_body on the line below which confirms that goal is in the response, we could omit this specific check for goal in response_body.
| "title": "Updated Goal Title" | ||
| } | ||
| } | ||
| assert goal.title == "Updated Goal Title" |
There was a problem hiding this comment.
Nice checking the DB contents!
No description provided.