Zoisite - Raissa Barbossa, Sabs Ford, Liz Trejo#23
Zoisite - Raissa Barbossa, Sabs Ford, Liz Trejo#23lizforloops wants to merge 15 commits intoAda-C19:mainfrom
Conversation
apradoada
left a comment
apradoada
left a comment
There was a problem hiding this comment.
This looks great, y'all! Well done!
| from .routes import planet_bp | ||
| app.register_blueprint(planet_bp) | ||
|
|
||
| return app No newline at end of file |
| Planet(6, "Saturn", "ringed planet", 72367), | ||
| Planet(7, "Neptune", "blue planet", 30599), | ||
| Planet(8, "Uranus", "furthest planet", 31518) | ||
| ] |
There was a problem hiding this comment.
Well done setting up the planet class and your planets list!
| @planet_bp.route("", methods=["GET"]) | ||
| def read_planets(): | ||
| planet_list = [planet.make_dict() for planet in planets] | ||
| return jsonify(planet_list) |
There was a problem hiding this comment.
Great use of a list comprehension here!
refactoring, adding error checking, adding additional tests
apradoada
left a comment
apradoada
left a comment
There was a problem hiding this comment.
Well done, y'all! This looks really great!
|
|
||
| else: | ||
| app.config["TESTING"] = True | ||
| app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False |
There was a problem hiding this comment.
Notice that lines 16 and 22 are the same. This can be pulled out of the conditional to avoid duplicate code!
| id = db.Column(db.Integer, primary_key=True, autoincrement=True) | ||
| name = db.Column(db.String) | ||
| description = db.Column(db.String) | ||
| diameter= db.Column(db.Float) |
There was a problem hiding this comment.
When designing your class, think about whether or not each attribute should be required. I can see a case for name being a required attribute!
| name=planet_data['name'], | ||
| description=planet_data['description'], | ||
| diameter=planet_data['diameter'] | ||
| ) No newline at end of file |
| message = f"{cls.__name__} {model_id} not found" | ||
| abort(make_response({"message": message}, 404)) | ||
|
|
||
| return model |
There was a problem hiding this comment.
Great job modifying the validate method to be non-class specific!
| "message": "Invalid request body", | ||
| "error": str(e) | ||
| } | ||
| abort(make_response(jsonify(error_dict), 400)) |
There was a problem hiding this comment.
I love the error handling y'all did here! Great idea making sure the appropriate keys are included in the request. As mentioned before, if you are going to include this error handling, it's a good idea to be consistent and make the class' attributes non-nullable!
| "message": "Invalid request body", | ||
| "error": str(e) | ||
| } | ||
| abort(make_response(jsonify(error_dict), 400)) |
There was a problem hiding this comment.
Well done with the error handling here as well!
| db.session.commit() | ||
|
|
||
| message = f"Planet #{planet.id} {planet.name} successfully deleted" | ||
| return make_response(jsonify(message)), 200 No newline at end of file |
| diameter="86881") | ||
|
|
||
| db.session.add_all([mercury_planet, jupter_planet]) | ||
| db.session.commit() |
There was a problem hiding this comment.
Great job creating a custom fixture that will update the database!
| def test_validate_model_invalid_id(two_saved_planets): | ||
| # Act & Assert | ||
| with pytest.raises(HTTPException): | ||
| result_planet = validate_model(Planet, "dog") No newline at end of file |
There was a problem hiding this comment.
This is how you do testing. This is so robust and well done! Great job y'all!
3 participants
No description provided.