refactor: replace inline duration formatting with utility functions for consistent text and table output
This commit is contained in:
@@ -6,6 +6,8 @@ from strava_mcp_server.utils import (
|
||||
parse_iso_to_unix,
|
||||
format_date_iso,
|
||||
format_date_human,
|
||||
format_duration_text,
|
||||
format_duration_table,
|
||||
)
|
||||
|
||||
|
||||
@@ -66,7 +68,7 @@ def register(mcp: FastMCP, strava: StravaClient) -> None:
|
||||
"start_date": format_date_iso(start_date_raw),
|
||||
"start_date_local": format_date_human(start_date_raw),
|
||||
"distance": f"{a.get('distance', 0) / 1000:.2f} km",
|
||||
"moving_time": f"{a.get('moving_time', 0) / 60:.1f} min",
|
||||
"moving_time": format_duration_table(a.get("moving_time")),
|
||||
"total_elevation_gain": f"{a.get('total_elevation_gain', 0):.0f} m",
|
||||
"average_heartrate": a.get("average_heartrate"),
|
||||
"gear_id": a.get("gear_id"),
|
||||
@@ -125,7 +127,7 @@ def register(mcp: FastMCP, strava: StravaClient) -> None:
|
||||
sport = activity.get("sport_type") or activity.get("type", "N/A")
|
||||
date = format_date_human(activity.get("start_date"))
|
||||
dist = f"{activity.get('distance', 0) / 1000:.2f} km"
|
||||
time = f"{activity.get('moving_time', 0) / 60:.1f} min"
|
||||
time = format_duration_text(activity.get("moving_time"))
|
||||
elev = f"{activity.get('total_elevation_gain', 0):.0f} m"
|
||||
avg_hr = (
|
||||
f"{activity.get('average_heartrate', 0):.0f} bpm"
|
||||
@@ -254,8 +256,8 @@ def register(mcp: FastMCP, strava: StravaClient) -> None:
|
||||
"lap_index": lap.get("lap_index"),
|
||||
"name": lap.get("name"),
|
||||
"distance": f"{lap.get('distance', 0) / 1000:.3f} km",
|
||||
"moving_time": f"{lap.get('moving_time', 0) / 60:.1f} min",
|
||||
"elapsed_time": f"{lap.get('elapsed_time', 0) / 60:.1f} min",
|
||||
"moving_time": format_duration_table(lap.get("moving_time")),
|
||||
"elapsed_time": format_duration_table(lap.get("elapsed_time")),
|
||||
"average_speed": f"{lap.get('average_speed', 0) * 3.6:.2f} km/h",
|
||||
"max_speed": f"{lap.get('max_speed', 0) * 3.6:.2f} km/h",
|
||||
"elevation_gain": f"{lap.get('total_elevation_gain', 0):.0f} m",
|
||||
@@ -301,7 +303,7 @@ def register(mcp: FastMCP, strava: StravaClient) -> None:
|
||||
"zone": i + 1,
|
||||
"min": b.get("min", 0),
|
||||
"max": b.get("max", -1),
|
||||
"time_in_zone": f"{b.get('time', 0) / 60:.1f} min",
|
||||
"time_in_zone": format_duration_text(b.get("time")),
|
||||
}
|
||||
for i, b in enumerate(zone.get("distribution_buckets", []))
|
||||
]
|
||||
|
||||
@@ -2,6 +2,7 @@ import json
|
||||
from mcp.server.fastmcp import FastMCP
|
||||
from mcp.types import TextContent, Annotations, EmbeddedResource, TextResourceContents
|
||||
from strava_mcp_server.strava_client import StravaClient
|
||||
from strava_mcp_server.utils import format_duration_table
|
||||
|
||||
|
||||
def _resource(uri: str, data) -> EmbeddedResource:
|
||||
@@ -79,7 +80,7 @@ def register(mcp: FastMCP, strava: StravaClient) -> None:
|
||||
"name": a.get("name"),
|
||||
"sport_type": a.get("sport_type") or a.get("type"),
|
||||
"distance": f"{a.get('distance', 0) / 1000:.2f} km",
|
||||
"moving_time": f"{a.get('moving_time', 0) / 60:.1f} min",
|
||||
"moving_time": format_duration_table(a.get("moving_time")),
|
||||
"total_elevation_gain": f"{a.get('total_elevation_gain', 0):.0f} m",
|
||||
"athlete": f"{a.get('athlete', {}).get('firstname')} {a.get('athlete', {}).get('lastname')}",
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ import json
|
||||
from mcp.server.fastmcp import FastMCP
|
||||
from mcp.types import TextContent, Annotations, EmbeddedResource, TextResourceContents
|
||||
from strava_mcp_server.strava_client import StravaClient
|
||||
from strava_mcp_server.utils import format_duration_text, format_duration_table
|
||||
|
||||
|
||||
def _resource(uri: str, data) -> EmbeddedResource:
|
||||
@@ -43,7 +44,9 @@ def register(mcp: FastMCP, strava: StravaClient) -> None:
|
||||
"sub_type": r.get("sub_type"),
|
||||
"distance": f"{r.get('distance', 0) / 1000:.2f} km",
|
||||
"elevation_gain": f"{r.get('elevation_gain', 0):.0f} m",
|
||||
"estimated_moving_time": f"{r.get('estimated_moving_time', 0) / 60:.0f} min",
|
||||
"estimated_moving_time": format_duration_table(
|
||||
r.get("estimated_moving_time")
|
||||
),
|
||||
"starred": r.get("starred", False),
|
||||
"private": r.get("private", False),
|
||||
}
|
||||
@@ -82,7 +85,9 @@ def register(mcp: FastMCP, strava: StravaClient) -> None:
|
||||
else "Other",
|
||||
"distance": f"{r.get('distance', 0) / 1000:.2f} km",
|
||||
"elevation_gain": f"{r.get('elevation_gain', 0):.0f} m",
|
||||
"estimated_moving_time": f"{r.get('estimated_moving_time', 0) / 60:.0f} min",
|
||||
"estimated_moving_time": format_duration_text(
|
||||
r.get("estimated_moving_time")
|
||||
),
|
||||
"starred": r.get("starred", False),
|
||||
"private": r.get("private", False),
|
||||
"segments": [
|
||||
|
||||
@@ -2,7 +2,12 @@ import json
|
||||
from mcp.server.fastmcp import FastMCP
|
||||
from mcp.types import TextContent, Annotations, EmbeddedResource, TextResourceContents
|
||||
from strava_mcp_server.strava_client import StravaClient
|
||||
from strava_mcp_server.utils import format_date_iso, format_date_human
|
||||
from strava_mcp_server.utils import (
|
||||
format_date_iso,
|
||||
format_date_human,
|
||||
format_duration_text,
|
||||
format_duration_table,
|
||||
)
|
||||
|
||||
|
||||
def _resource(uri: str, data) -> EmbeddedResource:
|
||||
@@ -35,8 +40,8 @@ def register(mcp: FastMCP, strava: StravaClient) -> None:
|
||||
data = {
|
||||
"id": str(e.get("id")),
|
||||
"name": e.get("name"),
|
||||
"elapsed_time": f"{e.get('elapsed_time', 0) / 60:.1f} min",
|
||||
"moving_time": f"{e.get('moving_time', 0) / 60:.1f} min",
|
||||
"elapsed_time": format_duration_text(e.get("elapsed_time")),
|
||||
"moving_time": format_duration_text(e.get("moving_time")),
|
||||
"start_date": format_date_iso(e.get("start_date")),
|
||||
"distance": f"{e.get('distance', 0) / 1000:.2f} km",
|
||||
"average_watts": e.get("average_watts"),
|
||||
@@ -99,8 +104,8 @@ def register(mcp: FastMCP, strava: StravaClient) -> None:
|
||||
data = [
|
||||
{
|
||||
"id": str(e.get("id")),
|
||||
"elapsed_time": f"{e.get('elapsed_time', 0) / 60:.1f} min",
|
||||
"moving_time": f"{e.get('moving_time', 0) / 60:.1f} min",
|
||||
"elapsed_time": format_duration_table(e.get("elapsed_time")),
|
||||
"moving_time": format_duration_table(e.get("moving_time")),
|
||||
"start_date": format_date_iso(e.get("start_date")),
|
||||
"average_watts": e.get("average_watts"),
|
||||
"average_heartrate": e.get("average_heartrate"),
|
||||
|
||||
@@ -61,3 +61,47 @@ def format_date_human(date_input: Optional[str | datetime]) -> str:
|
||||
return dt.strftime("%d.%m.%Y %H:%M")
|
||||
except Exception:
|
||||
return str(date_input)
|
||||
|
||||
|
||||
def format_duration_text(seconds_input: Optional[float | int]) -> str:
|
||||
"""
|
||||
Formats a duration in seconds to a human-readable text string (Stunden und Minuten).
|
||||
e.g., 9240 -> '2 Stunden 34 Minuten', 45 -> '1 Minute' (since 45s rounds to 1m), 0 -> '0 Minuten'.
|
||||
"""
|
||||
if seconds_input is None:
|
||||
return "N/A"
|
||||
|
||||
try:
|
||||
# Use round-half-up logic so 30 seconds correctly rounds up to 1 minute
|
||||
total_minutes = int(float(seconds_input) / 60 + 0.5)
|
||||
hours = total_minutes // 60
|
||||
minutes = total_minutes % 60
|
||||
|
||||
parts = []
|
||||
if hours > 0:
|
||||
parts.append(f"{hours} {'Stunde' if hours == 1 else 'Stunden'}")
|
||||
if minutes > 0 or not parts:
|
||||
parts.append(f"{minutes} {'Minute' if minutes == 1 else 'Minuten'}")
|
||||
|
||||
return " ".join(parts)
|
||||
except Exception:
|
||||
return str(seconds_input)
|
||||
|
||||
|
||||
def format_duration_table(seconds_input: Optional[float | int]) -> str:
|
||||
"""
|
||||
Formats a duration in seconds to a short table format (h:mm).
|
||||
e.g., 9240 -> '2:34', 2700 -> '0:45'.
|
||||
"""
|
||||
if seconds_input is None:
|
||||
return "N/A"
|
||||
|
||||
try:
|
||||
# Use round-half-up logic so 30 seconds correctly rounds up to 1 minute
|
||||
total_minutes = int(float(seconds_input) / 60 + 0.5)
|
||||
hours = total_minutes // 60
|
||||
minutes = total_minutes % 60
|
||||
|
||||
return f"{hours}:{minutes:02d}"
|
||||
except Exception:
|
||||
return str(seconds_input)
|
||||
|
||||
@@ -6,6 +6,8 @@ from strava_mcp_server.utils import (
|
||||
parse_iso_to_unix,
|
||||
format_date_iso,
|
||||
format_date_human,
|
||||
format_duration_text,
|
||||
format_duration_table,
|
||||
)
|
||||
|
||||
|
||||
@@ -96,3 +98,69 @@ class TestFormatDateHuman:
|
||||
import re
|
||||
|
||||
assert re.match(r"\d{2}\.\d{2}\.\d{4} \d{2}:\d{2}", result)
|
||||
|
||||
|
||||
class TestFormatDurationText:
|
||||
def test_none_returns_na(self):
|
||||
assert format_duration_text(None) == "N/A"
|
||||
|
||||
def test_zero_seconds(self):
|
||||
assert format_duration_text(0) == "0 Minuten"
|
||||
|
||||
def test_under_one_minute_rounds_to_zero_but_shows_zero(self):
|
||||
# 20 seconds rounds to 0 minutes
|
||||
assert format_duration_text(20) == "0 Minuten"
|
||||
|
||||
def test_half_minute_rounds_up(self):
|
||||
# 30 seconds rounds up to 1 minute
|
||||
assert format_duration_text(30) == "1 Minute"
|
||||
|
||||
def test_singular_minute(self):
|
||||
# 60 seconds = 1 minute
|
||||
assert format_duration_text(60) == "1 Minute"
|
||||
|
||||
def test_plural_minutes(self):
|
||||
# 120 seconds = 2 minutes
|
||||
assert format_duration_text(120) == "2 Minuten"
|
||||
|
||||
def test_singular_hour_and_plural_minutes(self):
|
||||
# 3600 + 120 = 3720 seconds = 1 hour 2 minutes
|
||||
assert format_duration_text(3720) == "1 Stunde 2 Minuten"
|
||||
|
||||
def test_plural_hours_and_singular_minute(self):
|
||||
# 7200 + 60 = 7260 seconds = 2 hours 1 minute
|
||||
assert format_duration_text(7260) == "2 Stunden 1 Minute"
|
||||
|
||||
def test_plural_hours_and_plural_minutes(self):
|
||||
# 9240 seconds = 2 hours 34 minutes
|
||||
assert format_duration_text(9240) == "2 Stunden 34 Minuten"
|
||||
|
||||
def test_only_hours_no_minutes(self):
|
||||
# 7200 seconds = 2 hours 0 minutes
|
||||
assert format_duration_text(7200) == "2 Stunden"
|
||||
|
||||
def test_fractional_input(self):
|
||||
assert format_duration_text(9240.4) == "2 Stunden 34 Minuten"
|
||||
|
||||
|
||||
class TestFormatDurationTable:
|
||||
def test_none_returns_na(self):
|
||||
assert format_duration_table(None) == "N/A"
|
||||
|
||||
def test_zero_seconds(self):
|
||||
assert format_duration_table(0) == "0:00"
|
||||
|
||||
def test_under_one_hour(self):
|
||||
# 2700 seconds = 45 minutes
|
||||
assert format_duration_table(2700) == "0:45"
|
||||
|
||||
def test_over_one_hour(self):
|
||||
# 9240 seconds = 2h 34m
|
||||
assert format_duration_table(9240) == "2:34"
|
||||
|
||||
def test_hours_with_leading_zero_minute(self):
|
||||
# 7260 seconds = 2h 1m -> '2:01'
|
||||
assert format_duration_table(7260) == "2:01"
|
||||
|
||||
def test_fractional_input(self):
|
||||
assert format_duration_table(2700.2) == "0:45"
|
||||
|
||||
Reference in New Issue
Block a user