Conversation
alope107
left a comment
There was a problem hiding this comment.
Awesome job Alma! This was a huge project and it's all looking great! Very solid on both the models and routes. This project is definitely green! 🎉
| # Initialize SQLAlchemy | ||
| db = SQLAlchemy() | ||
| migrate = Migrate() | ||
| DATABASE_CONNECTION_STRING='postgresql+psycopg2://postgres:postgres@localhost:5432/task_list_api_development' |
There was a problem hiding this comment.
Because you now get this string from environment variables you no longer need this line.
| def to_dict(self): | ||
| return{ | ||
| "id": self.goal_id, | ||
| "title": self.title | ||
| } |
| def to_dict_with_tasks(self, goal_id): | ||
| tasks = Task.query.filter_by(fk_goal_id=f"{goal_id}") | ||
| task_list = [] | ||
| for task in tasks: | ||
| task_list.append(task.to_dict()) | ||
| return{ | ||
| "id":self.goal_id, | ||
| "title":self.title, | ||
| } No newline at end of file |
There was a problem hiding this comment.
It looks like this isn't ever used and can be removed.
| "id":self.task_id, | ||
| "title": self.title, | ||
| "description": self.description, | ||
| "is_complete": True if self.completed_at else False, |
| if self.goal_id: | ||
| return { | ||
| "id":self.task_id, | ||
| "title": self.title, | ||
| "description": self.description, | ||
| "is_complete": True if self.completed_at else False, | ||
| "goal_id": self.goal_id | ||
| } | ||
|
|
||
| else: | ||
| return{ | ||
| "id":self.task_id, | ||
| "title": self.title, | ||
| "description": self.description, | ||
| "is_complete": True if self.completed_at else False | ||
| } |
There was a problem hiding this comment.
This definitely works! Can you think of a way to restructure it so we set up most of the dictionary outside of the if/else and only add the "goal_id" key when needed? This could help reduce repetition in your code.
| if not goal: | ||
| return make_response("", 404) |
There was a problem hiding this comment.
We don't need this part. Goal.query.all() should only give us valid goals back.
|
|
||
| @goals_bp.route("/<goal_id>", methods=["GET", "PUT", "DELETE"]) | ||
| def handle_goal(goal_id): | ||
| goal_id=int(goal_id) |
There was a problem hiding this comment.
Make sure to do input validation here - if the user passed in an invalid goal_id this code would crash.
| answer = { | ||
| "id": goal.goal_id, | ||
| "title": goal.title, | ||
| "tasks": [task.to_dict() for task in goal.tasks] | ||
| } |
| goal.tasks.append(task) | ||
|
|
||
| db.session.commit() | ||
| return jsonify({"id":goal.goal_id, "task_ids": [task.task_id for task in goal.tasks]}), 200 |
| assert response.status_code == 200 | ||
| assert "goal" in response_body | ||
| assert response_body == { | ||
| "goal": { | ||
| "id": 1, | ||
| "title": "Updated Goal Title" | ||
| } | ||
| } | ||
| goal = Goal.query.get(1) |
There was a problem hiding this comment.
Nice job asserting what should be in the response body! It would also be a good idea to assert things about the goal we get back from querying the DB. Perhaps something like assert goal.title == "Updated Goal Title"
got to Wave 5