Sharks - Jeannie Z. & Juli T. by jterapin · Pull Request #15 · ada-c17/solar-system-api

jterapin · 2022-04-26T20:18:29Z

Completed Wave 1 & 2.

audreyandoy

Part 1 looks great! 👍

audreyandoy · 2022-04-28T06:59:52Z

app/__init__.py

+    from .routes import planet_bp
+    app.register_blueprint(planet_bp)


👍 Looks good! We can also move this file into a routes folder which would change the path of this import.

audreyandoy · 2022-04-28T07:00:11Z

app/routes.py

+class Planet:
+    def __init__(self, id, name, description, distance_from_earth):
+        self.id = id
+        self.name = name
+        self.description = description
+        self.distance_from_earth = distance_from_earth
+
+    def to_json(self):
+        return {
+                "id": self.id,
+                "name": self.name,
+                "description": self.description,
+                "Distance from Earth": self.distance_from_earth
+            }


👍 Good work!

audreyandoy · 2022-04-28T07:00:56Z

app/routes.py

+planets = [
+    Planet(1, "Mars", "Next livable planet", "131.48 million mi"),
+    Planet(2, "Mercury", "Smallest planet", "94.025 million mi"),
+    Planet(3, "Earth", "We live here, slowly dying", "0.0 million mi")


Lol the description... 🤣 😭

audreyandoy · 2022-04-28T07:02:42Z

app/routes.py

+def get_planets():
+    planets_response = []
+    for planet in planets:
+        planets_response.append(
+            {
+                "id": planet.id,
+                "name": planet.name,
+                "description": planet.description,
+                "Distance from Earth": planet.distance_from_earth
+            })
+    return jsonify(planets_response)


This is a good place to use the to_json() helper method from the Planet class:

def get_planets(): planets_response = [] for planet in planets: planets_response.append(planet.to_json()) return jsonify(planets_response)

We can also use list comprehension to build planets_response.

audreyandoy · 2022-04-28T07:04:57Z

app/routes.py

+def validate_planet(planet_id):
+    try:
+        planet_id = int(planet_id)
+    except:
+        return abort(make_response({"message": f"planet {planet_id} is invaild"}, 400))
+
+    for planet in planets:
+        if planet.id == planet_id:
+            return planet
+    return abort(make_response({"message": f"planet {planet_id} does not exist"}, 404))


The validation for detecting bad ids and ids with no record looks good!

audreyandoy · 2022-04-28T07:05:12Z

app/routes.py

+@planet_bp.route("/<planet_id>", methods=["GET"])
+def get_one_planet(planet_id):
+    planet = validate_planet(planet_id)
+    return jsonify(planet.to_json(), 200)


👍 nice use of the helper method!

audreyandoy

Jeannie and Julie! Great work in finishing solar system. I have added comments on small ways to improve the API.

🦈 ✨

audreyandoy · 2022-05-12T06:20:40Z

app/__init__.py

+from flask_sqlalchemy import SQLAlchemy
+from flask_migrate import Migrate
+from dotenv import load_dotenv
+import os
+
+
+db = SQLAlchemy()
+migrate = Migrate()
+load_dotenv()


 def create_app(test_config=None):
    app = Flask(__name__)

+
+    app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
+    if not test_config:
+        app.config['SQLALCHEMY_DATABASE_URI'] = os.environ.get(
+            "SQLALCHEMY_DATABASE_URI")
+    else:
+        app.config["TESTING"] = True
+        app.config["SQLALCHEMY_DATABASE_URI"] = os.environ.get(
+            "SQLALCHEMY_TEST_DATABASE_URI")
+
+    db.init_app(app)
+    migrate.init_app(app, db)
+    from app.models.planet import Planet
+
+    from .routes.planet_routes import planet_bp
+    app.register_blueprint(planet_bp)


👍 Good work in setting up the Flask app!

audreyandoy · 2022-05-12T06:21:28Z

app/models/planet.py

+    name = db.Column(db.String)
+    description = db.Column(db.String)
+    color = db.Column(db.String)


Should we be able to make planets with no name? To prevent our API from creating a planet with a null name, we can set that column to nullable = False.

name = db.Column(db.String, nullable=False)

audreyandoy · 2022-05-12T06:21:51Z

app/models/planet.py

+    def to_json(self):
+        return {
+            "id": self.id,
+            "name": self.name,
+            "description": self.description,
+            "color": self.color
+        }
+
+    def update(self, request_body):
+        self.name = request_body["name"]
+        self.description = request_body["description"]
+        self.color = request_body["color"]
+
+    @classmethod
+    def create(cls, request_body):
+        new_planet = cls(
+            name=request_body['name'],
+            description=request_body['description'],
+            color=request_body['color']
+        )
+        return new_planet


Nice use of helper methods and a class method!

audreyandoy · 2022-05-12T06:23:40Z

app/routes/helpers.py

+def validate_planet(planet_id):
+    try:
+        planet_id = int(planet_id)
+    except:
+        return abort(make_response(jsonify(f"Planet {planet_id} is invalid"), 400))
+
+    planet = Planet.query.get(planet_id)
+
+    if not planet:
+        return abort(make_response(jsonify(f"Planet {planet_id} does not exist"), 404))
+    return planet


Helpers look great! 👍 We may want to consider making the validation code as generic as possible to utilize any class and id or renaming this file to helper_planet_routes or planet_routes_helper. Either would be helpful if we expand this project to validate other objects like moons, suns, etc.

A more generic validation helper method would look like:

from flask import make_response, abort def validate_object(cls, id): try: id = int(id) except: return abort(make_response({"message": f"{cls} {id} is invalid"}, 400)) obj = cls.query.get(id) if not obj: abort(make_response({"message": f"{cls} {id} not found"},404)) return obj

audreyandoy · 2022-05-12T06:24:47Z

app/routes/planet_routes.py

+@planet_bp.route("", methods=["POST"])
+def create_planet():
+    request_body = request.get_json()
+
+    new_planet = Planet.create(request_body)
+
+    db.session.add(new_planet)
+    db.session.commit()
+
+    return make_response(jsonify(f"Planet {new_planet.name} has been created"), 201)


jsonify() is sufficient enough to generate a Response object

@planet_bp.route("", methods=["POST"]) def create_planet(): request_body = request.get_json() new_planet = Planet.create(request_body) db.session.add(new_planet) db.session.commit() return jsonify(f"Planet {new_planet.name} has been created"), 201

audreyandoy · 2022-05-12T06:26:17Z

app/routes/planet_routes.py

+@ planet_bp.route("/<planet_id>", methods=["GET"])
+def get_one_planet(planet_id):
+    planet = validate_planet(planet_id)
+    return make_response(jsonify(planet.to_json()), 200)


👍 Nice work!

Also, if a view function returns a dictionary (planet.to_json() in this case), Flask will automatically convert the dictionary into a JSON response object (aka Flask will turn dictionaries into JSON data for us). This means you can leave out make_response() like so:

@planets_bp.route("/<planet_id>", methods=["GET"]) def get_one_planet(planet_id): planet = validate_planet(planet_id) return planet.to_json(), 200

Here's the documentation with more info:
https://flask.palletsprojects.com/en/2.0.x/quickstart/#apis-with-json

audreyandoy · 2022-05-12T06:27:55Z

app/routes/planet_routes.py

+@ planet_bp.route("/<planet_id>", methods=["PUT"])
+def update_planet(planet_id):
+    planet = validate_planet(planet_id)
+    request_body = request.get_json()
+
+    try:
+        Planet.update(request_body)
+        db.session.commit()
+    except KeyError:
+        return abort(make_response(jsonify("Missing information")), 400)
+
+    return make_response(jsonify(f"Planet #{planet.id} successfully updated"), 200)


Nice work! We may want to consider validating the request_body data in a separate helper method.

audreyandoy · 2022-05-12T06:28:17Z

app/routes/planet_routes.py

+@ planet_bp.route("/<planet_id>", methods=["DELETE"])
+def delete_one_planet(planet_id):
+    planet = validate_planet(planet_id)
+
+    db.session.delete(planet)
+    db.session.commit()
+
+    return make_response(jsonify(f"Planet {planet.id} has been deleted"), 200)


👍 Comment about not needing make_response also applies.

audreyandoy · 2022-05-12T06:28:31Z

app/tests/conftest.py

+def two_saved_planets(app):
+    # Arrange
+    planet_one = Planet(name="Mars",
+                        description="Close enough",
+                        color="Red")
+    planet_two = Planet(name="Earth",
+                        description="we out here",
+                        color = "Green")
+
+    db.session.add_all([planet_one, planet_two])
+    db.session.commit()


audreyandoy · 2022-05-12T06:29:09Z

app/tests/test_routes.py

@@ -0,0 +1,73 @@
+#`GET` `/planets` returns `200` and an empty array.


Test routes look great 👍 !

jterapin and others added 7 commits April 22, 2022 14:16

added planet class and endpoints

85c8372

changes to __init__ _

ba93e91

updated __init__.py

2576c0c

Merge branch 'main' of https://github.com/jterapin/solar-system-api

78c4603

Finished wave 2

9fccce8

Updated error messages.

dd61ed8

Minor updates.

8a4fcb6

audreyandoy reviewed Apr 28, 2022

View reviewed changes

jeanniezheng and others added 15 commits April 29, 2022 13:04

created models for planet

880cfea

Merge branch 'main' of https://github.com/jterapin/solar-system-api

456e4a0

post one planet

ce18f36

read all planets

604991f

Revised String Attributes to match.

feaf92e

Added validate planet and get One Planet routes

ae0ab71

added update endpoint route

cd4ee9b

Added Delete route

5513730

Test

cca4d02

Refactored per Audrey's suggestions.

3242fb4

testing pull

7aadde2

testing commit

549f779

Another test.

ef7d221

refactored and added q parm

c2be4e0

Added tests

fdde89e

audreyandoy reviewed May 12, 2022

View reviewed changes

		from .routes import planet_bp
		app.register_blueprint(planet_bp)

		@@ -0,0 +1,73 @@
		#`GET` `/planets` returns `200` and an empty array.

Conversation

jterapin commented Apr 26, 2022

Uh oh!

audreyandoy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

audreyandoy Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

audreyandoy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

audreyandoy Apr 28, 2022 •

edited

Loading