From 9505447b02a60b8b4ff8c25810fd6949f37c43c0 Mon Sep 17 00:00:00 2001 From: "shai.ungar" Date: Fri, 8 Nov 2024 15:13:20 +0200 Subject: [PATCH 1/5] Fix issue when timestamp is None. Check first for None before applying isoformat on it --- .../components/seventeentrack/services.py | 4 +- .../snapshots/test_services.ambr | 30 +++++++++++++++ .../seventeentrack/test_services.py | 38 +++++++++++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/seventeentrack/services.py b/homeassistant/components/seventeentrack/services.py index 0833bc0a97bc70..0942d515709449 100644 --- a/homeassistant/components/seventeentrack/services.py +++ b/homeassistant/components/seventeentrack/services.py @@ -89,7 +89,9 @@ async def get_packages(call: ServiceCall) -> ServiceResponse: ATTR_TRACKING_NUMBER: package.tracking_number, ATTR_LOCATION: package.location, ATTR_STATUS: package.status, - ATTR_TIMESTAMP: package.timestamp.isoformat(), + ATTR_TIMESTAMP: package.timestamp.isoformat() + if package.timestamp + else "", ATTR_INFO_TEXT: package.info_text, ATTR_FRIENDLY_NAME: package.friendly_name, } diff --git a/tests/components/seventeentrack/snapshots/test_services.ambr b/tests/components/seventeentrack/snapshots/test_services.ambr index 568acea33a5fe5..42084462c8b9fe 100644 --- a/tests/components/seventeentrack/snapshots/test_services.ambr +++ b/tests/components/seventeentrack/snapshots/test_services.ambr @@ -71,3 +71,33 @@ ]), }) # --- +# name: test_packages_with_none_timestamp + dict({ + 'packages': list([ + dict({ + 'destination_country': 'Belgium', + 'friendly_name': 'friendly name 1', + 'info_text': 'info text 1', + 'location': 'location 1', + 'origin_country': 'Belgium', + 'package_type': 'Registered Parcel', + 'status': 'In Transit', + 'timestamp': '', + 'tracking_info_language': 'Unknown', + 'tracking_number': '456', + }), + dict({ + 'destination_country': 'Belgium', + 'friendly_name': 'friendly name 2', + 'info_text': 'info text 1', + 'location': 'location 1', + 'origin_country': 'Belgium', + 'package_type': 'Registered Parcel', + 'status': 'Delivered', + 'timestamp': '2020-08-10T10:32:00+00:00', + 'tracking_info_language': 'Unknown', + 'tracking_number': '789', + }), + ]), + }) +# --- diff --git a/tests/components/seventeentrack/test_services.py b/tests/components/seventeentrack/test_services.py index 54c9349c121fdc..bbd5644ad63c7d 100644 --- a/tests/components/seventeentrack/test_services.py +++ b/tests/components/seventeentrack/test_services.py @@ -150,6 +150,28 @@ async def test_archive_package( ) +async def test_packages_with_none_timestamp( + hass: HomeAssistant, + mock_seventeentrack: AsyncMock, + mock_config_entry: MockConfigEntry, + snapshot: SnapshotAssertion, +) -> None: + """Ensure service returns all packages when non provided.""" + await _mock_invalid_packages(mock_seventeentrack) + await init_integration(hass, mock_config_entry) + service_response = await hass.services.async_call( + DOMAIN, + SERVICE_GET_PACKAGES, + { + CONFIG_ENTRY_ID_KEY: mock_config_entry.entry_id, + }, + blocking=True, + return_response=True, + ) + + assert service_response == snapshot + + async def _mock_packages(mock_seventeentrack): package1 = get_package(status=10) package2 = get_package( @@ -167,3 +189,19 @@ async def _mock_packages(mock_seventeentrack): package2, package3, ] + + +async def _mock_invalid_packages(mock_seventeentrack): + package1 = get_package( + status=10, + timestamp=None, + ) + package2 = get_package( + tracking_number="789", + friendly_name="friendly name 2", + status=40, + ) + mock_seventeentrack.return_value.profile.packages.return_value = [ + package1, + package2, + ] From 5702015ef80874371d21e9579f2e04111095e10e Mon Sep 17 00:00:00 2001 From: "shai.ungar" Date: Fri, 8 Nov 2024 16:00:48 +0200 Subject: [PATCH 2/5] make timestamp optional --- .../components/seventeentrack/services.py | 33 +++++++++++-------- .../snapshots/test_services.ambr | 1 - 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/seventeentrack/services.py b/homeassistant/components/seventeentrack/services.py index 0942d515709449..047b39a5911d30 100644 --- a/homeassistant/components/seventeentrack/services.py +++ b/homeassistant/components/seventeentrack/services.py @@ -81,20 +81,7 @@ async def get_packages(call: ServiceCall) -> ServiceResponse: return { "packages": [ - { - ATTR_DESTINATION_COUNTRY: package.destination_country, - ATTR_ORIGIN_COUNTRY: package.origin_country, - ATTR_PACKAGE_TYPE: package.package_type, - ATTR_TRACKING_INFO_LANGUAGE: package.tracking_info_language, - ATTR_TRACKING_NUMBER: package.tracking_number, - ATTR_LOCATION: package.location, - ATTR_STATUS: package.status, - ATTR_TIMESTAMP: package.timestamp.isoformat() - if package.timestamp - else "", - ATTR_INFO_TEXT: package.info_text, - ATTR_FRIENDLY_NAME: package.friendly_name, - } + package_to_dict(package) for package in live_packages if slugify(package.status) in package_states or package_states == [] ] @@ -112,6 +99,24 @@ async def archive_package(call: ServiceCall) -> None: await seventeen_coordinator.client.profile.archive_package(tracking_number) + def package_to_dict(package): + return { + ATTR_DESTINATION_COUNTRY: package.destination_country, + ATTR_ORIGIN_COUNTRY: package.origin_country, + ATTR_PACKAGE_TYPE: package.package_type, + ATTR_TRACKING_INFO_LANGUAGE: package.tracking_info_language, + ATTR_TRACKING_NUMBER: package.tracking_number, + ATTR_LOCATION: package.location, + ATTR_STATUS: package.status, + ATTR_INFO_TEXT: package.info_text, + ATTR_FRIENDLY_NAME: package.friendly_name, + **( + {ATTR_TIMESTAMP: package.timestamp.isoformat()} + if package.timestamp + else {} + ), + } + async def _validate_service(config_entry_id): entry: ConfigEntry | None = hass.config_entries.async_get_entry(config_entry_id) if not entry: diff --git a/tests/components/seventeentrack/snapshots/test_services.ambr b/tests/components/seventeentrack/snapshots/test_services.ambr index 42084462c8b9fe..e172a2de5947dd 100644 --- a/tests/components/seventeentrack/snapshots/test_services.ambr +++ b/tests/components/seventeentrack/snapshots/test_services.ambr @@ -82,7 +82,6 @@ 'origin_country': 'Belgium', 'package_type': 'Registered Parcel', 'status': 'In Transit', - 'timestamp': '', 'tracking_info_language': 'Unknown', 'tracking_number': '456', }), From 26f690ede609d0945171cce5cbe56a3a94a79ab5 Mon Sep 17 00:00:00 2001 From: Shai Ungar Date: Fri, 8 Nov 2024 16:06:05 +0200 Subject: [PATCH 3/5] Update homeassistant/components/seventeentrack/services.py Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> --- homeassistant/components/seventeentrack/services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/seventeentrack/services.py b/homeassistant/components/seventeentrack/services.py index 047b39a5911d30..63aee15d6b9809 100644 --- a/homeassistant/components/seventeentrack/services.py +++ b/homeassistant/components/seventeentrack/services.py @@ -100,7 +100,7 @@ async def archive_package(call: ServiceCall) -> None: await seventeen_coordinator.client.profile.archive_package(tracking_number) def package_to_dict(package): - return { + result = { ATTR_DESTINATION_COUNTRY: package.destination_country, ATTR_ORIGIN_COUNTRY: package.origin_country, ATTR_PACKAGE_TYPE: package.package_type, From c630f4e4afd8917ee4ef35982d7aa7cba46dd8a9 Mon Sep 17 00:00:00 2001 From: Shai Ungar Date: Fri, 8 Nov 2024 16:06:12 +0200 Subject: [PATCH 4/5] Update homeassistant/components/seventeentrack/services.py Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> --- homeassistant/components/seventeentrack/services.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/homeassistant/components/seventeentrack/services.py b/homeassistant/components/seventeentrack/services.py index 63aee15d6b9809..c76e9a6982a239 100644 --- a/homeassistant/components/seventeentrack/services.py +++ b/homeassistant/components/seventeentrack/services.py @@ -110,12 +110,10 @@ def package_to_dict(package): ATTR_STATUS: package.status, ATTR_INFO_TEXT: package.info_text, ATTR_FRIENDLY_NAME: package.friendly_name, - **( - {ATTR_TIMESTAMP: package.timestamp.isoformat()} - if package.timestamp - else {} - ), } + if timestamp := package.timestamp: + result[ATTR_TIMESTAMP] = timestamp.isoformat() + return result async def _validate_service(config_entry_id): entry: ConfigEntry | None = hass.config_entries.async_get_entry(config_entry_id) From 85d8e3ede187e74ea1005567833bd066cf5adfa7 Mon Sep 17 00:00:00 2001 From: "shai.ungar" Date: Fri, 8 Nov 2024 16:10:35 +0200 Subject: [PATCH 5/5] add type hints --- homeassistant/components/seventeentrack/services.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/seventeentrack/services.py b/homeassistant/components/seventeentrack/services.py index c76e9a6982a239..54c23e6d6191ae 100644 --- a/homeassistant/components/seventeentrack/services.py +++ b/homeassistant/components/seventeentrack/services.py @@ -1,8 +1,8 @@ """Services for the seventeentrack integration.""" -from typing import Final +from typing import Any, Final -from pyseventeentrack.package import PACKAGE_STATUS_MAP +from pyseventeentrack.package import PACKAGE_STATUS_MAP, Package import voluptuous as vol from homeassistant.config_entries import ConfigEntry, ConfigEntryState @@ -99,7 +99,7 @@ async def archive_package(call: ServiceCall) -> None: await seventeen_coordinator.client.profile.archive_package(tracking_number) - def package_to_dict(package): + def package_to_dict(package: Package) -> dict[str, Any]: result = { ATTR_DESTINATION_COUNTRY: package.destination_country, ATTR_ORIGIN_COUNTRY: package.origin_country,