From bc7a2e89d0c450367cf0d6d5e275cf1fdb8810fa Mon Sep 17 00:00:00 2001 From: Edward Betts Date: Tue, 30 Jun 2026 12:31:51 +0100 Subject: [PATCH] Various improvements --- agenda/data.py | 20 +++-- templates/trip_page.html | 29 +++--- tests/test_agenda.py | 25 +++++- tests/test_trip_page_route.py | 161 ++++++++++++++++++++++------------ 4 files changed, 160 insertions(+), 75 deletions(-) diff --git a/agenda/data.py b/agenda/data.py index 6b19e73..5517fc3 100644 --- a/agenda/data.py +++ b/agenda/data.py @@ -3,13 +3,13 @@ import asyncio import os import typing -from datetime import date, datetime, timedelta +from datetime import date, datetime, timedelta, timezone from time import time import dateutil.rrule import dateutil.tz import flask -import lxml +import lxml # type: ignore[import-untyped] import pytz from . import ( @@ -68,7 +68,7 @@ async def n_somerset_waste_collection_events( html = await n_somerset_waste.get_html(data_dir, postcode, uprn, force_cache) root = lxml.html.fromstring(html) events = n_somerset_waste.parse(root) - return events + return typing.cast(list[Event], events) async def bristol_waste_collection_events( @@ -76,7 +76,9 @@ async def bristol_waste_collection_events( ) -> list[Event]: """Waste colllection events.""" cache = "force" if force_cache else "recent" - return await bristol_waste.get(start_date, data_dir, uprn, cache) + return typing.cast( + list[Event], await bristol_waste.get(start_date, data_dir, uprn, cache) + ) def find_events_during_stay( @@ -168,6 +170,14 @@ def rocket_launch_events(rockets: list[thespacedevs.Summary]) -> list[Event]: return events +def event_sort_datetime(event: Event) -> datetime: + """Return a timezone-normalized datetime suitable for sorting events.""" + dt = typing.cast(datetime, event.as_datetime) + if dt.tzinfo is None or dt.utcoffset() is None: + return dt + return dt.astimezone(timezone.utc).replace(tzinfo=None) + + async def get_data(now: datetime, config: flask.config.Config) -> AgendaData: """Get data to display on agenda dashboard.""" data_dir = config["DATA_DIR"] @@ -290,7 +300,7 @@ async def get_data(now: datetime, config: flask.config.Config) -> AgendaData: # at the top of the list for today. This is achieved by sorting first by # the datetime attribute, and then ensuring that events with the name # "today" are ordered before others on the same date. - events.sort(key=lambda e: (e.as_datetime, e.name != "today")) + events.sort(key=lambda e: (event_sort_datetime(e), e.name != "today")) reply["gaps"] = gaps diff --git a/templates/trip_page.html b/templates/trip_page.html index 56f61b7..3360414 100644 --- a/templates/trip_page.html +++ b/templates/trip_page.html @@ -7,11 +7,13 @@ {% set row = {"flight": flight_row, "train": train_row, "ferry": ferry_row, "coach": coach_row, "bus": bus_row} %} {% macro trip_duration(depart, arrive) -%} - {%- set mins = ((arrive - depart).total_seconds() // 60) | int -%} - {%- set h = mins // 60 -%} - {%- set m = mins % 60 -%} - {%- if h %}{{ h }}h {% endif -%} - {%- if m %}{{ m }}m{% elif h %}0m{% endif -%} + {%- if depart.hour is defined and arrive.hour is defined -%} + {%- set mins = ((arrive - depart).total_seconds() // 60) | int -%} + {%- set h = mins // 60 -%} + {%- set m = mins % 60 -%} + {%- if h %}{{ h }}h {% endif -%} + {%- if m %}{{ m }}m{% elif h %}0m{% endif -%} + {%- endif -%} {%- endmacro %} {% macro next_and_previous() %} @@ -329,9 +331,10 @@ {% elif e.element_type == "train" %} {% set item = e.detail %} - {% set depart_date = item.depart.date() if item.depart.hour is defined else item.depart %} - {% set arrive_date = item.arrive.date() if item.arrive.hour is defined else item.arrive %} - {% set is_overnight = depart_date != arrive_date %} + {% set has_time = item.depart.hour is defined and item.arrive.hour is defined %} + {% set depart_date = item.depart.date() if has_time else item.depart %} + {% set arrive_date = item.arrive.date() if has_time else item.arrive %} + {% set is_overnight = has_time and depart_date != arrive_date %}
@@ -341,12 +344,16 @@ {% if is_overnight %}Night train{% endif %}

