fix: many code improvements

Author: elor555

app/internal/locations.py

@@ -1,6 +1,6 @@
-import requests
-from typing import Dict
+from typing import Any, Dict, Optional
 
+import requests
 from sqlalchemy.orm import Session
 
 from app.database.models import Location
@@ -16,39 +16,43 @@ def create_location_object(location_: Dict[str, str]) -> Location:
         A new Location object.
     """
     return Location(
-        country=location_['country'],
-        city=location_['city'],
-        zip_number=location_['zip_number'],
+        country=location_["country"],
+        city=location_["city"],
+        zip_number=location_["zip_number"],
     )
 
 
-def return_zip_to_location(session: Session) -> str:
-    """Returns the zip number of the user IP location that match location object.
+def return_zip_to_location(session: Session) -> Optional[str]:
+    """Returns the zip number of the user IP location that match location
+     object.
 
     Args:
         session: The database connection.
 
     Returns:
         A zip number for the user location.
     """
-    ip_and_location = requests.get('http://ipinfo.io/json').json()
+    response = requests.get("http://ipinfo.io/json").json()
+    if not response.ok:
+        return None
     for location in session.query(Location).all():
-        if (location.city == ip_and_location['city']) and \
-                (location.country == ip_and_location['country']):
+        if (location.city == response["city"]
+                and location.country == response["country"]):
             return location.zip_number
 
 
-def get_user_location(session: Session) -> str:
+def get_user_location(session: Session) -> Optional[Dict[str, Any]]:
     """Returns the user location.
 
         Args:
             session: The database connection.
 
         Returns:
-            A user location string.
+            A user location.
         """
     my_location = return_zip_to_location(session)
-    location_details = requests.get(
+    response = requests.get(
         f"https://www.hebcal.com/shabbat?cfg=json&geonameid={my_location}"
     )
-    return location_details.json()
+    if response.ok:
+        return response.json()

app/internal/shabbat.py

@@ -1,44 +1,46 @@
-from typing import Dict, Optional, Union
-from datetime import datetime
-
-
-def shabbat_time_by_user_location(
-        shabbat_time: Dict[str, str]
-) -> Dict[str, Union[str, datetime]]:
-    """Returns the shabbat time of the user location..
-
-        Args:
-            Shabbat details.
-
-        Returns:
-            Shabbat start end ending time.
-        """
-    shabbat_limit = {
-        'start_hour': shabbat_time['items'][5]['title'].split(': ')[1],
-        'start_date': datetime.strptime(
-            shabbat_time['items'][5]['date'].split('T')[0], "%Y-%m-%d"
-        ).date(),
-        'end_hour': shabbat_time['items'][7]['title'].split(': ')[1],
-        'end_date': datetime.strptime(
-            shabbat_time['items'][7]['date'].split('T')[0], "%Y-%m-%d"
-        ).date()}
-    return shabbat_limit
+from datetime import date, datetime
+from typing import Any, Dict, Optional, Union
 
 
 def get_shabbat_if_date_friday(
-        shabbat_time: Dict[str, str],
-        date: datetime
-) -> Optional[Dict[str, Union[str, datetime]]]:
+        shabbat_time: Dict[str, Any],
+        calendar_date: date,
+) -> Optional[Dict[str, Union[str, date]]]:
     """Returns shabbat start end ending time if specific date
      is Saturday, else None.
 
         Args:
-            Shabbat details and date.
+            shabbat_time: Shabbat details.
+            calendar_date: date.
 
         Returns:
             Shabbat start end ending time if specific date
              is Saturday, else None
         """
     shabbat_obj = shabbat_time_by_user_location(shabbat_time)
-    if date == shabbat_obj['start_date']:
+    if calendar_date == shabbat_obj["start_date"]:
         return shabbat_obj
