Skip to content

Sea Turtles - Katina Carranza/Theresa Davis#30

Open
kmcarranza wants to merge 8 commits intoada-c17:mainfrom
perugia33:branch-for-learn-submission
Open

Sea Turtles - Katina Carranza/Theresa Davis#30
kmcarranza wants to merge 8 commits intoada-c17:mainfrom
perugia33:branch-for-learn-submission

Conversation

@kmcarranza
Copy link
Copy Markdown

No description provided.

Comment thread app/__init__.py
# app.config['SQLALCHEMY_DATABASE_URI'] = 'postgresql+psycopg2://postgres:postgres@localhost:5432/solar_system_development'

if not test_config:
app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if you move this line to line 16 then you don't have to put it on line 23

Comment thread app/__init__.py
Comment on lines +14 to +15
# app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
# app.config['SQLALCHEMY_DATABASE_URI'] = 'postgresql+psycopg2://postgres:postgres@localhost:5432/solar_system_development'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you can remove these

Comment thread app/models/planet.py
@@ -0,0 +1,8 @@
from app import db

class Planet(db.Model):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

looks good. Would you make these columns nullable?

Comment thread app/routes.py
Comment on lines +8 to +24
# class Planet:
# def __init__(self, id, name, description, moons):
# self.id = id
# self.name = name
# self.description = description
# self.moons = moons

# planets = [
# Planet(1, "Mercury", "Mercury is the first planet from the Sun. It is the smallest and the fastest of all planets in our Solar system",0),
# Planet(2, "Venus", "Venus is the second planet from the Sun. It spins clockwise on its axis and is the second brightest natural object in the night sky after the Moon", 0),
# Planet(3, "Earth", "Third planet from the Sun and our home planet is 4.5 billion years old. The only planet to sustain a liquid surface area", 1),
# Planet(4, "Mars", "This planet is named after Mars, the Roman god of war. It's landmass which is similar to Earth, has a reddish-brown color.", 2),
# Planet(5, "Jupiter", "Jupiter is the largest of all planets in our Solar system. This gaseous planet is more than twice as large as the other planets combined", 79),
# Planet(6, "Saturn", "Saturn is the second largest of the planets and is primarily composed of hydrogen gas. It is known for its over 30 stunning Rings of ice", 82),
# Planet(7, "Uranus", "This ice giant is massive with a size four times wider than of Earth. It has 13 known Rings", 27),
# Planet(8, "Neptune", "At a distance of 2.8 billion miles from the Sun, this massive planet is four times wider than Earth. It's mainly made up of icy materials- water, methane and ammonia with 7 Rings", 14),
# ]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you can remove this or move it to another file that has your notes

Comment thread app/routes.py
Comment on lines +41 to +46
planets_response.append({
"id": planet.id,
"name": planet.name,
"description": planet.description,
"moons": planet.moons
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you can move this to your Planet class and create an instance method that you can reference multiple times.

Comment thread app/routes.py
return make_response(f"Planet with id {planet.id} was successfully deleted!")

# Helper Function
def validate_planet(id):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

looks good

Comment thread app/routes.py
Comment on lines +114 to +132
# @planets_bp.route("/<planet_id>", methods=["GET"])
# def handle_planet(planet_id):
# planet = validate_planet(planet_id)
# return {
# "id": planet.id,
# "name": planet.name,
# "description": planet.description,
# "moons": planet.moons
# }
# def validate_planet(planet_id):
# try:
# planet_id = int(planet_id)
# except:
# abort(make_response({"message":f"planet {planet_id} invalid"}, 400))

# for planet in planets:
# if planet.id == planet_id:
# return planet
# abort(make_response({"message": f"planet {planet_id} not found"}, 404))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you can remove this

Comment thread tests/conftest.py
from app.models.planet import Planet


@pytest.fixture
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

looks good

Comment thread tests/test_routes.py
assert response.status_code == 200
assert response_body == []

def test_get_planets(client, two_saved_planets):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this test name could be more descriptive. test_get_planet_by_id since you are looking for planet 1

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