Skip to content

Theresa Davis and Katina Carranza Sea Turtles , Solar-System-api#4

Open
perugia33 wants to merge 11 commits into
ada-c17:mainfrom
perugia33:main
Open

Theresa Davis and Katina Carranza Sea Turtles , Solar-System-api#4
perugia33 wants to merge 11 commits into
ada-c17:mainfrom
perugia33:main

Conversation

@perugia33

@perugia33 perugia33 commented Apr 26, 2022

Copy link
Copy Markdown

Pull Request updated on May 6th to add Test Folder and Files

@perugia33 perugia33 changed the title Tieresa Davis Sea Turtles , Solar-System-apu Tieresa Davis Sea Turtles , Solar-System-api Apr 26, 2022
Comment thread app/routes.py Outdated
Comment on lines +30 to +34
planets_response.append({
"id": planet.id,
"name": planet.name,
"description": planet.description,
"moons": planet.moons

@tgoslee tgoslee Apr 27, 2022

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 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,
            moons=self.moons  
        )

Comment thread app/routes.py Outdated
"description": planet.description,
"moons": planet.moons
})
return jsonify(planets_response)

Copy link
Copy Markdown

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_response), 200

Comment thread app/routes.py Outdated
"description": planet.description,
"moons": planet.moons
}
def validate_planet(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.

💃🏽💃🏽

@perugia33 perugia33 changed the title Tieresa Davis Sea Turtles , Solar-System-api Theresa Davis Sea Turtles , Solar-System-api May 7, 2022
@perugia33 perugia33 changed the title Theresa Davis Sea Turtles , Solar-System-api Theresa Davis and Katina Carranza Sea Turtles , Solar-System-api May 7, 2022
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 lines

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 +16 to +19
planets_response.append({
"id": planet.id,
"name": planet.name,
})

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 I'm not querying by name then I want to get all the information back from the planet. So I would want id, name, description, and moons.

Comment thread app/routes.py
Comment on lines +44 to +49
return {
"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 make an instance method

Comment thread tests/conftest.py
return app.test_client()

@pytest.fixture
def two_saved_planets(app):

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