sync status fixes

ckdake · ckdake · commit 51fa6b234a89 · 2026-03-18T01:15:08.000Z
diff --git a/TODO.md b/TODO.md
@@ -5,8 +5,7 @@ Tracked here for visibility. PRs and issues welcome.
 ## Overall capabilitites
 
 next:
-- [ ] bug: sync weirdness with current month, button updates timestamp but not activities, and month shows as synced when its not.
-- [ ] feat: trigger sync whenever new data is collected, for the month. use the Activity table as Source Of Truth for "matching", and ensure it stays update to date with a provider id for each activity. Update / page to show summary stats.
+- [ ] feat: trigger sync whenever new data is collected, for the month. use the Activity table as Source Of Truth for "matching", and ensure it stays update to date with a provider id for each activity. Update / page to show summary stats. Allow "link" when different device was used to record.
 - [ ] feat: only users with active subscription, or admin, can go back further than current month.  all other users: data expires out at start of new month.
 - [ ] feat: except admin, "full provider sync" is only allowed once per day.
 - [ ] data: 2024-02-29 19:20 garmin shows up on march 2024 review
diff --git a/app/routes/calendar.py b/app/routes/calendar.py
@@ -91,8 +91,10 @@ def sync_provider_month(year_month: str, provider_name: str):
                 409,
             )
 
+        from tracekit.provider_sync import ProviderSync, SyncStatus
         from tracekit.user_context import get_user_id
 
+        ProviderSync.upsert_status(year_month, provider_name, SyncStatus.ENQUEUED)
         task = pull_provider_month.delay(year_month, provider_name, user_id=get_user_id())
         set_pull_status(year_month, provider_name, PULL_STATUS_QUEUED, job_id=task.id)
         return jsonify(
diff --git a/app/routes/files.py b/app/routes/files.py
@@ -47,6 +47,9 @@ def _file_exists_in_folder(data_folder: str, filename: str) -> bool:
 def api_file_download():
     """Serve a file activity as a download, verifying user ownership."""
     raw_name = request.args.get("name", "").strip()
+    # Reject path traversal attempts before secure_filename can silently strip them
+    if ".." in raw_name or "/" in raw_name or "\\" in raw_name:
+        return "Invalid filename", 400
     # Normalize to a secure, base-name-only filename
     filename = secure_filename(raw_name)
     if not filename:
diff --git a/app/routes/intervalsicu_webhook.py b/app/routes/intervalsicu_webhook.py
@@ -185,4 +185,11 @@ def _sync_local_activity(activity_id, user_id: int) -> None:
         return
 
     provider = IntervalsICUProvider(config=icu_cfg)
-    provider.sync_single_activity(str(activity_id))
+    activity = provider.sync_single_activity(str(activity_id))
+    if activity and activity.start_time:
+        from datetime import UTC, datetime
+
+        from tracekit.provider_status import MONTH_SYNC_UNKNOWN, set_month_sync_status
+
+        year_month = datetime.fromtimestamp(activity.start_time, tz=UTC).strftime("%Y-%m")
+        set_month_sync_status(year_month, MONTH_SYNC_UNKNOWN)
diff --git a/app/routes/ridewithgps_webhook.py b/app/routes/ridewithgps_webhook.py
@@ -229,4 +229,11 @@ def _sync_local_trip(trip_id, user_id: int) -> None:
         return
 
     provider = RideWithGPSProvider(config=rwgps_cfg)
-    provider.sync_single_activity(str(trip_id))
+    activity = provider.sync_single_activity(str(trip_id))
+    if activity and activity.start_time:
+        from datetime import UTC, datetime
+
+        from tracekit.provider_status import MONTH_SYNC_UNKNOWN, set_month_sync_status
+
+        year_month = datetime.fromtimestamp(activity.start_time, tz=UTC).strftime("%Y-%m")
+        set_month_sync_status(year_month, MONTH_SYNC_UNKNOWN)
diff --git a/app/routes/strava_webhook.py b/app/routes/strava_webhook.py
@@ -225,7 +225,14 @@ def _sync_local_activity(activity_id, user_id: int):
         token_expires=strava_cfg.get("token_expires", "0"),
         config=strava_cfg,
     )
