Fix wrongly calculated dashboards
This commit is contained in:
parent
f70c95b3bb
commit
e78a03b41b
6 changed files with 67 additions and 26 deletions
|
|
@ -208,15 +208,22 @@ public interface ActivityRepository extends JpaRepository<Activity, UUID> {
|
||||||
boolean existsByUserIdAndDate(@Param("userId") UUID userId, @Param("date") java.time.LocalDate date);
|
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,
|
* Returns the start timestamps of all activities for a user since the given
|
||||||
* since the given timestamp, ordered most-recent first. Used by the streak
|
* cutoff, ordered most-recent first. Used by the streak calculation in
|
||||||
* calculation in {@code AchievementService} to walk activity history with a single
|
* {@code AchievementService} to walk activity history with a single query
|
||||||
* query instead of one {@code existsByUserIdAndDate} query per day.
|
* instead of one {@code existsByUserIdAndDate} query per day.
|
||||||
|
*
|
||||||
|
* <p>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<LocalDate>} (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 " +
|
"WHERE a.userId = :userId AND a.startedAt >= :since " +
|
||||||
"ORDER BY cast(a.startedAt as date) DESC")
|
"ORDER BY a.startedAt DESC")
|
||||||
List<java.time.LocalDate> findDistinctActivityDatesSince(
|
List<java.time.LocalDateTime> findActivityStartTimestampsSince(
|
||||||
@Param("userId") UUID userId,
|
@Param("userId") UUID userId,
|
||||||
@Param("since") java.time.LocalDateTime since
|
@Param("since") java.time.LocalDateTime since
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -403,11 +403,16 @@ public class AchievementService {
|
||||||
/**
|
/**
|
||||||
* Calculate current activity streak (consecutive days).
|
* Calculate current activity streak (consecutive days).
|
||||||
*
|
*
|
||||||
* <p>Loads all distinct activity dates for the user in the last 366 days in a
|
* <p>Loads all activity timestamps for the user in the last 366 days in a single
|
||||||
* single query and walks the resulting set in memory. Previously this method
|
* query, deduplicates them to a {@code Set<LocalDate>} in Java, and walks the
|
||||||
* issued one {@code SELECT EXISTS} query per day (up to 365 round-trips per
|
* resulting set in memory. Previously this method issued one {@code SELECT EXISTS}
|
||||||
* activity upload), which was the single biggest performance hot spot in the
|
* query per day (up to 365 round-trips per activity upload), which was the single
|
||||||
* achievement evaluation path.
|
* biggest performance hot spot in the achievement evaluation path.
|
||||||
|
*
|
||||||
|
* <p>Java-side date deduplication is intentional: Hibernate 6 + Spring Data 3 do
|
||||||
|
* not reliably convert SQL date scalar projections to {@code List<LocalDate>}.
|
||||||
|
* The result set is small (a few hundred timestamps at most) so the cost of
|
||||||
|
* Java-side distinct is negligible.
|
||||||
*
|
*
|
||||||
* <p>The streak / rest-day logic is preserved bug-for-bug from the previous
|
* <p>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
|
* 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
|
// 366 to safely cover the lookback window even if today's activity is in the
|
||||||
// future relative to the cutoff (timezone edge cases).
|
// future relative to the cutoff (timezone edge cases).
|
||||||
LocalDateTime since = today.minusDays(366).atStartOfDay();
|
LocalDateTime since = today.minusDays(366).atStartOfDay();
|
||||||
Set<LocalDate> activityDates = new HashSet<>(
|
Set<LocalDate> activityDates = new HashSet<>();
|
||||||
activityRepository.findDistinctActivityDatesSince(userId, since)
|
for (LocalDateTime ts : activityRepository.findActivityStartTimestampsSince(userId, since)) {
|
||||||
);
|
activityDates.add(ts.toLocalDate());
|
||||||
|
}
|
||||||
|
|
||||||
if (activityDates.isEmpty()) {
|
if (activityDates.isEmpty()) {
|
||||||
return 0;
|
return 0;
|
||||||
|
|
|
||||||
|
|
@ -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) {
|
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(
|
PersonalRecord maxSpeedRecord = checkRecord(
|
||||||
activity.getUserId(),
|
activity.getUserId(),
|
||||||
activityType,
|
activityType,
|
||||||
PersonalRecord.RecordType.MAX_SPEED,
|
PersonalRecord.RecordType.MAX_SPEED,
|
||||||
activity.getMetrics().getMaxSpeed(),
|
maxSpeedMps,
|
||||||
"mps",
|
"mps",
|
||||||
activity.getId(),
|
activity.getId(),
|
||||||
activity.getStartedAt()
|
activity.getStartedAt()
|
||||||
|
|
|
||||||
|
|
@ -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';
|
||||||
|
|
@ -305,7 +305,10 @@
|
||||||
}
|
}
|
||||||
|
|
||||||
function formatPRValue(type, value, unit) {
|
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 hours = Math.floor(value / 3600);
|
||||||
const minutes = Math.floor((value % 3600) / 60);
|
const minutes = Math.floor((value % 3600) / 60);
|
||||||
const seconds = Math.floor(value % 60);
|
const seconds = Math.floor(value % 60);
|
||||||
|
|
|
||||||
|
|
@ -59,9 +59,9 @@ class AchievementServiceTest {
|
||||||
when(activityRepository.countByUserIdAndActivityType(userId, Activity.ActivityType.RUN)).thenReturn(1L);
|
when(activityRepository.countByUserIdAndActivityType(userId, Activity.ActivityType.RUN)).thenReturn(1L);
|
||||||
when(activityRepository.sumDistanceByUserId(userId)).thenReturn(BigDecimal.valueOf(5000));
|
when(activityRepository.sumDistanceByUserId(userId)).thenReturn(BigDecimal.valueOf(5000));
|
||||||
when(activityRepository.countDistinctActivityTypesByUserId(userId)).thenReturn(1L);
|
when(activityRepository.countDistinctActivityTypesByUserId(userId)).thenReturn(1L);
|
||||||
// Streak source: today has activity (1-day streak — not enough to trigger any streak achievement)
|
// Streak source: today has one activity (1-day streak — not enough to trigger any streak achievement)
|
||||||
lenient().when(activityRepository.findDistinctActivityDatesSince(any(), any()))
|
lenient().when(activityRepository.findActivityStartTimestampsSince(any(), any()))
|
||||||
.thenReturn(List.of(java.time.LocalDate.now()));
|
.thenReturn(List.of(java.time.LocalDateTime.now()));
|
||||||
when(achievementRepository.save(any(Achievement.class))).thenAnswer(invocation -> invocation.getArgument(0));
|
when(achievementRepository.save(any(Achievement.class))).thenAnswer(invocation -> invocation.getArgument(0));
|
||||||
|
|
||||||
// When
|
// When
|
||||||
|
|
@ -292,11 +292,11 @@ class AchievementServiceTest {
|
||||||
when(activityRepository.countByUserIdAndActivityType(any(), any())).thenReturn(10L);
|
when(activityRepository.countByUserIdAndActivityType(any(), any())).thenReturn(10L);
|
||||||
when(activityRepository.sumDistanceByUserId(userId)).thenReturn(BigDecimal.valueOf(100000));
|
when(activityRepository.sumDistanceByUserId(userId)).thenReturn(BigDecimal.valueOf(100000));
|
||||||
when(activityRepository.countDistinctActivityTypesByUserId(userId)).thenReturn(1L);
|
when(activityRepository.countDistinctActivityTypesByUserId(userId)).thenReturn(1L);
|
||||||
// Streak source: 8 consecutive days of activity ending today, ordered most-recent first
|
// Streak source: 8 consecutive days of activity ending today, as raw timestamps
|
||||||
java.time.LocalDate today = java.time.LocalDate.now();
|
java.time.LocalDateTime now = java.time.LocalDateTime.now();
|
||||||
when(activityRepository.findDistinctActivityDatesSince(any(), any())).thenReturn(List.of(
|
when(activityRepository.findActivityStartTimestampsSince(any(), any())).thenReturn(List.of(
|
||||||
today, today.minusDays(1), today.minusDays(2), today.minusDays(3),
|
now, now.minusDays(1), now.minusDays(2), now.minusDays(3),
|
||||||
today.minusDays(4), today.minusDays(5), today.minusDays(6), today.minusDays(7)
|
now.minusDays(4), now.minusDays(5), now.minusDays(6), now.minusDays(7)
|
||||||
));
|
));
|
||||||
when(achievementRepository.save(any(Achievement.class))).thenAnswer(invocation -> invocation.getArgument(0));
|
when(achievementRepository.save(any(Achievement.class))).thenAnswer(invocation -> invocation.getArgument(0));
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue