Panthers - Kallie and Billie resubmission #140
Conversation
There was a problem hiding this comment.
Great work Kallie and Billie! Your code was very clean due to helper functions and instance/class methods!
Y'all 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! 🐈⬛ ✨
| def validate_goal(goal_id): | ||
| try: | ||
| goal_id = int(goal_id) | ||
| except: | ||
| abort(make_response({"message":f"Goal {goal_id} invalid"}, 400)) | ||
|
|
||
| goal = Goal.query.get(goal_id) | ||
|
|
||
| if not goal: | ||
| abort(make_response({"message":f"Goal {goal_id} not found"}, 404)) | ||
|
|
||
| return goal |
| @goals_bp.route("", methods=["POST"]) | ||
| def create_goal(): | ||
| request_body = request.get_json() | ||
|
|
||
| if "title" not in request_body: | ||
| return make_response({"details": "Invalid data"}, 400) | ||
|
|
||
| new_goal = Goal(title = request_body["title"]) | ||
|
|
||
| db.session.add(new_goal) | ||
| db.session.commit() | ||
|
|
||
| return make_response({"goal":new_goal.to_dict()}, 201) |
There was a problem hiding this comment.
👍 Great work using to_dict() here! Because we're returning a dictionary, we can take advantage of Flask's default behavior of turning dictionaries into JSON format as according to their documentation:
If you return a dict from a view, it will be converted to a JSON response.
https://flask.palletsprojects.com/en/1.1.x/quickstart/#apis-with-json
We can apply this same comment to the rest of this project.
| @goals_bp.route("", methods=["GET"]) | ||
| def get_goals(): | ||
| goal_query = Goal.query | ||
| goals = goal_query.all() | ||
|
|
||
| goals_response = [] | ||
| for goal in goals: | ||
| goals_response.append({ | ||
| "id": goal.goal_id, | ||
| "title": goal.title, | ||
| }) | ||
|
|
||
| return make_response(jsonify(goals_response), 200) |
There was a problem hiding this comment.
👍 Nice work! We can use list comprehension to build goal_response and use to_dict like so:
goals_response = [goal.to_dict() for goal in goals]
return jsonify(goals_response), 200| @goals_bp.route(GOAL_ID_PREFIX, methods=["GET"]) | ||
| def get_one_goal(goal_id): | ||
| goal = validate_goal(goal_id) | ||
| goal = Goal.query.get(goal_id) | ||
|
|
||
| return {"goal": goal.to_dict()} |
| @goals_bp.route(GOAL_ID_PREFIX,methods=['PUT']) | ||
| def update_task(goal_id): | ||
| request_body = request.get_json() | ||
| if "title" not in request_body: | ||
| return make_response("Invalid Request, Goal Must Have Title", 400) | ||
|
|
||
| goal = validate_goal(goal_id) | ||
| goal = Goal.query.get(goal_id) | ||
|
|
||
| goal.title = request_body["title"] | ||
|
|
||
| db.session.commit() | ||
| return make_response({"goal":goal.to_dict()}, 200) |
| assert response.status_code == 404 | ||
| assert response_body == {"message":f"Goal 1 not found"} |
| assert response.status_code == 200 | ||
| assert "goal" in response_body | ||
| assert response_body == { | ||
| "goal": { | ||
| "id": 1, | ||
| "title": "My Updated New Goal" | ||
| } | ||
| } | ||
| goal = Goal.query.get(1) | ||
| assert goal.title == "My Updated New Goal" |
| response = client.put("/goals/1", json={ | ||
| "title": "Updated Task 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":f"Goal 1 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.status_code == 404 | ||
| assert response_body == {"message":f"Goal 1 not found"} | ||
|
|
||
| assert Goal.query.all() == [] |
| # ***************************************************************** | ||
| # **Complete test with assertion about response body*************** | ||
| # ***************************************************************** | ||
| assert response_body == {"message":f"Goal 1 not found"} |
No description provided.