Conversation
spitsfire
left a comment
spitsfire
left a comment
There was a problem hiding this comment.
Part 1! Nicely done! Left a few suggestions here and there, but your logic looks good
| planet_response.append({ | ||
| "id" : planet.planet_id, | ||
| "name" : planet.name, | ||
| "description" : planet.description, | ||
| "color" : planet.color | ||
| }) |
There was a problem hiding this comment.
this would make a good instance method to define in the Planet class
| if not planet_response: | ||
| return abort(make_response({"message": f"Planet {name_query} is not found"}, 404)) | ||
|
|
||
| return jsonify(planet_response) |
There was a problem hiding this comment.
don't forget your status code!
| return jsonify(planet_response) | |
| return jsonify(planet_response), 200 |
|
|
||
| #POST new planet | ||
| @planet_bp.route("", methods=["POST"]) | ||
| def add_new_planet(): |
| return { | ||
| "id" : planet.planet_id, | ||
| "name" : planet.name, | ||
| "description" : planet.description, | ||
| "color" : planet.color | ||
| } |
There was a problem hiding this comment.
another good reason to make this an instance method! we won't need to duplicate this code again
app/routes.py
Outdated
|
|
||
| db.session.commit() | ||
|
|
||
| return make_response(f"Planet #{planet.planet_id}, '{planet.name}', successfully updated.") |
app/routes.py
Outdated
| db.session.delete(planet) | ||
| db.session.commit() | ||
|
|
||
| return make_response(f"Planet #{planet.planet_id}, '{planet.name}', successfully deleted.") No newline at end of file |
| @@ -1,2 +1,93 @@ | |||
| from flask import Blueprint | |||
| import re | |||
There was a problem hiding this comment.
are you using re for anything? I didn't see any regular expressions
| import re | ||
| from flask import Blueprint, jsonify, abort, make_response, request | ||
| from app import db | ||
| from app.models.Planet import Planet |
There was a problem hiding this comment.
something to consider: the file itself should be lowercase as in planet.py whereas the class itself is uppercase
spitsfire
left a comment
spitsfire
left a comment
There was a problem hiding this comment.
Part 2! Nicely done Chinazo and Bahareh! One thing to consider is pulling out some functionality into its own files or put it inside the Planet class.
There was one small logic error, where I think you pulled code from Hello Books and forgot to change out the attribute name.
Other than that, it looked just fine!
| planet.name = request_body["name"] | ||
| planet.description = request_body["description"] | ||
| planet.color = request_body["color"] |
There was a problem hiding this comment.
This could also make a good instance method inside the Planet class
| def validate_planet(planet_id): | ||
| try: | ||
| planet_id = int(planet_id) | ||
| except: | ||
| return abort(make_response({"message": f"Planet {planet_id} is invalid"}, 400)) | ||
|
|
||
| planet = Planet.query.get(planet_id) | ||
|
|
||
| if not planet: | ||
| return abort(make_response({"message": f"Planet {planet_id} is not found"}, 404)) | ||
| return planet |
There was a problem hiding this comment.
We could move this out of the routes file into its own helper function file, so that the routes are cleaner
|
|
||
| name_query = request.args.get("name") | ||
| if name_query: | ||
| planets = Planet.query.filter_by(name=name_query.title()) |
2 participants
No description provided.