Skip to content

Conversation

Paje-Bishop
Copy link

No description provided.

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.

Nicely done Paje & Lain. I left some comments/suggestions, if you have questions ping me on Slack.

Comment on lines +10 to +16
def planet_dict(self):
return {
"id": self.id,
"name": self.name,
"description": self.description,
"mass": self.mass,
}

Choose a reason for hiding this comment

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

Good helper function

planets_result = Planet.query.all()
return jsonify([p.planet_dict() for p in planets_result]), 200
elif request.method == "POST":
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 to make sure the request_body is valid (has required fields) would be good.

assert response.status_code == 400


def test_delete_planet(client, two_saved_planets):

Choose a reason for hiding this comment

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

You should also test what happens when you delete a planet that doesn't exist.

assert response_body == {"success": "Planet #1 successfully deleted"}


def test_update_planet(client, two_saved_planets):

Choose a reason for hiding this comment

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

You should also test invalid updates like when the id doesn't exist or there is an invalid request body.

assert response_body == {"success": "Planet #1 successfully updated"}


def test_name_query(client, two_saved_planets):

Choose a reason for hiding this comment

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

Love this test ❤️

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