Skip to content

Conversation

mariia-iureva
Copy link

Waave 1 and 2

Copy link

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work Masha & Melly,

I left some minor comments, but this is quite well done. Let me know if you have questions via slack.

Comment on lines +10 to +29
def to_dict(self):

return {
"name": self.name,
"color": self.color,
"description": self.description,
"id": self.id
}
@classmethod
def from_json(cls, req_body):
return cls(
name= req_body["name"],
color= req_body["color"],
description= req_body["description"]
)

def update(self, req_body):
self.name= req_body["name"],
self.color= req_body["color"],
self.description= req_body["description"] No newline at end of file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great helper methods

Comment on lines +8 to +19
def validate_planet(class_obj, planet_id):
try:
planet_id = int(planet_id)
except:
abort(make_response({"message": f"planet {planet_id} has an invalid planet_id"}, 400))

query_result = class_obj.query.get(planet_id)

if not query_result:
abort(make_response({"message": f"planet {planet_id} not found"}, 404))

return query_result

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good helper function


@planets_bp.route("", methods=["POST"])
def create_planet():
request_body = request.get_json()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some validation on the request_body here would be good to ensure it has the required fields.

@planets_bp.route("/<planet_id>", methods=["PUT"])
def update_planet(planet_id):
planet = validate_planet(planet_id)
request_body = request.get_json()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again validation here would be appropriate.

assert response.status_code == 400
assert response_body == "{\"message\":\"planet ABC has an invalid planet_id\"}\n"

def test_create_planet_can_create_planet_in_empty_db(client):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a create request with an invalid request body would be a good test as well.

return make_response (f"Planet #{planet.id} successfully updated")

@planets_bp.route("/<planet_id>", methods=["DELETE"])
def delete_planet(planet_id):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting that this and the update action are untested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants