From e78a03b41b4cc280f414d5e3149863f182ff34bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Z=C3=B6ller?= Date: Wed, 8 Apr 2026 00:03:10 +0200 Subject: [PATCH] Fix wrongly calculated dashboards --- .../fitpub/repository/ActivityRepository.java | 21 ++++++++++++------ .../fitpub/service/AchievementService.java | 22 ++++++++++++------- .../fitpub/service/PersonalRecordService.java | 12 ++++++++-- ...0__fix_max_speed_personal_record_units.sql | 17 ++++++++++++++ .../templates/analytics/dashboard.html | 5 ++++- .../service/AchievementServiceTest.java | 16 +++++++------- 6 files changed, 67 insertions(+), 26 deletions(-) create mode 100644 src/main/resources/db/migration/V30__fix_max_speed_personal_record_units.sql diff --git a/src/main/java/net/javahippie/fitpub/repository/ActivityRepository.java b/src/main/java/net/javahippie/fitpub/repository/ActivityRepository.java index 3f6a1bc..55483a2 100644 --- a/src/main/java/net/javahippie/fitpub/repository/ActivityRepository.java +++ b/src/main/java/net/javahippie/fitpub/repository/ActivityRepository.java @@ -208,15 +208,22 @@ public interface ActivityRepository extends JpaRepository { boolean existsByUserIdAndDate(@Param("userId") UUID userId, @Param("date") java.time.LocalDate date); /** - * Returns the distinct calendar dates on which a user has at least one activity, - * since the given timestamp, ordered most-recent first. Used by the streak - * calculation in {@code AchievementService} to walk activity history with a single - * query instead of one {@code existsByUserIdAndDate} query per day. + * Returns the start timestamps of all activities for a user since the given + * cutoff, ordered most-recent first. Used by the streak calculation in + * {@code AchievementService} to walk activity history with a single query + * instead of one {@code existsByUserIdAndDate} query per day. + * + *

Returns raw {@code LocalDateTime} values rather than distinct {@code LocalDate}s + * because Hibernate 6 + Spring Data 3 do not reliably convert SQL date scalar + * projections to {@code List} (the result comes back as + * {@code ArrayList} and the per-element conversion fails). The caller is + * responsible for deduplicating to dates in Java — the result set is bounded + * (max ~366 days × small per-day activity count) so Java-side distinct is cheap. */ - @Query("SELECT DISTINCT cast(a.startedAt as date) FROM Activity a " + + @Query("SELECT a.startedAt FROM Activity a " + "WHERE a.userId = :userId AND a.startedAt >= :since " + - "ORDER BY cast(a.startedAt as date) DESC") - List findDistinctActivityDatesSince( + "ORDER BY a.startedAt DESC") + List findActivityStartTimestampsSince( @Param("userId") UUID userId, @Param("since") java.time.LocalDateTime since ); diff --git a/src/main/java/net/javahippie/fitpub/service/AchievementService.java b/src/main/java/net/javahippie/fitpub/service/AchievementService.java index c62ee2a..df38231 100644 --- a/src/main/java/net/javahippie/fitpub/service/AchievementService.java +++ b/src/main/java/net/javahippie/fitpub/service/AchievementService.java @@ -403,11 +403,16 @@ public class AchievementService { /** * Calculate current activity streak (consecutive days). * - *

Loads all distinct activity dates for the user in the last 366 days in a - * single query and walks the resulting set in memory. Previously this method - * issued one {@code SELECT EXISTS} query per day (up to 365 round-trips per - * activity upload), which was the single biggest performance hot spot in the - * achievement evaluation path. + *

Loads all activity timestamps for the user in the last 366 days in a single + * query, deduplicates them to a {@code Set} in Java, and walks the + * resulting set in memory. Previously this method issued one {@code SELECT EXISTS} + * query per day (up to 365 round-trips per activity upload), which was the single + * biggest performance hot spot in the achievement evaluation path. + * + *

Java-side date deduplication is intentional: Hibernate 6 + Spring Data 3 do + * not reliably convert SQL date scalar projections to {@code List}. + * The result set is small (a few hundred timestamps at most) so the cost of + * Java-side distinct is negligible. * *

The streak / rest-day logic is preserved bug-for-bug from the previous * implementation: a missing day after a streak has started is silently skipped @@ -419,9 +424,10 @@ public class AchievementService { // 366 to safely cover the lookback window even if today's activity is in the // future relative to the cutoff (timezone edge cases). LocalDateTime since = today.minusDays(366).atStartOfDay(); - Set activityDates = new HashSet<>( - activityRepository.findDistinctActivityDatesSince(userId, since) - ); + Set activityDates = new HashSet<>(); + for (LocalDateTime ts : activityRepository.findActivityStartTimestampsSince(userId, since)) { + activityDates.add(ts.toLocalDate()); + } if (activityDates.isEmpty()) { return 0; diff --git a/src/main/java/net/javahippie/fitpub/service/PersonalRecordService.java b/src/main/java/net/javahippie/fitpub/service/PersonalRecordService.java index 67ba8d1..9485944 100644 --- a/src/main/java/net/javahippie/fitpub/service/PersonalRecordService.java +++ b/src/main/java/net/javahippie/fitpub/service/PersonalRecordService.java @@ -95,13 +95,21 @@ public class PersonalRecordService { } } - // Check max speed (from metrics) + // Check max speed (from metrics). + // + // ActivityMetrics.maxSpeed is already in km/h (FitParser/GpxParser convert from m/s + // before persisting). The personal_records.unit column says "mps" though, so the + // display side multiplies by 3.6 to "convert to km/h" — which produced values 3.6× + // too high. Convert to true m/s here so the stored value matches its labelled unit. + // Existing wrongly-labelled rows are corrected by V30 in the same change. if (activity.getMetrics() != null && activity.getMetrics().getMaxSpeed() != null) { + BigDecimal maxSpeedMps = activity.getMetrics().getMaxSpeed() + .divide(BigDecimal.valueOf(3.6), 2, RoundingMode.HALF_UP); PersonalRecord maxSpeedRecord = checkRecord( activity.getUserId(), activityType, PersonalRecord.RecordType.MAX_SPEED, - activity.getMetrics().getMaxSpeed(), + maxSpeedMps, "mps", activity.getId(), activity.getStartedAt() diff --git a/src/main/resources/db/migration/V30__fix_max_speed_personal_record_units.sql b/src/main/resources/db/migration/V30__fix_max_speed_personal_record_units.sql new file mode 100644 index 0000000..312310b --- /dev/null +++ b/src/main/resources/db/migration/V30__fix_max_speed_personal_record_units.sql @@ -0,0 +1,17 @@ +-- Migration V30: Fix max_speed personal records that were stored in km/h but +-- labelled as m/s. +-- +-- PersonalRecordService used to copy ActivityMetrics.maxSpeed (already in km/h +-- per the parser) into the personal_records.value column with unit = "mps". +-- The display layer then multiplied by 3.6 to "convert m/s to km/h", producing +-- values 3.6× the real speed (a 33 km/h ride was shown as 120 km/h). +-- +-- The application code is fixed in PersonalRecordService to actually divide by +-- 3.6 before storing. This migration brings existing rows in line with that +-- contract: any MAX_SPEED row currently stored in km/h is divided by 3.6 to +-- become a real m/s value. After this, value × 3.6 always yields the correct +-- km/h display. + +UPDATE personal_records +SET value = value / 3.6 +WHERE record_type = 'MAX_SPEED' AND unit = 'mps'; diff --git a/src/main/resources/templates/analytics/dashboard.html b/src/main/resources/templates/analytics/dashboard.html index a26f756..0f09a32 100644 --- a/src/main/resources/templates/analytics/dashboard.html +++ b/src/main/resources/templates/analytics/dashboard.html @@ -305,7 +305,10 @@ } function formatPRValue(type, value, unit) { - if (unit === 'seconds') { + // 'seconds' (race finish times) and 'seconds_per_km' (pace) both render + // as h:mm:ss / mm:ss. Previously the dashboard only handled 'seconds', + // so pace records fell through and showed as raw integer seconds. + if (unit === 'seconds' || unit === 'seconds_per_km') { const hours = Math.floor(value / 3600); const minutes = Math.floor((value % 3600) / 60); const seconds = Math.floor(value % 60); diff --git a/src/test/java/net/javahippie/fitpub/service/AchievementServiceTest.java b/src/test/java/net/javahippie/fitpub/service/AchievementServiceTest.java index 2f8a20e..e9cd220 100644 --- a/src/test/java/net/javahippie/fitpub/service/AchievementServiceTest.java +++ b/src/test/java/net/javahippie/fitpub/service/AchievementServiceTest.java @@ -59,9 +59,9 @@ class AchievementServiceTest { when(activityRepository.countByUserIdAndActivityType(userId, Activity.ActivityType.RUN)).thenReturn(1L); when(activityRepository.sumDistanceByUserId(userId)).thenReturn(BigDecimal.valueOf(5000)); when(activityRepository.countDistinctActivityTypesByUserId(userId)).thenReturn(1L); - // Streak source: today has activity (1-day streak — not enough to trigger any streak achievement) - lenient().when(activityRepository.findDistinctActivityDatesSince(any(), any())) - .thenReturn(List.of(java.time.LocalDate.now())); + // Streak source: today has one activity (1-day streak — not enough to trigger any streak achievement) + lenient().when(activityRepository.findActivityStartTimestampsSince(any(), any())) + .thenReturn(List.of(java.time.LocalDateTime.now())); when(achievementRepository.save(any(Achievement.class))).thenAnswer(invocation -> invocation.getArgument(0)); // When @@ -292,11 +292,11 @@ class AchievementServiceTest { when(activityRepository.countByUserIdAndActivityType(any(), any())).thenReturn(10L); when(activityRepository.sumDistanceByUserId(userId)).thenReturn(BigDecimal.valueOf(100000)); when(activityRepository.countDistinctActivityTypesByUserId(userId)).thenReturn(1L); - // Streak source: 8 consecutive days of activity ending today, ordered most-recent first - java.time.LocalDate today = java.time.LocalDate.now(); - when(activityRepository.findDistinctActivityDatesSince(any(), any())).thenReturn(List.of( - today, today.minusDays(1), today.minusDays(2), today.minusDays(3), - today.minusDays(4), today.minusDays(5), today.minusDays(6), today.minusDays(7) + // Streak source: 8 consecutive days of activity ending today, as raw timestamps + java.time.LocalDateTime now = java.time.LocalDateTime.now(); + when(activityRepository.findActivityStartTimestampsSince(any(), any())).thenReturn(List.of( + now, now.minusDays(1), now.minusDays(2), now.minusDays(3), + now.minusDays(4), now.minusDays(5), now.minusDays(6), now.minusDays(7) )); when(achievementRepository.save(any(Achievement.class))).thenAnswer(invocation -> invocation.getArgument(0));