Sphinx - Aleida Vieyra & Vlada Rapaport#17
Conversation
…o errors with importing
vladarap88
changed the title
…updating project for Wave 5
…d .env file, ran pytest
mikellewade
left a comment
mikellewade
left a comment
There was a problem hiding this comment.
Great work, everyone! Very clean code!
| name=self.name, | ||
| description=self.description, | ||
| distance_from_sun=self.distance_from_sun, | ||
| ) |
There was a problem hiding this comment.
We could also have a from_dict class method that we could use to construct our plant instances.
| from app.models.planet import Planet | ||
|
|
||
|
|
||
| planets_bp = Blueprint("planets_bp", __name__, url_prefix="/planets") |
There was a problem hiding this comment.
Convention is to name this bp, inside of app/__init__.py we could then import this under an alias to prevent name conflicts like so:
from app.routes.planet_routes.py import bp as planets_bp| request_body = request.get_json() | ||
| name = request_body["name"] | ||
| description = request_body["description"] | ||
| distance_from_sun = request_body["distance_from_sun"] | ||
|
|
||
| new_planet = Planet( | ||
| name=name, description=description, distance_from_sun=distance_from_sun | ||
| ) | ||
| db.session.add(new_planet) | ||
| db.session.commit() | ||
|
|
||
| response = new_planet.to_dict() | ||
|
|
||
| return response, 201 |
There was a problem hiding this comment.
We could move all of this logic into a function like create_model that we created in class.
| if name_param: | ||
| query = query.where(Planet.name == name_param) | ||
|
|
||
| description_param = request.args.get("description") | ||
| if description_param: | ||
| query = query.where(Planet.description.ilike(f"%{description_param}%")) | ||
|
|
||
| distance_from_sun_param = request.args.get("distance_from_sun") | ||
| if distance_from_sun_param: | ||
| query = query.where(Planet.distance_from_sun.ilike(f"%{distance_from_sun_param}%")) | ||
|
|
||
| query = query.order_by(Planet.id) | ||
|
|
||
| planets = db.session.scalars(query) | ||
|
|
||
| planets_response = [planet.to_dict() for planet in planets] | ||
| return planets_response |
There was a problem hiding this comment.
Like create_model we could also move this logic into a helper function like get_models_with_filters. How could the helper function look different to serve your needs here?
| def get_one_planet(planet_id): | ||
| planet = validate_planet(planet_id) | ||
|
|
||
| response = planet.to_dict() |
There was a problem hiding this comment.
I think I would be okay with returning this on a single line since we aren't doing anything extra here.
| request_body = request.get_json() | ||
|
|
||
| planet.name = request_body["name"] | ||
| planet.description = request_body["description"] | ||
| planet.distance_from_sun = request_body["distance_from_sun"] | ||
| db.session.commit() |
There was a problem hiding this comment.
Could we move this into a helper function? How would it look based off what we learned during refactoring?
| def validate_planet(planet_id): | ||
| try: | ||
| planet_id = int(planet_id) | ||
| except: | ||
| response = {"message": f"planet {planet_id} invalid"} | ||
| abort(make_response(response, 400)) | ||
|
|
||
| query = db.select(Planet).where(Planet.id == planet_id) | ||
| planet = db.session.scalar(query) | ||
|
|
||
| if not planet: | ||
| response = {"message": f"planet {planet_id} not found"} | ||
| abort(make_response(response, 404)) | ||
|
|
||
| return planet |
There was a problem hiding this comment.
Where could this function live to better organize our code?
|
|
||
| db.session.add_all([planet_1, planet_2]) | ||
|
|
||
| db.session.commit() No newline at end of file |
There was a problem hiding this comment.
Nice work on these fixtures here! 👍🏿
| assert response.status_code == 404 | ||
| assert response_body == {"message": f"planet 1 not found"} |
| assert response.status_code == 201 | ||
| assert response_body == { | ||
| "id": 1, | ||
| "name": "Pluto", | ||
| "description": "Previously our 9th planet, is now classified as a dwarf planet.", | ||
| "distance_from_sun": 3700, | ||
| } |
There was a problem hiding this comment.
How could we test to see if the created planet is actually in the database?
3 participants
No description provided.