Skip to content

Sea Turtles Heather M & Jenny C#19

Open
cathos wants to merge 22 commits intoada-c17:mainfrom
cathos:main
Open

Sea Turtles Heather M & Jenny C#19
cathos wants to merge 22 commits intoada-c17:mainfrom
cathos:main

Conversation

@cathos
Copy link

@cathos cathos commented Apr 26, 2022

No description provided.

app/routes.py Outdated
description = planet.description,
gravity = planet.gravity
))
return jsonify(planets_result)
Copy link

Choose a reason for hiding this comment

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

Here its nice to add a 200 status code. Even though it happens by default it adds readability to your code.

return jsonify(planets_result), 200

Choose a reason for hiding this comment

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

That is a good idea. We'll add this.Thank you Trenisha

app/routes.py Outdated
from flask import Blueprint, jsonify, make_response, abort


class Planet:
Copy link

Choose a reason for hiding this comment

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

this looks good!

app/routes.py Outdated
Comment on lines 23 to 28
planets_result.append(dict(
id = planet.id,
name = planet.name,
description = planet.description,
gravity = planet.gravity
))
Copy link

Choose a reason for hiding this comment

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

this could be moved to your planets class as an instance method. Then you would call that instance method here.

 def make_dict(self):
        return dict(
            id=self.id,
            name=self.name,
            description=self.description,
            gravity=self.gravity,  
        )

Choose a reason for hiding this comment

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

Yup. We should refactor it like that. Thank you for the suggestion.

app/routes.py Outdated
planets_bp = Blueprint("planets", __name__, url_prefix="/planets")

@planets_bp.route("", methods=["GET"])
def handle_planets():
Copy link

Choose a reason for hiding this comment

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

Conventionally you would see the naming of the function be similar to the CRUD operation you are performing. Like get_planets or get_all_planets


return app
else:
print("Testing is on")
Copy link

Choose a reason for hiding this comment

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

Was this for testing your code? Or was this intentional so that you know if you are testing?

from app import db


class Planet(db.Model):
Copy link

Choose a reason for hiding this comment

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

🙌🏽

Comment on lines +8 to +11
@planets_bp.route("/teapot", methods=["GET", "POST"])
def handle_teapot():
return make_response("I'm a teapot!", 418)

Copy link

Choose a reason for hiding this comment

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

Was this also yall testing some stuff out?

Comment on lines +28 to +34
return {
"id": planet.id,
"order_from_sun": planet.order_from_sun,
"name": planet.name,
"description": planet.description,
"gravity": planet.gravity
}
Copy link

Choose a reason for hiding this comment

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

in your Planet class you can create an instance method to return this information. Then you can use the instance method over and over again.

Comment on lines +38 to +40
name_query = request.args.get("name")
description_query = request.args.get("description")
order_from_sun_query = request.args.get("order_from_sun")
Copy link

Choose a reason for hiding this comment

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

I like how you all added different ways to query for the planets

Comment on lines +52 to +58
planets_response.append({
"id": planet.id,
"order_from_sun": planet.order_from_sun,
"name": planet.name,
"description": planet.description,
"gravity": planet.gravity
})
Copy link

Choose a reason for hiding this comment

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

this could also be made an instance method in your Planet class.



@pytest.fixture
def app():
Copy link

Choose a reason for hiding this comment

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

🙌🏽

@@ -0,0 +1,71 @@
# from hello-books

Copy link

Choose a reason for hiding this comment

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

tests look good!

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