mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
fix: observe published workouts instead of scanning, and translate durations
Two review comments on #4129. **The dashboard now observes the cache.** publishedWorkoutsOf walked every note in LocalCache on a suspend call, so it was both O(cache) per refresh and a snapshot: a workout arriving from a relay — or the one the user had just posted — only appeared on the next resume. It is now LocalCache.observeEvents(Filter(kinds = [1301], authors = [me])), which is indexed by kind and author, so it neither walks the cache nor needs a timer, and the screen updates itself. That turned the ViewModel inside out for the better. Health Connect stays a push source — the platform has no change feed to observe, so refresh() still re-reads it on resume and on a permission grant — but the published side is a flow, and state is now a combine() of the two, keyed on the account via flatMapLatest so an account switch re-subscribes. Loading is expressed as "the Health Connect status is not resolved yet" rather than a separate flag. The carousel takes .first() from the same flow: it reloads on resume anyway, and one definition of "what have I already posted" is the point — two would let it offer something the dashboard knows is a duplicate. This does not close the REQ gap documented in the PR: observing the cache still only sees what something else fetched. It does mean that once the REQ exists, the dashboard will pick its results up without further work. **Durations are translatable.** formatDuration hardcoded "7h 12m" / "42m" / "45s". The suffixes are English abbreviations and the order of the two parts is not universal either, so both move into string resources and the function becomes @Composable. The zero case reuses the minutes form rather than a second literal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019egdJyBHnrATZHjs86up8f
This commit is contained in:
+14
-18
@@ -24,30 +24,26 @@ import com.vitorpamplona.amethyst.commons.fitness.DetectedWorkout
|
||||
import com.vitorpamplona.amethyst.commons.fitness.toDetectedWorkout
|
||||
import com.vitorpamplona.amethyst.model.LocalCache
|
||||
import com.vitorpamplona.quartz.experimental.fitness.workout.WorkoutRecordEvent
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.withContext
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.map
|
||||
|
||||
/**
|
||||
* The workouts [pubkeyHex] has published, from [sinceEpochSeconds] onwards, as the cache currently
|
||||
* holds them.
|
||||
* The workouts [pubkeyHex] has published, as a live view of the cache.
|
||||
*
|
||||
* Shared by the My Fitness dashboard, which counts them, and the New Workout carousel, which uses
|
||||
* them to avoid offering a workout the user already shared. Both need the same view of "what have
|
||||
* I already posted", and two answers to that question would mean the carousel offering something
|
||||
* the dashboard knows is a duplicate.
|
||||
*
|
||||
* Scans off the main thread: LocalCache holds every event the session has seen and this walks all
|
||||
* of them. It reports only what is already cached — it issues no REQ of its own.
|
||||
* [LocalCache.observeEvents] rather than a scan: it is indexed by kind and author, so it neither
|
||||
* walks every note in the cache nor needs re-running on a timer — the dashboard updates itself
|
||||
* when a relay delivers a workout, including the one the user just published.
|
||||
*
|
||||
* Reports only what the cache holds; it issues no REQ of its own.
|
||||
*/
|
||||
suspend fun publishedWorkoutsOf(
|
||||
pubkeyHex: String,
|
||||
sinceEpochSeconds: Long,
|
||||
): List<DetectedWorkout> =
|
||||
withContext(Dispatchers.Default) {
|
||||
LocalCache.notes
|
||||
.filterIntoSet { _, note ->
|
||||
val event = note.event
|
||||
event is WorkoutRecordEvent && event.pubKey == pubkeyHex
|
||||
}.mapNotNull { (it.event as WorkoutRecordEvent).toDetectedWorkout() }
|
||||
.filter { it.startTimeEpochSeconds >= sinceEpochSeconds }
|
||||
}
|
||||
fun publishedWorkoutsOf(pubkeyHex: String): Flow<List<DetectedWorkout>> =
|
||||
LocalCache
|
||||
.observeEvents<WorkoutRecordEvent>(
|
||||
Filter(kinds = listOf(WorkoutRecordEvent.KIND), authors = listOf(pubkeyHex)),
|
||||
).map { events -> events.mapNotNull { it.toDetectedWorkout() } }
|
||||
|
||||
+15
-5
@@ -22,6 +22,9 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.fitness
|
||||
|
||||
import androidx.compose.runtime.Composable
|
||||
import com.vitorpamplona.amethyst.commons.resources.Res
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_duration_hours_minutes
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_duration_minutes
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_duration_seconds
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_unit_ft
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_unit_km
|
||||
import com.vitorpamplona.amethyst.commons.resources.my_fitness_unit_m
|
||||
@@ -40,17 +43,24 @@ import kotlin.math.roundToLong
|
||||
*/
|
||||
internal fun prefersMiles(): Boolean = phonePrefersMiles()
|
||||
|
||||
/** `7h 12m` / `42m` / `45s` — a total, so hours run past 24 rather than wrapping. */
|
||||
/**
|
||||
* `7h 12m` / `42m` / `45s` — a total, so hours run past 24 rather than wrapping.
|
||||
*
|
||||
* The unit suffixes come from string resources: "h"/"m"/"s" are English abbreviations, and the
|
||||
* order of the two parts is not universal either, so both belong to the translator rather than
|
||||
* to this function.
|
||||
*/
|
||||
@Composable
|
||||
internal fun formatDuration(totalSeconds: Long): String {
|
||||
if (totalSeconds <= 0) return "0m"
|
||||
if (totalSeconds <= 0) return stringRes(Res.string.my_fitness_duration_minutes, 0)
|
||||
|
||||
val hours = totalSeconds / 3600
|
||||
val minutes = (totalSeconds % 3600) / 60
|
||||
|
||||
return when {
|
||||
hours > 0 -> "${hours}h ${minutes}m"
|
||||
minutes > 0 -> "${minutes}m"
|
||||
else -> "${totalSeconds}s"
|
||||
hours > 0 -> stringRes(Res.string.my_fitness_duration_hours_minutes, hours, minutes)
|
||||
minutes > 0 -> stringRes(Res.string.my_fitness_duration_minutes, minutes)
|
||||
else -> stringRes(Res.string.my_fitness_duration_seconds, totalSeconds)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+61
-26
@@ -25,13 +25,22 @@ import androidx.compose.runtime.Immutable
|
||||
import androidx.compose.runtime.Stable
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.vitorpamplona.amethyst.commons.fitness.DetectedWorkout
|
||||
import com.vitorpamplona.amethyst.commons.fitness.TrainingLog
|
||||
import com.vitorpamplona.amethyst.commons.fitness.WorkoutStats
|
||||
import com.vitorpamplona.amethyst.service.workouts.health.HealthConnectManager
|
||||
import com.vitorpamplona.amethyst.service.workouts.health.publishedWorkoutsOf
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.SharingStarted
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.combine
|
||||
import kotlinx.coroutines.flow.flatMapLatest
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import kotlinx.coroutines.flow.flowOn
|
||||
import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.launch
|
||||
import java.time.Duration
|
||||
import java.time.Instant
|
||||
@@ -64,7 +73,7 @@ class MyFitnessViewModel : ViewModel() {
|
||||
|
||||
@Immutable
|
||||
sealed interface State {
|
||||
/** First load, or a reload after a permission change. */
|
||||
/** First load, before Health Connect has been checked. */
|
||||
data object Loading : State
|
||||
|
||||
/**
|
||||
@@ -78,46 +87,72 @@ class MyFitnessViewModel : ViewModel() {
|
||||
) : State
|
||||
}
|
||||
|
||||
private val _state = MutableStateFlow<State>(State.Loading)
|
||||
val state: StateFlow<State> = _state.asStateFlow()
|
||||
private val pubkeyHex = MutableStateFlow<String?>(null)
|
||||
|
||||
/**
|
||||
* Health Connect's contribution. A push source: the platform has no change feed we can
|
||||
* observe, so [refresh] re-reads it when the screen resumes or a permission is granted.
|
||||
*/
|
||||
private val fromHealthConnect = MutableStateFlow<List<DetectedWorkout>>(emptyList())
|
||||
|
||||
/** Null until the first [refresh] resolves, which is what keeps the screen on [State.Loading]. */
|
||||
private val healthConnectStatus = MutableStateFlow<HealthConnectStatus?>(null)
|
||||
|
||||
private var manager: HealthConnectManager? = null
|
||||
|
||||
/** The pubkey whose workouts this dashboard summarises. Set by the screen before refreshing. */
|
||||
private var pubkeyHex: String? = null
|
||||
|
||||
fun init(pubkeyHex: String) {
|
||||
if (this.pubkeyHex != pubkeyHex) {
|
||||
this.pubkeyHex = pubkeyHex
|
||||
_state.value = State.Loading
|
||||
/**
|
||||
* The user's published workouts, live. Re-subscribes on an account switch; a workout arriving
|
||||
* from a relay — or the one the user just posted — lands here without a refresh.
|
||||
*/
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
private val fromRelays: Flow<List<DetectedWorkout>> =
|
||||
pubkeyHex.flatMapLatest { me ->
|
||||
if (me == null) flowOf(emptyList()) else publishedWorkoutsOf(me)
|
||||
}
|
||||
|
||||
val state: StateFlow<State> =
|
||||
combine(fromHealthConnect, fromRelays, healthConnectStatus) { healthConnect, published, status ->
|
||||
if (status == null) {
|
||||
State.Loading
|
||||
} else {
|
||||
val now = Instant.now()
|
||||
val since = now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS)).epochSecond
|
||||
|
||||
State.Ready(
|
||||
report =
|
||||
WorkoutStats.report(
|
||||
TrainingLog.merge(healthConnect, published.filter { it.startTimeEpochSeconds >= since }),
|
||||
now,
|
||||
),
|
||||
healthConnect = status,
|
||||
)
|
||||
}
|
||||
}.flowOn(Dispatchers.Default)
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), State.Loading)
|
||||
|
||||
/** The account whose workouts this dashboard summarises. */
|
||||
fun init(pubkeyHex: String) {
|
||||
this.pubkeyHex.value = pubkeyHex
|
||||
}
|
||||
|
||||
/**
|
||||
* Rebuilds the dashboard. Safe to call on every resume: it re-checks permissions as well as
|
||||
* data, so revoking access in Health Connect drops those workouts out of the log rather than
|
||||
* leaving stale numbers on display.
|
||||
* Re-reads Health Connect. Safe to call on every resume: it re-checks permissions as well as
|
||||
* data, so revoking access drops those workouts out of the log rather than leaving stale
|
||||
* numbers on display. The published side needs no refresh — it is observed.
|
||||
*/
|
||||
fun refresh(context: Context) {
|
||||
viewModelScope.launch {
|
||||
val now = Instant.now()
|
||||
val since = now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS))
|
||||
|
||||
val status = healthConnectStatus(context)
|
||||
val fromHealthConnect =
|
||||
|
||||
fromHealthConnect.value =
|
||||
if (status == HealthConnectStatus.CONNECTED) {
|
||||
manager?.readWorkouts(since, now).orEmpty()
|
||||
val now = Instant.now()
|
||||
manager?.readWorkouts(now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS)), now).orEmpty()
|
||||
} else {
|
||||
emptyList()
|
||||
}
|
||||
|
||||
val fromRelays = publishedWorkoutsOf(pubkeyHex ?: return@launch, since.epochSecond)
|
||||
|
||||
_state.value =
|
||||
State.Ready(
|
||||
report = WorkoutStats.report(TrainingLog.merge(fromHealthConnect, fromRelays), now),
|
||||
healthConnect = status,
|
||||
)
|
||||
healthConnectStatus.value = status
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+2
-1
@@ -78,6 +78,7 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.health.HealthConne
|
||||
import com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.labelRes
|
||||
import com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.symbol
|
||||
import com.vitorpamplona.amethyst.ui.stringRes
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.launch
|
||||
import java.time.Duration
|
||||
import java.time.Instant
|
||||
@@ -123,7 +124,7 @@ fun DetectedWorkoutCarousel(
|
||||
// the same effort. merge() flags the Health Connect copies that match something
|
||||
// already posted, so drop those; what is left is genuinely unshared.
|
||||
TrainingLog
|
||||
.merge(manager.readWorkouts(since), publishedWorkoutsOf(myPubkey, since.epochSecond))
|
||||
.merge(manager.readWorkouts(since), publishedWorkoutsOf(myPubkey).first())
|
||||
.filter { it.origin == WorkoutOrigin.HEALTH_CONNECT && !it.alreadyPublished }
|
||||
.sortedByDescending { it.startTimeEpochSeconds }
|
||||
} else {
|
||||
|
||||
@@ -694,6 +694,11 @@
|
||||
<string name="my_fitness_active_days">Active days</string>
|
||||
<string name="my_fitness_streak">Day streak</string>
|
||||
<string name="my_fitness_time">Time</string>
|
||||
<!-- Workout duration totals. %1$d is hours, %2$d is minutes; reorder and re-abbreviate
|
||||
freely — these are English abbreviations for hour/minute/second. -->
|
||||
<string name="my_fitness_duration_hours_minutes">%1$dh %2$dm</string>
|
||||
<string name="my_fitness_duration_minutes">%1$dm</string>
|
||||
<string name="my_fitness_duration_seconds">%1$ds</string>
|
||||
<string name="my_fitness_distance">Distance</string>
|
||||
<string name="my_fitness_calories">Calories</string>
|
||||
<string name="my_fitness_steps">Steps</string>
|
||||
|
||||
Reference in New Issue
Block a user