+
+
+def shabbat_time_by_user_location(
+        shabbat_time: Dict[str, Any]
+) -> Dict[str, Union[str, date]]:
+    """Returns the shabbat time of the user location..
+
+        Args:
+            shabbat_time: Shabbat details.
+
+        Returns:
+            Shabbat start end ending time.
+        """
+    shabbat_entry = shabbat_time["items"][5]
+    shabbat_exit = shabbat_time["items"][7]
+    shabbat_entry_date = shabbat_entry["date"].split("T")[0]
+    shabbat_exit_date = shabbat_entry["date"].split("T")[0]
+    shabbat_limit = {
+        "start_hour": shabbat_entry["title"].split(": ")[1],
+        "start_date": datetime.strptime(shabbat_entry_date, "%Y-%m-%d").date(),
+        "end_hour": shabbat_exit["title"].split(": ")[1],
+        "end_date": datetime.strptime(shabbat_exit_date, "%Y-%m-%d").date(),
+    }
+    return shabbat_limit

app/routers/dayview.py

@@ -4,10 +4,11 @@
 
 from fastapi import APIRouter, Depends, HTTPException, Request
 
-from app.database.models import Event, User
+from app.database.models import Event
 from app.dependencies import get_db, templates
 from app.internal import international_days, locations, shabbat, zodiac
 from app.internal.security.dependencies import current_user
+from app.internal.security.schema import CurrentUser
 from app.routers.user import get_all_user_events
 
 router = APIRouter()
@@ -191,7 +192,7 @@ async def dayview(
     date: str,
     view="day",
     session=Depends(get_db),
-    user: User = Depends(current_user),
+    user: CurrentUser = Depends(current_user),
 ):
     try:
         day = datetime.strptime(date, "%Y-%m-%d")
@@ -211,10 +212,10 @@ async def dayview(
     current_time_with_attrs = CurrentTimeAttributes(date=day)
     inter_day = international_days.get_international_day_per_day(session, day)
     location_and_shabbat = locations.get_user_location(session)
-    location = location_and_shabbat['location']['title']
+    location = location_and_shabbat["location"]["title"]
     shabbat_obj = shabbat.get_shabbat_if_date_friday(
         location_and_shabbat,
-        day.date()
+        day.date(),
     )
     month = day.strftime("%B").upper()
     return templates.TemplateResponse(

tests/fixtures/location_fixture.py

@@ -2,28 +2,40 @@
 
 from app.internal import shabbat
 
-SHABBAT_TIME = {'items': [
-    {'title': 'Fast begins'},
-    {'title': "Ta'anit Esther"},
-    {'title': 'Fast ends'},
-    {'title': 'Erev Purim'},
-    {'title': 'Purim'},
-    {'title': 'Candle lighting: 17:15',
-     'date': '2021-02-26T17:15:00+02:00',
-     'category': 'candles',
-     'title_orig': 'Candle lighting',
-     'hebrew': 'הדלקת נרות'},
-    {'title': 'Parashat Tetzaveh'},
-    {'title': 'Havdalah: 18:11',
-     'date': '2021-02-27T18:11:00+02:00',
-     'category': 'havdalah',
-     'title_orig': 'Havdalah',
-     'hebrew': 'הבדלה'}
-]}
-DATE1 = date(2021, 2, 27)
-FRIDAY = date(2021, 2, 27)
+SHABBAT_TIME = {
+    "items":
+        [
+            {"title": "Fast begins"},
+            {"title": "Ta'anit Esther"},
+            {"title": "Fast ends"},
+            {"title": "Erev Purim"},
+            {"title": "Purim"},
+            {
+                "title": "Candle lighting: 17:15",
+                "date": "2021-02-26T17:15:00+02:00",
+                "category": "candles",
+                "title_orig": "Candle lighting",
+                "hebrew": "הדלקת נרות",
+            },
+            {"title": "Parashat Tetzaveh"},
+            {
+                "title": "Havdalah: 18:11",
+                "date": "2021-02-27T18:11:00+02:00",
+                "category": "havdalah",
+                "title_orig": "Havdalah",
+                "hebrew": "הבדלה",
+            },
+        ]
+}
+BAD_DAY = date(2021, 2, 27)
+FRIDAY = date(2021, 2, 26)
 
 
 def test_return_none_if_date_no_friday():
-    result = shabbat.get_shabbat_if_date_friday(SHABBAT_TIME, DATE1)
+    result = shabbat.get_shabbat_if_date_friday(SHABBAT_TIME, BAD_DAY)
     assert result is None
+
+
+def test_return_if_date_is_friday():
+    result = shabbat.get_shabbat_if_date_friday(SHABBAT_TIME, FRIDAY)
+    assert result