Conversation
tgoslee
left a comment
tgoslee
left a comment
There was a problem hiding this comment.
Good job Theresa. I added some comments on some ways you could refactor. Two tests in wave 3 and a few tests in Wave 6 seem to not be passing. Let's take a look in our 1:1.
app/__init__.py
Outdated
| # from app.models.task import Task | ||
| # from app.models.goal import Goal |
There was a problem hiding this comment.
I've got internet again. Removed lines 26 and 27
| goals_bp = Blueprint("goals_bp", __name__, url_prefix="/goals") | ||
|
|
||
| # Helper Function | ||
| def validate_goal(id): |
There was a problem hiding this comment.
like that you have specific error messages so the user knows exactly wants going on!
| response_body = { "goal": | ||
| { | ||
| "id": new_goal.goal_id, | ||
| "title": new_goal.title | ||
| } | ||
| }, 201 |
There was a problem hiding this comment.
How can you create an instance method or helper function so you don't have to repeat this code ?
There was a problem hiding this comment.
I can create an instance method but am unclear as to how to incorporate it in this function
app/goal_routes.py
Outdated
|
|
||
| @goals_bp.route("/<goal_id>/tasks", methods=["POST"]) | ||
|
|
||
| def post_list_of_ids(goal_id): |
There was a problem hiding this comment.
I would make this function name more specific. Something like post_tasks_for_goal
app/goal_routes.py
Outdated
| linked_task.goal_id = goal_id | ||
|
|
||
| db.session.commit() | ||
| print(goal.tasks) |
There was a problem hiding this comment.
remove print statements unless its beneficial for the user
| new_task = Task( | ||
| title=request_body["title"], | ||
| description=request_body["description"]) |
There was a problem hiding this comment.
you could create an instance method in your Task model to handle this
There was a problem hiding this comment.
Same as prior comment. Not clear as to how to introduce in Function
|
|
||
|
|
||
| return response_body | ||
| # return make_response(jsonify(response_body), 201) |
There was a problem hiding this comment.
you want to keep this line. It is best practice to use jsonfiy and return a code like 201
There was a problem hiding this comment.
Code 201 was included in the response body. There was an error when I used line 55 and therefore switched to this method
| # task.task_id = request_body["task_id"] | ||
| task.title = request_body["title"] | ||
| task.description = request_body["description"] | ||
| # task.completed_at = request_body["completed_at"] |
There was a problem hiding this comment.
make sure to remove commented code that isn't explaining how your code is working
|
|
||
|
|
||
|
|
||
| tasks_bp = Blueprint("tasks", __name__, url_prefix="/tasks") |
There was a problem hiding this comment.
Because this file is specific to tasks you could rename it to task_routes.py and you could move both of your routes to a routes directory to organize it a little better.
app/routes.py
Outdated
| # { "details": "Task {task_id} \ {descrtiption}\" successfully deleted" | ||
| # } |
2 participants
I will continue to try and resolve some fail tests