Sharks: Kassidy Buslach and Gabriela Ibarra#5
Conversation
| earth = Planet(1,"Earth","home planet","blue, brown, green, and white") | ||
| mercury = Planet(2, "Mercury", "closest planet to the sun", "grey") | ||
| neptune = Planet(3, "Neptune", "eight planet, farthest from sun", "blue") | ||
| jupiter = Planet(4, "Jupiter", "fifth planet from the sun, largest planet", "brown, orange and tan") | ||
|
|
||
| planets = [earth, mercury, neptune, jupiter] |
There was a problem hiding this comment.
Nice, you can also instantiate these objects directly in your list too like:
planets = [
Planet(1,"Earth","home planet","blue, brown, green, and white"),
Planet(2, "Mercury", "closest planet to the sun", "grey"),
Planet(3, "Neptune", "eight planet, farthest from sun", "blue"),
Planet(4, "Jupiter", "fifth planet from the sun, largest planet", "brown, orange and tan")
]| planets_bp = Blueprint("planets", __name__, url_prefix = "/planets") | ||
|
|
||
| @planets_bp.route("", methods = ["GET"]) | ||
| def get_all_planets(): |
There was a problem hiding this comment.
Nice job naming the method to describe what this route does.
In the future, if you end up splitting 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.
| for planet in planets: | ||
| planet_response.append(planet.to_json()) | ||
|
|
||
| return jsonify(planet_response, 200) |
There was a problem hiding this comment.
Nice work explicitly returning a 200 status code
| def validate_id(planet_id): | ||
| try: | ||
| planet_id = int(planet_id) | ||
| except: | ||
| return abort(make_response({"message": f"planet {planet_id} is not valid"}, 400)) | ||
|
|
||
| for planet in planets: | ||
| if planet.id == planet_id: | ||
| return planet | ||
|
|
||
| return abort(make_response({"message": f"planet {planet_id} does not exist"}, 404)) |
| @planets_bp.route("/<planet_id>", methods = ["GET"]) | ||
| def get_one_planet(planet_id): | ||
| planet = validate_id(planet_id) | ||
| return jsonify(planet.to_json(), 200) |
| def validate_id(planet_id): | ||
| try: | ||
| planet_id = int(planet_id) | ||
| except: | ||
| return abort(make_response({"message": f"Planet {planet_id} is not valid"}, 400)) | ||
|
|
||
| planet = Planet.query.get(planet_id) | ||
| if not planet: | ||
| return abort(make_response({"message": f"Planet {planet_id} does not exist"}, 404)) | ||
|
|
||
| return planet No newline at end of file |
There was a problem hiding this comment.
Nice work pulling this into a helper to keep your routes short and sweet. In the future, you might need to make 'helper.py' more specific. Think about Task List, you have Goal and Task, you may need helpers for each. If you refactor helper methods into a helper file then you could have goal_helper.py and task_helper.py
| def update_planet(self,request_body): | ||
| self.name = request_body["name"] | ||
| self.description =request_body["description"] | ||
| self.color= request_body["color"] | ||
|
|
||
| @classmethod | ||
| def create_planet(cls, request_body): | ||
| new_planet = cls(name = request_body["name"], | ||
| description = request_body["description"], | ||
| color = request_body["color"]) | ||
| return new_planet No newline at end of file |
There was a problem hiding this comment.
Nice refactor here to keep your routes concise.
This comment applies to your update_planet() and create_planet() methods:
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 color, then line 24 would throw KeyError that it couldn't find color.
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 color. Something to think about for Task List.
| @@ -0,0 +1,49 @@ | |||
| def test_get_all_planets_no_records(client): | |||
3 participants
No description provided.