Shark - Ying #98
Shark - Ying #98
Conversation
There was a problem hiding this comment.
Great work Ying! Your code was very clean due to your well-written helper functions.
You have earned a 🟢 green grade for this project.
In this PR, you'll find comments focused on compliments and suggestions for refactoring. With internship in a few months, I'd like you to submit PRs as if a manager is reviewing your code. So in this case, removing unneeded whitespace, unused code, and unused imports are all things that should be removed prior to submitting a PR to be reviewed. Let's practice that in future projects!
Keep up the great work! 🦈 ✨
| from app.routes.routes import tasks_bp | ||
| app.register_blueprint(tasks_bp) | ||
| from app.routes.goal_routes import goals_bp | ||
| app.register_blueprint(goals_bp) |
There was a problem hiding this comment.
Good work in organizing your routes into separate files. My one minor suggestion is to change the routes file name to task_routes.
| goal_id = db.Column(db.Integer, primary_key=True, autoincrement=True) | ||
| title = db.Column(db.String) | ||
| tasks = db.relationship("Task", back_populates="goal", lazy = True) |
There was a problem hiding this comment.
👍 Nice work making the Goal class! We may want to consider whether we should storing empty/null titles for a goal is useful (should goals have no titles?). How might we change line 6, title = db.Column(db.String) to prevent null values?
|
|
||
|
|
There was a problem hiding this comment.
We can get rid of extra whitespace
| def to_json(self): | ||
| return { | ||
| "id": self.goal_id, | ||
| "title": self.title | ||
| } | ||
|
|
||
| @classmethod | ||
| def create_goal(cls, req_body): | ||
| new_goal = cls( | ||
| title = req_body["title"], | ||
| ) | ||
| return new_goal |
|
|
||
| class Task(db.Model): | ||
| task_id = db.Column(db.Integer, primary_key=True) | ||
| id = db.Column(db.Integer, primary_key=True, ) |
There was a problem hiding this comment.
We may want to add autoincrement=True just to communicate to other developers that the id is being autoincremented by postgres/SQLAlchemy
| assert response_body == {"message": "Goal1 not found"} | ||
| # assertion 2 goes here | ||
| assert response.status_code == 404 |
| response = client.put("/goals/1", json={ | ||
| "title": "Updated goal title" | ||
| }) | ||
| response_body = response.get_json() | ||
|
|
||
| # Assert | ||
| # ---- Complete Assertions Here ---- | ||
| # assertion 1 goes here | ||
| # assertion 2 goes here | ||
| # assertion 3 goes here | ||
| # ---- Complete Assertions Here ---- | ||
| 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) | ||
| assert goal.title == "Updated goal title" |
| # raise Exception("Complete test") | ||
| # Act | ||
| # ---- Complete Act Here ---- | ||
|
|
||
| response = client.put("/goals/1", json={ | ||
| "title": "Updated Goal Title", | ||
| }) | ||
| response_body = response.get_json() | ||
|
|
||
| # Assert | ||
| # ---- Complete Assertions Here ---- | ||
| # assertion 1 goes here | ||
| # assertion 2 goes here | ||
| # ---- Complete Assertions Here ---- | ||
| assert response.status_code == 404 | ||
| assert response_body == {'message': 'Goal1 not found'} |
| response = client.delete("/goals/1") | ||
| response_body = response.get_json() | ||
|
|
||
| # Assert | ||
| # ---- Complete Assertions Here ---- | ||
| # assertion 1 goes here | ||
| # assertion 2 goes here | ||
| # ---- Complete Assertions Here ---- | ||
| assert response_body == {'message': 'Goal1 not found'} | ||
| assert Goal.query.all() == [] |
| # **Complete test with assertion about response body*************** | ||
| # ***************************************************************** | ||
| # raise Exception("Complete test with assertion about response body") | ||
| assert response_body == {"message": "Goal1 not found"} |
Finished all waves