Phoenix-Liubov_Davidova-Anh_Tran #63

momofAnAl · 2024-10-21T18:12:01Z

No description provided.

anselrognlie

You're off to a good start. Be sure to add a custom attribute to your Planet class, and there's a small typo in your validate method, but otherwise you're right on track.

anselrognlie · 2024-10-24T05:03:43Z

app/models/planets.py

@@ -0,0 +1,24 @@
+class Planet:
+    def __init__(self, id, name, description):


👀 The project specification asks

Define a Planet class with the attributes id, name, and description, and one additional attribute

You have 3 of the attributes, but be sure to add an additional attribute of your choosing as a fourth. It will be easiest to add this before taking on the database work we'll be doing in Wave 03.

anselrognlie · 2024-10-24T05:04:16Z

app/models/planets.py

+        self.name = name
+        self.description = description
+
+    def to_dict(self):


👍 Nice job incorporating this refactor from the live code.

anselrognlie · 2024-10-24T05:05:54Z

app/models/planets.py

+    Planet(4, "Mars", "The Red Planet, known for its volcanoes."),
+    Planet(5, "Jupiter", "The largest planet, famous for its Great Red Spot."),
+    Planet(6, "Saturn", "Known for its stunning rings."),
+    Planet(7, "Uranus", "An ice giant that rotates on its side."),


😍 Uranus is my favorite planet because of how it rotates on its side.

anselrognlie · 2024-10-24T05:07:29Z

app/routes/planets_routes.py

+        planets_response.append(dict(
+            id=planet.id,
+            name=planet.name,
+            description=planet.description    
+
+        ))


We could make use of the to_dict method here, just like you did in the "get one" endpoint.

planets_response.append(planet.to_dict())

anselrognlie · 2024-10-24T05:08:54Z

app/routes/planets_routes.py

+    planets_response = []
+    for planet in planets:
+        planets_response.append(dict(
+            id=planet.id,
+            name=planet.name,
+            description=planet.description    
+
+        ))


This is a great opportunity to try to apply a list comprehension. It works a little better if we also use the to_dict method you added.

planets_response = [planet.to_dict() for planet in planets]

anselrognlie · 2024-10-24T05:10:14Z

app/routes/planets_routes.py

+    return planet.to_dict(), 200
+
+
+def validate_palnet(planet_id):


👀 Watch out for the typo here. If you haven't, I suggest installing the Code Spell Checker VS Code extension.

anselrognlie · 2024-10-24T05:10:49Z

app/routes/planets_routes.py

+def get_one_planet(planet_id):
+    planet = validate_palnet(planet_id)
+
+    return planet.to_dict(), 200


The status code will be 200 by default, so we can leave that off.

return planet.to_dict()

…ibutes

…rate migration

…er of moons

…umber_of_moons

…test.py

anselrognlie

Nice work making the modifications to connect your API to your database, expanding the endpoints, and adding some tests. Everything here should be able to be applied to your task list implementations.

anselrognlie · 2024-11-03T20:58:29Z

app/__init__.py



-def create_app(test_config=None):
+def create_app(config=None):


👍 Thanks for renaming this. This param could be used for scenarios other than testing (the name was left over from the previous curriculum).

anselrognlie · 2024-11-03T21:00:25Z

app/__init__.py

@@ -1,9 +1,21 @@
 from flask import Flask
+from app.db import db, migrate
+from app.models import planets


Note that once we have the Planet model imported in the routes, and the routes are imported here, we don't technically need to import the planets module here. If you find the fact that VS Code shows this as not being used, it would now be safe to remove.

anselrognlie · 2024-11-03T21:04:11Z

app/models/planets.py

+    diameter: Mapped[int]
+    number_of_moons: Mapped[int]


👍 Nice additional data columns.

For numerical values that require a unit (like diameter), it can be useful to note the units somewhere. For example, we could call this diameter_in_km/diameter_in_mi so that it's explicitly a part of the name. If we feel that's too intrusive, a comment about the expected units can still be helpful. We could even add additional methods for retreiving the diameter in certain units to allow the name to remain simple, but have explicit logic that effectively documents the units.

anselrognlie · 2024-11-03T21:07:47Z

app/models/planets.py

+
+
+# class Planet:


It can be tempting to keep old code around for reference, but keep in mind that with git, we can always get back to previous versions if necessary. So in general, prefer to remove old code, or at least put very clear comments around why this code is here and commented out.

Even with git, it can be inconvenient to find a previous version that had our original approach. To help with that, we can create a side branch on a commit that still has the old code, so that we can find it more easily, then remove the code from the main branch.

anselrognlie · 2024-11-03T21:11:24Z

migrations/versions/b0952940e36d_adds_planet_model.py

