Sharks - Morgan B. and Huma H. #21
Conversation
app/routes.py
Outdated
| try: | ||
| id = int(id) | ||
| except: | ||
| abort(make_response({"message": f"Planet {id} is not valid"}, 400)) | ||
|
|
||
| for planet in planets: | ||
| if planet.id == id: | ||
| return planet | ||
|
|
||
| return abort(make_response({"message": f"Planet {id} not found"}, 404)) |
| return abort(make_response({"message": f"Planet {id} not found"}, 404)) | ||
|
|
||
| @planets_bp.route("", methods=["GET"]) | ||
| def get_planets(): |
There was a problem hiding this comment.
Nice job naming the method to describe what this route does.
In the future, when you split your routes up into distinct route files (cat_routes.py and dog_routes.py for example in a routes directory) then you could have method names like get_all() and get_one() since we know that all the routes in cat_routes.py are related to the Cat class.
app/routes.py
Outdated
| for planet in planets: | ||
| planet_response_body.append(planet.to_json()) | ||
|
|
||
| return jsonify(planet_response_body) |
There was a problem hiding this comment.
Adding 200 status code adds clarity even though it happens by default
return jsonify(planet_response_body), 200
app/routes.py
Outdated
| def read_one_planet(planet_id): | ||
| planet = validate_planet(planet_id) | ||
|
|
||
| return jsonify(planet.to_json()) |
There was a problem hiding this comment.
Same comment as above, you can return status code 200 here
| def create(cls, req_body): | ||
| new_planet = cls( | ||
| name=req_body["name"], | ||
| description=req_body["description"], | ||
| moon_count=req_body["moon_count"] | ||
| ) | ||
| return new_planet |
There was a problem hiding this comment.
Nice work refactoring this into a class method so that your route stays short. How would you add in validation to make sure that all the required fields are sent in the request to your server?
For example, if someone sent a POST request but left off moon_count, then line 27 would throw KeyError that it couldn't find moon_count.
it would be nice to handle the error and return a message so that the client knows their request was invalid and they need to include moon_count. Something to think about for Task List.
| def update(self, req_body): | ||
| self.name = req_body["name"] | ||
| self.description = req_body["description"] | ||
| self.moon_count = req_body["moon_count"] |
There was a problem hiding this comment.
Same thought here about validating the PUT request fields that your server needs to handle as with POST. How can you validate and handle the KeyError if a field is not present in the request?
| @@ -0,0 +1,39 @@ | |||
| import pytest | |||
| @@ -0,0 +1,65 @@ | |||
| from urllib import response | |||
No description provided.