Various improvements

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
 

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 -%}
+  {%- if depart.hour is defined and arrive.hour is defined -%}
-  {%- set h = mins // 60 -%}
+    {%- set mins = ((arrive - depart).total_seconds() // 60) | int -%}
-  {%- set m = mins % 60 -%}
+    {%- set h = mins // 60 -%}
-  {%- if h %}{{ h }}h {% endif -%}
+    {%- set m = mins % 60 -%}
-  {%- if m %}{{ m }}m{% elif h %}0m{% endif -%}
+    {%- 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 has_time = item.depart.hour is defined and item.arrive.hour is defined %}
-          {% set arrive_date = item.arrive.date() if item.arrive.hour is defined else item.arrive %}
+          {% set depart_date = item.depart.date() if has_time else item.depart %}
-          {% set is_overnight = depart_date != arrive_date %}
+          {% set arrive_date = item.arrive.date() if has_time else item.arrive %}
+          {% set is_overnight = has_time and depart_date != arrive_date %}
           <div class="trip-transport-card my-1">
             <div class="card-body">
               <h5 class="card-title">
@@ -341,12 +344,16 @@
                 {% if is_overnight %}<span class="badge bg-secondary text-nowrap ms-1">Night train</span>{% endif %}
               </h5>
               <p class="card-text">
-                {{ item.depart.strftime("%H:%M") }}
+                {% if has_time %}
-                → {{ item.arrive.strftime("%H:%M") }}{% if is_overnight %} <span class="text-muted small">+1 day</span>{% endif %}
+                  {{ item.depart.strftime("%H:%M") }}
+                  → {{ item.arrive.strftime("%H:%M") }}{% if is_overnight %} <span class="text-muted small">+1 day</span>{% endif %}
+                {% endif %}
                 {% if item.class %}
                   <span class="badge bg-info text-nowrap">{{ item.class }}</span>
                 {% endif %}
-                <span class="text-muted">🕒{{ trip_duration(item.depart, item.arrive) }}</span>
+                {% if has_time %}
+                  <span class="text-muted">🕒{{ trip_duration(item.depart, item.arrive) }}</span>
+                {% endif %}
                 {% if item.distance %}
                   <span class="text-muted">🛤️ {{ "{:,.0f} km".format(item.distance) }}</span>
                 {% endif %}

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

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(
+        def fake_add_coordinates(
-                _routes: list[typing.Any],
+            _routes: list[typing.Any],
-                _coordinates: list[typing.Any],
+            _coordinates: list[typing.Any],
-                data_dir: str,
+            data_dir: str,
-            ) -> None:
+        ) -> None:
-                captured["data_dir"] = data_dir
+            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"
 
+        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:
+def test_trip_page_renders_with_mixed_date_and_datetime_train_leg() -> None:
-    """Trip page should render when train legs use date values (no time)."""
+    """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
+        with (
-        original_get_trip_weather = web_view.agenda.weather.get_trip_weather
+            mock.patch.object(web_view, "get_trip_list", return_value=[trip]),
-        try:
+            mock.patch.object(agenda.weather, "get_trip_weather", return_value=[]),
-            web_view.get_trip_list = lambda: [trip]
+        ):
-            web_view.agenda.weather.get_trip_weather = lambda *_args, **_kwargs: []
             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