-    provider.sync_single_activity(str(activity_id))
+    activity = provider.sync_single_activity(str(activity_id))
+    if activity and activity.start_time:
+        from datetime import UTC, datetime
+
+        from tracekit.provider_status import MONTH_SYNC_UNKNOWN, set_month_sync_status
+
+        year_month = datetime.fromtimestamp(activity.start_time, tz=UTC).strftime("%Y-%m")
+        set_month_sync_status(year_month, MONTH_SYNC_UNKNOWN)
 
 
 def _handle_deauthorize(owner_id):
diff --git a/app/static/calendar.js b/app/static/calendar.js
@@ -409,7 +409,14 @@ document.addEventListener('DOMContentLoaded', async () => {
     try {
         const res  = await fetch('/api/calendar?from=' + from + '&to=' + to);
         const data = await res.json();
-        months.forEach(ym => renderGrid(ym, data[ym] || { error: 'No data' }));
+        months.forEach(ym => {
+            renderGrid(ym, data[ym] || { error: 'No data' });
+            const statuses = (data[ym] && data[ym].pull_statuses) || {};
+            const anyActive = Object.values(statuses).some(
+                s => s && (s.status === 'queued' || s.status === 'started')
+            );
+            if (anyActive) pollCard(ym, { minElapsed: 0 });
+        });
     } catch (e) {
         months.forEach(ym => {
             const grid = document.getElementById('grid-' + ym);
diff --git a/app/tests/test_files_routes.py b/app/tests/test_files_routes.py
@@ -132,21 +132,21 @@ def test_file_missing_on_disk_returns_404(self, client):
         assert resp.status_code == 404
 
     def test_valid_file_returns_200_and_attachment(self, client):
-        c, _ = client
-        with tempfile.NamedTemporaryFile(suffix=".fit", delete=False) as f:
-            f.write(b"FIT file content")
-            tmp_path = f.name
-        try:
+        c, user_id = client
+        with tempfile.TemporaryDirectory() as data_dir:
+            user_dir = os.path.join(data_dir, "activities", str(user_id))
+            os.makedirs(user_dir)
+            tmp_path = os.path.join(user_dir, "activity.fit")
+            with open(tmp_path, "wb") as f:
+                f.write(b"FIT file content")
             with (
                 patch("routes.files.FileActivity") as mock_fa,
-                patch("routes.files._glob.glob", return_value=[tmp_path]),
+                patch("routes.files.os.environ.get", return_value=data_dir),
             ):
                 mock_fa.get_or_none.return_value = MagicMock()
                 resp = c.get("/api/file/download?name=activity.fit")
             assert resp.status_code == 200
             assert "attachment" in resp.headers.get("Content-Disposition", "")
-        finally:
-            os.remove(tmp_path)
 
     def test_ownership_check_uses_user_id(self, client):
         """get_or_none must be called with the logged-in user's ID."""
diff --git a/tests/providers/test_garmin.py b/tests/providers/test_garmin.py
@@ -3,6 +3,7 @@
 import pytest
 from garminconnect import GarminConnectConnectionError
 
+from tracekit.provider_sync import SyncStatus
 from tracekit.providers.garmin.garmin_provider import GarminProvider
 
 
@@ -114,9 +115,7 @@ def test_pull_activities_already_synced(self, mock_provider_sync):
         """Test pull_activities when month is already synced."""
         provider = GarminProvider()
 
-        # Mock existing sync record
-        mock_sync = Mock()
-        mock_provider_sync.get_or_none.return_value = mock_sync
+        mock_provider_sync.is_done.return_value = True
 
         # Mock the _get_garmin_activities_for_month method
         mock_activities = [Mock(), Mock()]
@@ -126,7 +125,7 @@ def test_pull_activities_already_synced(self, mock_provider_sync):
 
         # Should return activities from database without fetching new ones
         assert result == mock_activities
-        mock_provider_sync.get_or_none.assert_called_once_with("2021-01", "garmin")
+        mock_provider_sync.is_done.assert_called_once_with("2021-01", "garmin")
         provider._get_garmin_activities_for_month.assert_called_once_with("2021-01")
 
     @pytest.mark.skip(reason="Complex mocking of GarminActivity instantiation in real execution flow")
@@ -178,7 +177,7 @@ def test_pull_activities_fetches_gear(self, mock_provider_sync, mock_garmin_acti
         """Test that pull_activities calls get_activity_gear and sets equipment."""
         provider = GarminProvider()
 
-        mock_provider_sync.get_or_none.return_value = None
+        mock_provider_sync.is_done.return_value = False
 
         raw_activity = {
             "activityId": 12345,
@@ -210,7 +209,7 @@ def test_pull_activities_gear_api_error_does_not_abort(self, mock_provider_sync,
         """Test that a gear API error does not prevent the activity from being saved."""
         provider = GarminProvider()
 
-        mock_provider_sync.get_or_none.return_value = None
+        mock_provider_sync.is_done.return_value = False
 
         raw_activity = {"activityId": 12345, "activityName": "Morning Ride"}
         provider.fetch_activities_for_month = Mock(return_value=[raw_activity])
@@ -237,7 +236,7 @@ def test_pull_activities_with_duplicate_activity(self, mock_provider_sync):
         provider = GarminProvider()
 
         # Mock no existing sync record
-        mock_provider_sync.get_or_none.return_value = None
+        mock_provider_sync.is_done.return_value = False
 
         # Mock fetching raw activities
         mock_raw_activity = {"activityId": 12345, "activityName": "Test Activity"}
@@ -261,7 +260,7 @@ def test_pull_activities_with_duplicate_activity(self, mock_provider_sync):
 
                 # Verify duplicate was skipped (save not called)
                 mock_activity.save.assert_not_called()
-                mock_provider_sync.create.assert_called_once_with(year_month="2021-01", provider="garmin", user_id=0)
+                mock_provider_sync.upsert_status.assert_called_once_with("2021-01", "garmin", SyncStatus.DONE)
 
 
 class TestGarminProviderFetchActivities:
diff --git a/tests/providers/test_strava.py b/tests/providers/test_strava.py
@@ -3,6 +3,7 @@
 
 import pytest
 
+from tracekit.provider_sync import SyncStatus
 from tracekit.providers.strava.strava_provider import StravaProvider
 
 
@@ -437,8 +438,7 @@ def test_pull_activities_already_synced(self, mock_provider_sync):
         provider = StravaProvider(token="test_token", refresh_token="test_refresh", token_expires="999999999")
 
         # Mock existing sync record
-        mock_sync = Mock()
-        mock_provider_sync.get_or_none.return_value = mock_sync
+        mock_provider_sync.is_done.return_value = True
 
         # Mock the _get_strava_activities_for_month method
         mock_activities = [Mock(), Mock()]
@@ -448,7 +448,7 @@ def test_pull_activities_already_synced(self, mock_provider_sync):
 
         # Should return activities from database without fetching new ones
         assert result == mock_activities
-        mock_provider_sync.get_or_none.assert_called_once_with("2021-01", "strava")
+        mock_provider_sync.is_done.assert_called_once_with("2021-01", "strava")
         provider._get_strava_activities_for_month.assert_called_once_with("2021-01")
 
     @patch("tracekit.providers.strava.strava_provider.ProviderSync")
@@ -457,7 +457,7 @@ def test_pull_activities_new_month(self, mock_provider_sync):
         provider = StravaProvider(token="test_token", refresh_token="test_refresh", token_expires="999999999")
 
         # Mock no existing sync record
-        mock_provider_sync.get_or_none.return_value = None
+        mock_provider_sync.is_done.return_value = False
 
         # Mock fetching raw activities
         mock_raw_activities = [Mock(), Mock()]
@@ -483,17 +483,17 @@ def test_pull_activities_new_month(self, mock_provider_sync):
 
             # Verify the flow
             assert result == mock_final_activities
-            mock_provider_sync.get_or_none.assert_called_once_with("2021-01", "strava")
+            mock_provider_sync.is_done.assert_called_once_with("2021-01", "strava")
             provider._fetch_strava_activities_for_month.assert_called_once_with("2021-01")
-            mock_provider_sync.create.assert_called_once_with(year_month="2021-01", provider="strava", user_id=0)
+            mock_provider_sync.upsert_status.assert_called_once_with("2021-01", "strava", SyncStatus.DONE)
 
     @patch("tracekit.providers.strava.strava_provider.ProviderSync")
     def test_pull_activities_with_duplicate_activity(self, mock_provider_sync):
         """Test pull_activities when encountering duplicate activity."""
         provider = StravaProvider(token="test_token", refresh_token="test_refresh", token_expires="999999999")
 
         # Mock no existing sync record
-        mock_provider_sync.get_or_none.return_value = None
+        mock_provider_sync.is_done.return_value = False
 
         # Mock fetching raw activities
         mock_raw_activity = Mock()
@@ -517,15 +517,15 @@ def test_pull_activities_with_duplicate_activity(self, mock_provider_sync):
 
             # Verify duplicate was skipped (save not called)
             mock_converted_activity.save.assert_not_called()
-            mock_provider_sync.create.assert_called_once_with(year_month="2021-01", provider="strava", user_id=0)
+            mock_provider_sync.upsert_status.assert_called_once_with("2021-01", "strava", SyncStatus.DONE)
 
     @patch("tracekit.providers.strava.strava_provider.ProviderSync")
     def test_pull_activities_with_error(self, mock_provider_sync):
         """Test pull_activities when processing error occurs."""
         provider = StravaProvider(token="test_token", refresh_token="test_refresh", token_expires="999999999")
 
         # Mock no existing sync record
-        mock_provider_sync.get_or_none.return_value = None
+        mock_provider_sync.is_done.return_value = False
 
         # Mock fetching raw activities
         mock_raw_activity = Mock()
@@ -542,7 +542,7 @@ def test_pull_activities_with_error(self, mock_provider_sync):
 
         # Should handle error gracefully and still mark as synced
         assert result == mock_final_activities
-        mock_provider_sync.create.assert_called_once_with(year_month="2021-01", provider="strava", user_id=0)
+        mock_provider_sync.upsert_status.assert_called_once_with("2021-01", "strava", SyncStatus.DONE)
 
     @patch("tracekit.providers.strava.strava_provider.ProviderSync")
     def test_pull_activities_rate_limit_propagates(self, mock_provider_sync):
@@ -554,7 +554,7 @@ def test_pull_activities_rate_limit_propagates(self, mock_provider_sync):
 
         provider = StravaProvider(token="test_token", refresh_token="test_refresh", token_expires="999999999")
 
-        mock_provider_sync.get_or_none.return_value = None
+        mock_provider_sync.is_done.return_value = False
 
         mock_raw_activity = Mock()
         provider._fetch_strava_activities_for_month = Mock(return_value=[mock_raw_activity])
@@ -574,7 +574,7 @@ def test_pull_activities_rate_limit_propagates(self, mock_provider_sync):
             provider.pull_activities(date_filter="2021-01")
 
         # The month must NOT be marked as synced when a rate-limit aborts the pull.
-        mock_provider_sync.create.assert_not_called()
+        mock_provider_sync.upsert_status.assert_not_called()
 
 
 class TestStravaProviderCreateActivity:
diff --git a/tracekit/calendar.py b/tracekit/calendar.py
@@ -112,7 +112,7 @@ def get_single_month_data(year_month: str, home_timezone: str = "UTC") -> dict[s
     import pytz
 
     from tracekit.provider_status import get_month_pull_statuses, get_month_sync_status
-    from tracekit.provider_sync import ProviderSync
+    from tracekit.provider_sync import ProviderSync, SyncStatus
     from tracekit.providers.file.file_activity import FileActivity
     from tracekit.providers.garmin.garmin_activity import GarminActivity
     from tracekit.providers.intervalsicu.intervalsicu_activity import (
@@ -127,11 +127,32 @@ def get_single_month_data(year_month: str, home_timezone: str = "UTC") -> dict[s
     month_sync_status = get_month_sync_status(year_month)
 
     uid = get_user_id()
+
+    # Only fully-synced months count as "synced"
     synced_rows = ProviderSync.select(ProviderSync.provider).where(
-        (ProviderSync.year_month == year_month) & (ProviderSync.user_id == uid)
+        (ProviderSync.year_month == year_month)
+        & (ProviderSync.user_id == uid)
+        & (ProviderSync.status == SyncStatus.DONE)
     )
     synced_providers = [r.provider for r in synced_rows]
 
+    # Providers currently in-flight: synthesize pull_statuses entries so the UI
+    # renders spinners and starts polling even after a page reload.
+    in_flight_rows = ProviderSync.select().where(
+        (ProviderSync.year_month == year_month)
+        & (ProviderSync.user_id == uid)
+        & (ProviderSync.status != SyncStatus.DONE)
+    )
+    for row in in_flight_rows:
+        existing = pull_statuses.get(row.provider)
+        if not existing or existing.get("status") not in ("queued", "started"):
+            pull_statuses[row.provider] = {
+                "status": "queued" if row.status == SyncStatus.ENQUEUED else "started",
+                "job_id": None,
+                "message": None,
+                "updated_at": None,
+            }
+
     all_rows = ProviderSync.select(ProviderSync.provider).where(ProviderSync.user_id == uid).distinct()
     providers = sorted({r.provider for r in all_rows})
 
diff --git a/tracekit/core.py b/tracekit/core.py
@@ -256,7 +256,7 @@ def pull_provider_activities(self, year_month: str, provider_name: str) -> list[
         try:
             from tracekit.provider_sync import ProviderSync as _ProviderSync
 
-            _was_cached = _ProviderSync.get_or_none(year_month, provider_name) is not None
+            _was_cached = _ProviderSync.is_done(year_month, provider_name)
         except Exception:
             _was_cached = False
 
diff --git a/tracekit/database.py b/tracekit/database.py
@@ -226,6 +226,7 @@ def _run_schema_upgrades() -> None:
             ("user", "stripe_subscription_status", "VARCHAR(64)"),
             ("user", "stripe_subscription_end", "INTEGER"),
             ("user", "allow_impersonation", "BOOLEAN NOT NULL DEFAULT FALSE"),
+            ("providersync", "status", "VARCHAR(16) NOT NULL DEFAULT 'done'"),
         ],
     )
 
diff --git a/tracekit/provider_sync.py b/tracekit/provider_sync.py
@@ -1,15 +1,24 @@
+from enum import StrEnum
+
 from peewee import CharField, IntegerField, Model
 
 from tracekit.db import db
 from tracekit.user_context import get_user_id
 
 
+class SyncStatus(StrEnum):
+    ENQUEUED = "enqueued"
+    STARTED = "started"
+    DONE = "done"
+
+
 class ProviderSync(Model):
     """Tracks which months have been synced for each provider."""
 
     year_month = CharField()  # Format: YYYY-MM
     provider = CharField()  # e.g., 'strava', 'spreadsheet', 'ridewithgps'
     user_id = IntegerField(default=0)
+    status = CharField(default=SyncStatus.DONE)  # enqueued | started | done
 
     class Meta:
         database = db
@@ -23,3 +32,24 @@ def get_or_none(cls, year_month: str, provider: str) -> "ProviderSync":
             .where((cls.year_month == year_month) & (cls.provider == provider) & (cls.user_id == get_user_id()))
             .first()
         )
+
+    @classmethod
+    def is_done(cls, year_month: str, provider: str) -> bool:
+        """Return True if this month has been fully synced (status=done)."""
+        return (
+            cls.select()
+            .where(
+                (cls.year_month == year_month)
+                & (cls.provider == provider)
+                & (cls.user_id == get_user_id())
+                & (cls.status == SyncStatus.DONE)
+            )
+            .exists()
+        )
+
+    @classmethod
+    def upsert_status(cls, year_month: str, provider: str, status: SyncStatus) -> None:
+        """Create or replace the sync record with the given status."""
+        uid = get_user_id()
+        cls.delete().where((cls.year_month == year_month) & (cls.provider == provider) & (cls.user_id == uid)).execute()
+        cls.create(year_month=year_month, provider=provider, user_id=uid, status=status)
diff --git a/tracekit/providers/file/file_provider.py b/tracekit/providers/file/file_provider.py
diff --git a/tracekit/providers/garmin/garmin_provider.py b/tracekit/providers/garmin/garmin_provider.py
diff --git a/tracekit/providers/intervalsicu/intervalsicu_provider.py b/tracekit/providers/intervalsicu/intervalsicu_provider.py
diff --git a/tracekit/providers/ridewithgps/ridewithgps_provider.py b/tracekit/providers/ridewithgps/ridewithgps_provider.py
diff --git a/tracekit/providers/spreadsheet/spreadsheet_provider.py b/tracekit/providers/spreadsheet/spreadsheet_provider.py
diff --git a/tracekit/providers/strava/strava_provider.py b/tracekit/providers/strava/strava_provider.py
diff --git a/tracekit/worker.py b/tracekit/worker.py

Original file line number	Diff line number	Diff line change
`@@ -91,8 +91,10 @@ def sync_provider_month(year_month: str, provider_name: str):`
`91`	`91`	`409,`
`92`	`92`	`)`
`93`	`93`
	`94`	`+ from tracekit.provider_sync import ProviderSync, SyncStatus`
`94`	`95`	`from tracekit.user_context import get_user_id`
`95`	`96`
	`97`	`+ ProviderSync.upsert_status(year_month, provider_name, SyncStatus.ENQUEUED)`
`96`	`98`	`task = pull_provider_month.delay(year_month, provider_name, user_id=get_user_id())`
`97`	`99`	`set_pull_status(year_month, provider_name, PULL_STATUS_QUEUED, job_id=task.id)`
`98`	`100`	`return jsonify(`