Skip to content

Commit 670e50d

Browse files
committed
fix: address PR review comments
- Remove unused import calculate_current_month_orders_total - Fix timezone inconsistencies (use ZoneInfo consistently) - Create parse_delivery_datetime_string helper to eliminate code duplication - Refactor NextOrderSince and NextOrderTill to use helper function - Fix performance issue: move order processing to async callback instead of native_value property - Remove redundant len(orders) == 0 check in utils.py Addresses all Copilot AI review comments from PR #54
1 parent 119ac3e commit 670e50d

2 files changed

Lines changed: 42 additions & 34 deletions

File tree

custom_components/rohlikcz/sensor.py

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
ICON_NEXT_ORDER_TILL, ICON_INFO, ICON_DELIVERY_TIME, ICON_MONTHLY_SPENT
2121
from .entity import BaseEntity
2222
from .hub import RohlikAccount
23-
from .utils import extract_delivery_datetime, calculate_current_month_orders_total, get_earliest_order
23+
from .utils import extract_delivery_datetime, get_earliest_order, parse_delivery_datetime_string
2424

2525
SCAN_INTERVAL = timedelta(seconds=600)
2626

@@ -441,7 +441,7 @@ def __init__(self, rohlik_account: RohlikAccount) -> None:
441441
super().__init__(rohlik_account)
442442
self._monthly_total: float = 0.0
443443
self._processed_orders: set[str] = set() # Store order IDs
444-
self._current_month: str = datetime.now().strftime("%Y-%m")
444+
self._current_month: str = datetime.now(ZoneInfo("Europe/Prague")).strftime("%Y-%m")
445445
self._last_reset: datetime | None = None
446446

447447
def _is_order_final(self, order: dict) -> bool:
@@ -480,18 +480,18 @@ async def async_added_to_hass(self) -> None:
480480
if (last_state := await self.async_get_last_state()) is not None:
481481
self._monthly_total = last_state.attributes.get("monthly_total", 0.0)
482482
self._processed_orders = set(last_state.attributes.get("processed_orders", []))
483-
self._current_month = last_state.attributes.get("current_month", datetime.now().strftime("%Y-%m"))
483+
self._current_month = last_state.attributes.get("current_month", datetime.now(ZoneInfo("Europe/Prague")).strftime("%Y-%m"))
484484
if last_reset_str := last_state.attributes.get("last_reset"):
485485
self._last_reset = datetime.fromisoformat(last_reset_str)
486486

487487
self._check_and_reset_month()
488488
self._process_new_orders()
489489

490-
self._rohlik_account.register_callback(self.async_write_ha_state)
490+
self._rohlik_account.register_callback(self._handle_coordinator_update)
491491

492492
def _check_and_reset_month(self) -> None:
493493
"""Reset total if month changed."""
494-
current_month = datetime.now().strftime("%Y-%m")
494+
current_month = datetime.now(ZoneInfo("Europe/Prague")).strftime("%Y-%m")
495495
if current_month != self._current_month:
496496
_LOGGER.info(f"Month changed from {self._current_month} to {current_month}, resetting monthly total")
497497
self._monthly_total = 0.0
@@ -509,7 +509,7 @@ def _process_new_orders(self) -> None:
509509
if not orders:
510510
return
511511

512-
current_month_pattern = datetime.now().strftime("%Y-%m-")
512+
current_month_pattern = datetime.now(ZoneInfo("Europe/Prague")).strftime("%Y-%m-")
513513
new_orders_count = 0
514514

515515
for order in orders:
@@ -554,11 +554,15 @@ def _process_new_orders(self) -> None:
554554
if new_orders_count > 0:
555555
_LOGGER.info(f"Processed {new_orders_count} new order(s). Monthly total: {self._monthly_total} CZK")
556556

557+
async def _handle_coordinator_update(self) -> None:
558+
"""Handle coordinator updates by processing new orders and updating state."""
559+
self._check_and_reset_month()
560+
self._process_new_orders()
561+
await self.async_write_ha_state()
562+
557563
@property
558564
def native_value(self) -> float | None:
559565
"""Returns amount spent in current month."""
560-
self._check_and_reset_month()
561-
self._process_new_orders()
562566
return self._monthly_total if self._monthly_total > 0 else 0.0
563567