+"""adds 
+Planet model


Descriptive message for this migration. I'm not sure what caused this to wrap the line like this (generally it should appear as a single line), so try to keep an eye out for what caused this.

anselrognlie · 2024-11-03T21:22:29Z

app/routes/planets_routes.py


+    return Response(status=204, mimetype='application/json')


👍 This is how we say there's no content in the response, but even still, we should keep our type consistently as json for all our endpoints.

anselrognlie · 2024-11-03T21:23:58Z

tests/conftest.py

+    return app.test_client()
+
+@pytest.fixture
+def two_save_planets(app):


👍 Nice test fixture adding two records.

anselrognlie · 2024-11-03T21:27:07Z

tests/conftest.py

+    planet_2 = Planet(name="Neptune", description="The farthest planet, known for strong winds", diameter=49244, number_of_moons=14)
+
+    db.session.add_all([planet_1, planet_2])
+    db.session.commit()


Consider returning the list of records. The return value becomes the value passed in for the fixture in a test. By returning the records, we could use their ids in the dependent tests. While our approach of dropping and recreating the database for each test should guarantee that these records are ids 1 and 2, we could use the ids actually assigned by the database to remove that assumption.

anselrognlie · 2024-11-03T21:28:31Z

tests/test_planets_routes.py

+
+def test_get_one_planet(client, two_save_planets):
+    #Act
+    response = client.get("/planets/1")


If we returned the record list from the fixture, then we could write this line as

response = client.get(f"/planets/{two_save_planets[0].id}")

anselrognlie · 2024-11-03T21:29:35Z

tests/test_planets_routes.py

+    #Assert
+    assert response.status_code == 200
+    assert response_body == {
+        "id": 1,


If we returned the record list from the fixture, then we could check the id as

"id": two_save_planets[0].id,

anselrognlie · 2024-11-03T22:18:52Z

app/models/planets.py

👀 I hadn't noted this before, but prefer to have the name of the model file match the name of the class. So here, since the model is Planet, the file should be planet.py rather than planets.py.

…planets module in __init__.py since we have Planet model imported from the routes

momofAnAl and others added 12 commits October 18, 2024 12:00

Finish wave 1 together

fe0c62a

cann't resolve conflict

67a0ce7

Working on resolve conflict

1322e3f

Conflict resolved

fa4fef4

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

9e89a7c

Resolved conflict

3165fdf

Resolved conflicts

763417e

Working on resolve conflict

2eb21ed

Working on resolve conflict

1f9484f

Update get one planet and handeling errors

6945df6

Created do_dict function and return in planet_id

240d490

adding code 200 in line 23 of routes

ce5ab0d

anselrognlie reviewed Oct 24, 2024

View reviewed changes

momofAnAl and others added 17 commits October 24, 2024 20:35

Fix the typo on validate and add diameter and number_of_moons to attr…

c42ffb3

…ibutes

Adding Pluto

40d6289

using to_dict() in get_all_planets and update list comprehension

32cb34a

create base and db. Update migration and connect db Planet model gene…

7a86581

…rate migration

create POST request for Planet table and add 3 planets

52c0b7d

Create get method for all planets

af930b6

Nothing changed

13ff70f

resolvled conflict

bfd35df

remove id in POST method

1a2fd50

create get single planet, update and deleted

ab90026

there were nothing change

91a07db

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

c2c89a6

Update get all planets with add query params for description and numb…

c28922e

…er of moons

change the .get[] to .get()

461683d

Update the query for number_of_moons_param that take record set for n…

6516233

…umber_of_moons

Create tests environment and update all unit tests and conftest.py

7bccff0

changing the name for test in database variable name in .env and conf…

026ee76

…test.py

anselrognlie reviewed Nov 3, 2024

View reviewed changes

momofAnAl added 7 commits November 3, 2024 19:43

change the name of model file to macth the name of class. Remove the …

ae2d925

…planets module in __init__.py since we have Planet model imported from the routes

change name of column to diameter_in-km

c9a39df

return 2 planets in two_saved_planets

50f3748

update tests id after return the values of two planets

3020969

change the name of clumn diameter

343eb55

delete migration folder

062e52e

install gunicorn and add to requirements.txt

6bfb32b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Phoenix-Liubov_Davidova-Anh_Tran #63

Phoenix-Liubov_Davidova-Anh_Tran #63

momofAnAl commented Oct 21, 2024

anselrognlie left a comment

anselrognlie Oct 24, 2024

anselrognlie Oct 24, 2024

anselrognlie Oct 24, 2024

anselrognlie Oct 24, 2024

anselrognlie Oct 24, 2024

anselrognlie Oct 24, 2024

anselrognlie Oct 24, 2024

anselrognlie left a comment

anselrognlie Nov 3, 2024

anselrognlie Nov 3, 2024

anselrognlie Nov 3, 2024

anselrognlie Nov 3, 2024 •

edited

Loading

anselrognlie Nov 3, 2024

anselrognlie Nov 3, 2024

anselrognlie Nov 3, 2024

anselrognlie Nov 3, 2024

anselrognlie Nov 3, 2024

anselrognlie Nov 3, 2024

anselrognlie Nov 3, 2024

		@@ -0,0 +1,24 @@
		class Planet:
		def __init__(self, id, name, description):



		def create_app(test_config=None):
		def create_app(config=None):

Phoenix-Liubov_Davidova-Anh_Tran #63

Are you sure you want to change the base?

Phoenix-Liubov_Davidova-Anh_Tran #63

Conversation

momofAnAl commented Oct 21, 2024

anselrognlie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anselrognlie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anselrognlie Nov 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anselrognlie Nov 3, 2024 •

edited

Loading