Conversation
…asks with all Wave 1 tests passing
… routes, Waves 2 and 3 passing
…ne_goal, update_goal, delete_goal
…ses Wave 5 and 6 tests
spitsfire
left a comment
spitsfire
left a comment
There was a problem hiding this comment.
Heck yeah, Marie! I have to say, I didn't find a lot to give feedback on. I thought your code structuring was very beautiful, easy to read and understand.
I gave one or two suggestions to think about to improve readability or syntax, but that's about it! Great job!
| title = db.Column(db.String) | ||
| description = db.Column(db.String) |
There was a problem hiding this comment.
It turns out that nullable=True is the default value for nullable. So all of the columns for Task here are currently marked as nullable. But should title or description be allowed to be NULL? (Does that make sense from a data standpoint?) Consider adding nullable=False to those columns.
The way the project emphasized that completed_at needs to accept NULL values may make it seem like we needed to explicitly call out that nullable should be True, but it turns out this is the default for nullable. Instead, we should think about the other data in our model and consider whether it makes sense for any of it to be NULL. If not, we can have the database help us protect against that happening!
|
|
||
| class Goal(db.Model): | ||
| goal_id = db.Column(db.Integer, primary_key=True) | ||
| title = db.Column(db.String) |
There was a problem hiding this comment.
See the Task model for more information about required attributes being given nullable=True
|
|
||
| @classmethod | ||
| def from_dict(cls, goal_data): | ||
| new_goal = Goal(title=goal_data["title"]) |
There was a problem hiding this comment.
This works! But what if we changed the name of the model? Now this would continue creating Goal which no longer exists, so let's take advantage of the cls parameter. This will represent the current model, even if we change its name.
new_goal = cls(title=goal_data["title"])|
|
||
| @classmethod | ||
| def from_dict(cls, task_data): | ||
| new_task = Task(title=task_data["title"], |
There was a problem hiding this comment.
new_task = cls(title=task_data["title"],| task.title = request_body["title"] | ||
| task.description = request_body["description"] |
There was a problem hiding this comment.
We could turn this into a helper method in Task model
| db.session.delete(task) | ||
| db.session.commit() | ||
|
|
||
| return make_response({"details":f"Task {task.task_id} \"{task.title}\" successfully deleted"}), 200 |
There was a problem hiding this comment.
This works! We could also do this:
return make_response({"details":f"Task {task.task_id} '{task.title}' successfully deleted"}), 200We can take advantage of the two different sets of quotes
| for task in goal.tasks: | ||
| task_list.append(task.to_dict()) | ||
|
|
||
| return make_response({"id": goal.goal_id, "title": goal.title, "tasks": task_list}) |
There was a problem hiding this comment.
whoops! forgot your status code!
return make_response({"id": goal.goal_id, "title": goal.title, "tasks": task_list}, 200)| db.session.delete(goal) | ||
| db.session.commit() | ||
|
|
||
| return make_response({"details":f"Goal {goal.goal_id} \"{goal.title}\" successfully deleted"}), 200 No newline at end of file |
There was a problem hiding this comment.
This works! We could also do this:
return make_response({"details":f"Goal {goal.goal_id} '{goal.title}' successfully deleted"}), 200We can take advantage of the two different sets of quotes
| from dotenv import load_dotenv | ||
| load_dotenv() |
There was a problem hiding this comment.
We can get rid of this since we aren't access the environmental variables in this file!
| from dotenv import load_dotenv | |
| load_dotenv() |
2 participants
No description provided.