- {{ item.depart.strftime("%H:%M") }} - → {{ item.arrive.strftime("%H:%M") }}{% if is_overnight %} +1 day{% endif %} + {% if has_time %} + {{ item.depart.strftime("%H:%M") }} + → {{ item.arrive.strftime("%H:%M") }}{% if is_overnight %} +1 day{% endif %} + {% endif %} {% if item.class %} {{ item.class }} {% endif %} - 🕒{{ trip_duration(item.depart, item.arrive) }} + {% if has_time %} + 🕒{{ trip_duration(item.depart, item.arrive) }} + {% endif %} {% if item.distance %} 🛤️ {{ "{:,.0f} km".format(item.distance) }} {% endif %} diff --git a/tests/test_agenda.py b/tests/test_agenda.py index fb99451..78484cb 100644 --- a/tests/test_agenda.py +++ b/tests/test_agenda.py @@ -7,21 +7,22 @@ from unittest.mock import patch import pytest from agenda import format_list_with_ampersand, get_country, uk_time -from agenda.data import timezone_transition +from agenda.data import event_sort_datetime, timezone_transition from agenda.economist import publication_dates +from agenda.event import Event from agenda.fx import get_gbpusd from agenda.holidays import get_all from agenda.uk_holiday import bank_holiday_list, get_mothers_day from agenda.utils import timedelta_display -@pytest.fixture +@pytest.fixture # type: ignore[untyped-decorator] def mock_today() -> datetime.date: """Mock the current date for testing purposes.""" return datetime.date(2023, 10, 5) -@pytest.fixture +@pytest.fixture # type: ignore[untyped-decorator] def mock_now() -> datetime.datetime: """Mock the current date and time for testing purposes.""" return datetime.datetime(2023, 10, 5, 12, 0, 0) @@ -44,6 +45,24 @@ def test_timezone_transition(mock_now: datetime.datetime) -> None: assert transitions[0].date.date() == datetime.date(2023, 10, 29) +def test_event_sort_datetime_handles_mixed_timezone_awareness() -> None: + """Event sort keys should be comparable for date, naive, and aware datetimes.""" + events = [ + Event( + name="aware", + date=datetime.datetime( + 2026, 1, 1, 9, 0, tzinfo=datetime.timezone(datetime.timedelta(hours=1)) + ), + ), + Event(name="date", date=datetime.date(2026, 1, 1)), + Event(name="naive", date=datetime.datetime(2026, 1, 1, 8, 30)), + ] + + sorted_events = sorted(events, key=lambda e: (event_sort_datetime(e), e.name)) + + assert [event.name for event in sorted_events] == ["date", "aware", "naive"] + + def test_get_gbpusd_function_exists() -> None: """Test that get_gbpusd function exists and is callable.""" # Simple test to verify the function exists and has correct signature diff --git a/tests/test_trip_page_route.py b/tests/test_trip_page_route.py index d471caa..bf5e470 100644 --- a/tests/test_trip_page_route.py +++ b/tests/test_trip_page_route.py @@ -1,8 +1,14 @@ """Regression tests for trip page route wiring and rendering.""" -from datetime import date +from datetime import date, datetime import typing +from unittest import mock +import flask + +import agenda.trip +import agenda.trip_schengen +import agenda.weather import web_view from agenda.types import Trip @@ -13,61 +19,40 @@ def test_trip_page_passes_data_dir_to_unbooked_flight_helper() -> None: captured: dict[str, str] = {} with web_view.app.app_context(): - original_get_trip_list = web_view.get_trip_list - original_add_schengen = ( - web_view.agenda.trip_schengen.add_schengen_compliance_to_trip - ) - original_collect_trip_coordinates = ( - web_view.agenda.trip.collect_trip_coordinates - ) - original_get_trip_routes = web_view.agenda.trip.get_trip_routes - original_add_coordinates = ( - web_view.agenda.trip.add_coordinates_for_unbooked_flights - ) - original_get_trip_weather = web_view.agenda.weather.get_trip_weather - original_render_template = web_view.flask.render_template - try: - web_view.get_trip_list = lambda: [trip] - web_view.agenda.trip_schengen.add_schengen_compliance_to_trip = lambda t: t - web_view.agenda.trip.collect_trip_coordinates = lambda _trip: [] - web_view.agenda.trip.get_trip_routes = lambda _trip, _data_dir: [] - def fake_add_coordinates( - _routes: list[typing.Any], - _coordinates: list[typing.Any], - data_dir: str, - ) -> None: - captured["data_dir"] = data_dir - - web_view.agenda.trip.add_coordinates_for_unbooked_flights = ( - fake_add_coordinates - ) - web_view.agenda.weather.get_trip_weather = lambda *_args, **_kwargs: [] - web_view.flask.render_template = lambda *_args, **_kwargs: "ok" + def fake_add_coordinates( + _routes: list[typing.Any], + _coordinates: list[typing.Any], + data_dir: str, + ) -> None: + captured["data_dir"] = data_dir + with ( + mock.patch.object(web_view, "get_trip_list", return_value=[trip]), + mock.patch.object( + agenda.trip_schengen, + "add_schengen_compliance_to_trip", + side_effect=lambda t: t, + ), + mock.patch.object(agenda.trip, "collect_trip_coordinates", return_value=[]), + mock.patch.object(agenda.trip, "get_trip_routes", return_value=[]), + mock.patch.object( + agenda.trip, + "add_coordinates_for_unbooked_flights", + side_effect=fake_add_coordinates, + ), + mock.patch.object(agenda.weather, "get_trip_weather", return_value=[]), + mock.patch.object(flask, "render_template", return_value="ok"), + ): with web_view.app.test_request_context("/trip/2025-01-28"): result = web_view.trip_page("2025-01-28") assert result == "ok" assert captured["data_dir"] == web_view.app.config["PERSONAL_DATA"] - finally: - web_view.get_trip_list = original_get_trip_list - web_view.agenda.trip_schengen.add_schengen_compliance_to_trip = ( - original_add_schengen - ) - web_view.agenda.trip.collect_trip_coordinates = ( - original_collect_trip_coordinates - ) - web_view.agenda.trip.get_trip_routes = original_get_trip_routes - web_view.agenda.trip.add_coordinates_for_unbooked_flights = ( - original_add_coordinates - ) - web_view.agenda.weather.get_trip_weather = original_get_trip_weather - web_view.flask.render_template = original_render_template -def test_trip_page_renders_with_date_only_train_leg() -> None: - """Trip page should render when train legs use date values (no time).""" +def test_trip_page_renders_with_mixed_date_and_datetime_train_leg() -> None: + """Trip page should render when train leg times mix dates and datetimes.""" trip = Trip( start=date(2025, 1, 28), travel=[ @@ -93,7 +78,7 @@ def test_trip_page_renders_with_date_only_train_leg() -> None: "from": "A", "to": "B", "depart": date(2025, 1, 28), - "arrive": date(2025, 1, 28), + "arrive": datetime(2025, 1, 28, 16, 49), "from_station": { "name": "A", "country": "gb", @@ -114,11 +99,10 @@ def test_trip_page_renders_with_date_only_train_leg() -> None: ) with web_view.app.app_context(): - original_get_trip_list = web_view.get_trip_list - original_get_trip_weather = web_view.agenda.weather.get_trip_weather - try: - web_view.get_trip_list = lambda: [trip] - web_view.agenda.weather.get_trip_weather = lambda *_args, **_kwargs: [] + with ( + mock.patch.object(web_view, "get_trip_list", return_value=[trip]), + mock.patch.object(agenda.weather, "get_trip_weather", return_value=[]), + ): web_view.app.config["TESTING"] = True with web_view.app.test_client() as client: @@ -126,6 +110,71 @@ def test_trip_page_renders_with_date_only_train_leg() -> None: assert response.status_code == 200 assert b"Test Rail" in response.data - finally: - web_view.get_trip_list = original_get_trip_list - web_view.agenda.weather.get_trip_weather = original_get_trip_weather + + +def test_trip_page_hides_train_times_for_date_only_train_leg() -> None: + """Trip page should not show midnight times for unbooked date-only trains.""" + trip = Trip( + start=date(2026, 8, 23), + travel=[ + { + "type": "train", + "depart": date(2026, 8, 23), + "arrive": date(2026, 8, 23), + "from": "London St Pancras", + "to": "Paris Gare du Nord", + "from_station": { + "name": "London St Pancras", + "country": "gb", + "latitude": 51.532, + "longitude": -0.126, + }, + "to_station": { + "name": "Paris Gare du Nord", + "country": "fr", + "latitude": 48.88, + "longitude": 2.355, + }, + "legs": [ + { + "from": "London St Pancras", + "to": "Paris Gare du Nord", + "depart": date(2026, 8, 23), + "arrive": date(2026, 8, 23), + "from_station": { + "name": "London St Pancras", + "country": "gb", + "latitude": 51.532, + "longitude": -0.126, + }, + "to_station": { + "name": "Paris Gare du Nord", + "country": "fr", + "latitude": 48.88, + "longitude": 2.355, + }, + "operator": "Eurostar", + "class": "Eurostar Plus", + } + ], + } + ], + ) + + with web_view.app.app_context(): + with ( + mock.patch.object(web_view, "get_trip_list", return_value=[trip]), + mock.patch.object(agenda.weather, "get_trip_weather", return_value=[]), + ): + web_view.app.config["TESTING"] = True + + with web_view.app.test_client() as client: + response = client.get("/trip/2026-08-23") + + assert response.status_code == 200 + page = response.data.decode() + assert "London St Pancras → Paris Gare du Nord" in page + assert "Eurostar" in page + assert "Eurostar Plus" in page + assert "00:00" not in page + assert "🕒" not in page