SHARKS - Jande R. and Lindsey S. by janderoyalty · Pull Request #7 · ada-c17/solar-system-api

janderoyalty · 2022-04-26T18:05:56Z

No description provided.

audreyandoy

Part 1 looks great 👍

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

app/__init__.py

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


👍 Looks good! We can also move this file into a routes folder which would change the path of this import. Creating a routes folder makes sense if we have different routes for specific objects for example planet_routes, moon_routes, galaxy_routes, etc.

audreyandoy · 2022-04-28T07:16:25Z

app/routes.py

+class Planet():
+	def __init__(self, id, name, description, circumference, length_of_year):
+		self.id = id
+		self.name = name
+		self.description = description
+		self.circumference = circumference
+		self.length_of_year = length_of_year
+
+	def to_json(self): 
+		return {
+			"id" : self.id,
+			"name" : self.name,
+			"description" : self.description,
+			"circumference" : self.circumference,
+			"length_of_year" : self.length_of_year
+		}


👍 LGTM (looks good to me)

audreyandoy · 2022-04-28T07:16:53Z

app/routes.py

+	Planet(6, "Saturn", "sun's bae with all 7 rings", 235185, 10620),
+	Planet(7, "Uranus", "can only be seen with a telescope", 99739, 30240),
+	Planet(8, "Neptune", "it is an intense blue color", 96645, 59400),
+	Planet(9, "Pluto", "no dwarf in my book", 7144, 88920)


Yes! Pluto is a planet 💯

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

app/routes.py

+def get_all_planets():
+	planets_response = []
+	for planet in planets:
+		planets_response.append({
+			"id": planet.id,
+			"name": planet.name,
+			"description": planet.description,
+			"circumference": planet.circumference,
+			"length of year": planet.length_of_year
+		})
+	return jsonify(planets_response)


This is where we can use our handy dandy to_json() helper method in the Planet class. And we should also return a 200 status code.

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

We can also use list comprehension to build planets_response.

audreyandoy · 2022-04-28T07:25:34Z

app/routes.py

+def get_one_planet(id):
+	try:
+		id = int(id)
+	except:
+		return abort(make_response({"message":f"Planet {id} is invalid."}, 400))
+
+	for planet in planets:
+		if planet.id == id:
+			return jsonify(planet.to_json())
+	return abort(make_response({"message":f"Planet {id} not found."}, 404))


Nice use of a try/except. The validation for bad id's and for id's with no record looks good. We can also consider moving the validation portion of this function into a helper method.

audreyandoy · 2022-04-28T07:34:40Z

app/routes.py

+
+	for planet in planets:
+		if planet.id == id:
+			return jsonify(planet.to_json())


Don't forget the status code!

audreyandoy

Jande and Lindsey! Great work in finishing solar system. I have provided comments on small ways y'all can improve the API.

🦈 ✨

audreyandoy · 2022-05-12T06:30:45Z

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 import planets_bp
+    app.register_blueprint(planets_bp)
+


👍 Good work setting up the Flask app!

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

app/models/helpers.py

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


Helpers look great! 👍 We may want to consider making the validation code as generic as possible to process 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 contain more routes for 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:33:25Z

app/models/planet.py

+class Planet(db.Model):
+    id = db.Column(db.Integer, primary_key=True, autoincrement=True)
+    name = db.Column(db.String)
+    description = db.Column(db.String)
+    circumference = db.Column(db.Integer)
+    length_of_year= db.Column(db.Integer)


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.

class Planet(db.Model): id = db.Column(db.Integer, primary_key=True, autoincrement=True) name = db.Column(db.String, nullable=False) description = db.Column(db.String) circumference = db.Column(db.Integer) length_of_year= db.Column(db.Integer)

audreyandoy · 2022-05-12T06:33:48Z

app/models/planet.py

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


👍 Nice helper methods and use of a class method!

audreyandoy · 2022-05-12T06:34:34Z

app/routes.py

+@planets_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 successfully created"), 201 #use make response when you want to return something that is not json


Looks great! We could also consider validating the request_body in a separate helper function.

To add onto your comment about make_response, we can also use it when we want to add additional headers to a response. Here are some examples in the documentation:
https://flask.palletsprojects.com/en/2.0.x/api/#flask.make_response

audreyandoy · 2022-05-12T06:49:48Z

app/routes.py

+
+	# {
+	# 	name:
+	# 	description:
+	# 	circumference:
+	# 	length_of_year:
+    # }


We can get rid of unneeded comments when submitting code for review.

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

app/routes.py

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


Nice helper! 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 read_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:52:27Z

app/routes.py

+@planets_bp.route("/<id>", methods=["DELETE"])
+def delete_one_planet(id):
+    planet = validate_planet(id)
+
+    db.session.delete(planet)
+    db.session.commit()
+
+    return make_response(f"Planet #{id} was successfully deleted"), 200


When we pass a string to make_response, Flask will assume that we want to return HTML rather than JSON data. make_response is useful when we want to attach headers to a response but in this case, we're simply returning JSON, so jsonify() is sufficient enough.

return jsonify(f"Planet {id} successfully deleted"), 200

audreyandoy · 2022-05-12T06:53:10Z

app/tests/conftest.py

+@pytest.fixture
+def two_saved_planets(app):
+# Arrange
+    mercury = Planet(name="Mercury",
+    description="made mostly of rocks", circumference=9522, length_of_year = 88)
+    venus = Planet(name="Venus",
+    description="most like Earth", circumference=23617, length_of_year=225)
+
+    db.session.add_all([mercury, venus])
+    # Alternatively, we could do# db.session.add(mercury)# db.session.add(venus)
+    db.session.commit()


👍 Nice work on setting up the fixtures!

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

app/tests/test_routes.py

@@ -0,0 +1,70 @@
+


👍 Nice work on the tests!

still-lindsey and others added 2 commits April 23, 2022 03:36

completed wave 1

57f27d3

Part 1 finished

0efeb9a

janderoyalty changed the title ~~SHARKS = Jande R. and Lindsey S.~~ SHARKS - Jande R. and Lindsey S. Apr 26, 2022

audreyandoy reviewed Apr 28, 2022

View reviewed changes

app/routes.py Outdated

for planet in planets:

if planet.id == id:

return jsonify(planet.to_json())

Copy link

audreyandoy Apr 28, 2022

Choose a reason for hiding this comment

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

Don't forget the status code!

janderoyalty and others added 17 commits April 29, 2022 13:03

Cong. db

769c4cf

Connect database and Flask. Defined Model

6595fd2

Database Migration Setup

93954be

Wave 3 finished

ff2ec6e

Moved to_json

a8fae40

add to json method to cat class

cc4aec6

merge conflict resolved

443e0ca

added queries

2f0d8eb

refactored validate planet in helpers file

32bbf87

refactored to included update method in planet class

92d9344

refactored create factory method

645b5e6

refactored/created class ethod to clean up POST route

048f1fc

modify create_app to handle test mode

d0d1fd7

created test folders and made two tests

88f5073

wrote test_get_one_planet_from_fixture

a5f903f

Project Complete

8fd9181

removed comments

ba996be

audreyandoy reviewed May 12, 2022

View reviewed changes


		from .routes import planets_bp
		app.register_blueprint(planets_bp)

Conversation

janderoyalty 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!

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