564568
@property
@@ -577,7 +581,7 @@ def icon(self) -> str:
577581
return ICON_MONTHLY_SPENT
578582

579583
async def async_will_remove_from_hass(self) -> None:
580-
self._rohlik_account.remove_callback(self.async_write_ha_state)
584+
self._rohlik_account.remove_callback(self._handle_coordinator_update)
581585

582586

583587
class NoLimitOrders(BaseEntity, SensorEntity):
@@ -735,18 +739,8 @@ def native_value(self) -> datetime | None:
735739
"""Returns start of delivery window for the earliest order."""
736740
earliest_order = get_earliest_order(self._rohlik_account.data.get('next_order', []))
737741
if earliest_order:
738-
try:
739-
slot_start = datetime.strptime(earliest_order.get("deliverySlot", {}).get("since", None),
740-
"%Y-%m-%dT%H:%M:%S.%f%z")
741-
return slot_start
742-
except (ValueError, TypeError):
743-
# Try without microseconds if the format doesn't match
744-
try:
745-
slot_start = datetime.strptime(earliest_order.get("deliverySlot", {}).get("since", None),
746-
"%Y-%m-%dT%H:%M:%S%z")
747-
return slot_start
748-
except (ValueError, TypeError):
749-
return None
742+
since_str = earliest_order.get("deliverySlot", {}).get("since", None)
743+
return parse_delivery_datetime_string(since_str)
750744
return None
751745

752746
@property
@@ -771,18 +765,8 @@ def native_value(self) -> datetime | None:
771765
"""Returns end of delivery window for the earliest order."""
772766
earliest_order = get_earliest_order(self._rohlik_account.data.get('next_order', []))
773767
if earliest_order:
774-
try:
775-
slot_end = datetime.strptime(earliest_order.get("deliverySlot", {}).get("till", None),
776-
"%Y-%m-%dT%H:%M:%S.%f%z")
777-
return slot_end
778-
except (ValueError, TypeError):
779-
# Try without microseconds if the format doesn't match
780-
try:
781-
slot_end = datetime.strptime(earliest_order.get("deliverySlot", {}).get("till", None),
782-
"%Y-%m-%dT%H:%M:%S%z")
783-
return slot_end
784-
except (ValueError, TypeError):
785-
return None
768+
till_str = earliest_order.get("deliverySlot", {}).get("till", None)
769+
return parse_delivery_datetime_string(till_str)
786770
return None
787771

788772
@property

custom_components/rohlikcz/utils.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,30 @@ def extract_delivery_datetime(text: str) -> datetime | None:
162162
return None
163163

164164

165+
def parse_delivery_datetime_string(datetime_str: str) -> datetime | None:
166+
"""
167+
Parse a delivery datetime string with fallback for different formats.
168+
169+
Args:
170+
datetime_str (str): Datetime string to parse
171+
172+
Returns:
173+
datetime: Parsed datetime object, or None if parsing fails
174+
"""
175+
if datetime_str is None:
176+
return None
177+
178+
try:
179+
# Try parsing with microseconds first
180+
return datetime.strptime(datetime_str, "%Y-%m-%dT%H:%M:%S.%f%z")
181+
except ValueError:
182+
# Try without microseconds if the format doesn't match
183+
try:
184+
return datetime.strptime(datetime_str, "%Y-%m-%dT%H:%M:%S%z")
185+
except ValueError:
186+
return None
187+
188+
165189
def get_earliest_order(orders: list) -> dict | None:
166190
"""
167191
Find the order with the earliest delivery time from a list of orders.
@@ -172,7 +196,7 @@ def get_earliest_order(orders: list) -> dict | None:
172196
Returns:
173197
dict: The order with the earliest delivery time, or None if no valid order found
174198
"""
175-
if not orders or len(orders) == 0:
199+
if not orders:
176200
return None
177201

178202
earliest_order = None

0 commit comments

Comments
 (0)