Conversation
…functions to access data
audreyandoy
left a comment
audreyandoy
left a comment
There was a problem hiding this comment.
Great work Rebecca and Sarah! My feedback primarily focuses on refactoring :)
| app.config['SQLALCHEMY_ECHO'] = True | ||
|
|
||
| # import models here | ||
| from app.models.planet import Planet |
There was a problem hiding this comment.
The blueprints already import the models so we don't actually need the model import in this file anymore.
| id = db.Column(db.Integer, primary_key=True, autoincrement=True) | ||
| name = db.Column(db.String) | ||
| description = db.Column(db.String) | ||
| distance = db.Column(db.String) |
There was a problem hiding this comment.
Careful! Using the class as it currently is, we'd be able to make a planet with a null name, description, and color. I do see in the POST request that y'all checked for falsy values for name, description, and color, however, null values can still be inserted manually using SQL. When making classes in the future, consider which columns are required (non-nullable).
| description = db.Column(db.String) | ||
| distance = db.Column(db.String) | ||
|
|
||
| def planet_dict(self): |
| if request.method == "POST": | ||
| request_body = request.get_json() | ||
| if ("name" or "description" or "distance") not in request_body: | ||
| return jsonify("Invalid Request"), 404 | ||
|
|
||
| new_planet = Planet( | ||
| name=request_body["name"], | ||
| description=request_body["description"], | ||
| distance=request_body["distance"] | ||
| ) | ||
| db.session.add(new_planet) | ||
| db.session.commit() | ||
| return jsonify(new_planet.planet_dict()), 201 |
| elif request.method == "GET": | ||
| planet_name_query = request.args.get("name") | ||
| if planet_name_query: | ||
| planets = Planet.query.filter_by(name=planet_name_query) | ||
| else: | ||
| planets = Planet.query.all() | ||
|
|
||
| planets_response = [ | ||
| planet.planet_dict() for planet in planets | ||
| ] | ||
| return jsonify(planets_response) |
There was a problem hiding this comment.
👍 great use of a nested dictionary!
| return jsonify(f"{planet.name} was successfully updated"), 200 | ||
|
|
||
| elif request.method == "PATCH": | ||
| response_body = request.get_json() |
There was a problem hiding this comment.
Adding a check for an empty response_body can also be handy here
| db.session.commit() | ||
| return jsonify(f"{planet.name} was successfully updated"), 200 | ||
|
|
||
| elif request.method == "DELETE": |
| name="Earth", description="Blue green marble", distance="Right here") | ||
| planet2 = Planet(name="Mars", description="red", distance="next door") | ||
| db.session.add_all([planet1, planet2]) | ||
| db.session.commit() |
There was a problem hiding this comment.
Consider returning [planet1, planet2] after this line, so that tests can make use of the attributes of the records (like the id or name of planet)
| response = client.post("/planets", data=json.dumps(data), | ||
| headers={"Content-Type": "application/json"}) | ||
|
|
||
| assert response.status_code == 201 |
There was a problem hiding this comment.
We could tweak the POST requests in routes.py so that we can return a dictionary of the actual record that was added (just to ensure that all the attributes were valid and added).
We could perform a Planet.query.get call to retrieve the record from the database to confirm that it was actually added.
| def test_get_planet_returns_404(client): | ||
| response = client.get("/planets/1") | ||
|
|
||
| assert response.status_code == 404 No newline at end of file |
3 participants
No description provided.