From 0debf8a0233be1bd06debb7da7dba8b4b6e6ea87 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Thu, 17 Sep 2026 10:48:07 -0400 Subject: [PATCH] fix: don't offer to re-share a workout shared from Health Connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit gated the share button on WorkoutOrigin, which closes only half the case. When a user shares a workout their watch recorded, TrainingLog.merge keeps the Health Connect copy and drops the published one, so the survivor is still WorkoutOrigin.HEALTH_CONNECT even though a kind 1301 for it is already out there — and the dashboard went on offering to share it. That is the likelier route to a duplicate, since sharing from Health Connect is the feature's main flow. Origin answers "where did this data come from", which is the wrong question. DetectedWorkout now also carries alreadyPublished, the merge hands it to the surviving copy, and the published-event mapper sets it too, so the dashboard has one field to read. Confirmed on device: a Health Connect ride shared from My Fitness keeps its metrics, does not duplicate, and loses its share button, while the six unshared sessions keep theirs. Co-Authored-By: Claude Opus 5 (1M context) --- .../workouts/fitness/MyFitnessScreen.kt | 10 ++-- .../commons/fitness/DetectedWorkout.kt | 10 ++++ .../amethyst/commons/fitness/TrainingLog.kt | 18 ++++++- .../fitness/PublishedWorkoutMappingTest.kt | 3 ++ .../commons/fitness/TrainingLogTest.kt | 49 +++++++++++++++++++ 5 files changed, 84 insertions(+), 6 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessScreen.kt index bf1886b371..ef3405c8e9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessScreen.kt @@ -56,7 +56,6 @@ import androidx.lifecycle.compose.LifecycleResumeEffect import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewmodel.compose.viewModel import com.vitorpamplona.amethyst.commons.fitness.DetectedWorkout -import com.vitorpamplona.amethyst.commons.fitness.WorkoutOrigin import com.vitorpamplona.amethyst.commons.fitness.WorkoutStats import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols @@ -511,10 +510,11 @@ private fun RecentWorkouts( overflow = TextOverflow.Ellipsis, modifier = Modifier.weight(1f), ) - // Only what is still unpublished can be shared. A workout that came - // back from the relays is already posted: offering to share it again - // would publish a second kind 1301 for the same effort. - if (workout.origin != WorkoutOrigin.PUBLISHED) { + // Only what is still unpublished can be shared. Offering to share a + // workout that is already posted would publish a second kind 1301 for + // the same effort — whether it came back from a relay, or is the + // Health Connect copy of one the user shared earlier. + if (!workout.alreadyPublished) { TextButton(onClick = { onShare(workout, label) }) { Text(stringRes(Res.string.my_fitness_share), style = MaterialTheme.typography.labelMedium) } diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/DetectedWorkout.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/DetectedWorkout.kt index 05c4aa8129..a99e9bf58d 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/DetectedWorkout.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/DetectedWorkout.kt @@ -64,6 +64,16 @@ data class DetectedWorkout( * originated and where every constructor but the published-event mapper still builds from. */ val origin: WorkoutOrigin = WorkoutOrigin.HEALTH_CONNECT, + /** + * Whether a kind 1301 for this workout already exists. + * + * Not the same question as [origin]: a workout read from Health Connect is published the + * moment the user shares it, and [TrainingLog.merge] keeps the richer Health Connect copy + * rather than the published one — so the survivor is still [WorkoutOrigin.HEALTH_CONNECT] + * while a kind 1301 for it is already out there. Anything offering to publish a workout + * must read this, not the origin, or it offers to post the same effort twice. + */ + val alreadyPublished: Boolean = false, /** * How many Health Connect sessions this workout represents. 1 for a raw * session; higher when [WorkoutMerger] combined several close-by same-type diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLog.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLog.kt index ad34dd5671..ef62be0f78 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLog.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLog.kt @@ -50,17 +50,31 @@ object TrainingLog { * from their watch — the Health Connect copy wins: it carries the metrics the published * event may have dropped (heart rate, steps, climb), and its start time is the recorded one * rather than a publish timestamp. + * + * The winner keeps the loser's one piece of information: that a kind 1301 for this workout + * exists. Dropping the published copy would otherwise lose that fact, and the survivor — + * still flagged as Health Connect data — would read as never posted. See + * [DetectedWorkout.alreadyPublished]. */ fun merge( healthConnect: List, published: List, ): List { + val flagged = + healthConnect.map { recorded -> + if (published.any { recorded.isProbablySameWorkoutAs(it) }) { + recorded.copy(alreadyPublished = true) + } else { + recorded + } + } + val deduped = published.filterNot { candidate -> healthConnect.any { it.isProbablySameWorkoutAs(candidate) } } - return (healthConnect + deduped).sortedByDescending { it.startTimeEpochSeconds } + return (flagged + deduped).sortedByDescending { it.startTimeEpochSeconds } } private fun DetectedWorkout.isProbablySameWorkoutAs(other: DetectedWorkout): Boolean = @@ -97,5 +111,7 @@ fun WorkoutRecordEvent.toDetectedWorkout(): DetectedWorkout? { elevationGainMeters = elevationGain()?.toMeters()?.takeIf { it > 0 }, source = workoutSource() ?: "", origin = WorkoutOrigin.PUBLISHED, + // It came back from a relay, so by definition it is already out there. + alreadyPublished = true, ) } diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/PublishedWorkoutMappingTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/PublishedWorkoutMappingTest.kt index 6931914b53..264e4735cf 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/PublishedWorkoutMappingTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/PublishedWorkoutMappingTest.kt @@ -34,6 +34,7 @@ import com.vitorpamplona.quartz.experimental.fitness.workout.tags.WorkoutStartTi import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test class PublishedWorkoutMappingTest { @@ -73,6 +74,8 @@ class PublishedWorkoutMappingTest { assertEquals(6000, workout.steps) assertEquals(createdAt - 3600, workout.startTimeEpochSeconds) assertEquals(WorkoutOrigin.PUBLISHED, workout.origin) + // Recovered from a relay, so the dashboard must not offer to publish it again. + assertTrue(workout.alreadyPublished) } /** Miles on the wire must land as metres in the log, or every total is wrong by 1.6x. */ diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLogTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLogTest.kt index 51975cd716..bdfb0f8e1c 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLogTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/fitness/TrainingLogTest.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.amethyst.commons.fitness import com.vitorpamplona.quartz.experimental.fitness.workout.tags.ExerciseType import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test @@ -49,6 +50,8 @@ class TrainingLogTest { elevationGainMeters = null, source = "test", origin = origin, + // Matches the real mappers: anything recovered from a relay is by definition published. + alreadyPublished = origin == WorkoutOrigin.PUBLISHED, ) @Test @@ -82,6 +85,52 @@ class TrainingLogTest { assertEquals(150, merged[0].avgHeartRate) } + /** + * The survivor is Health Connect data, so its origin cannot answer "has this been posted?". + * Without the flag the dashboard offers to share it again and the user posts a second + * kind 1301 for the same effort. + */ + @Test + fun `the surviving copy remembers that the workout was already published`() { + val hc = listOf(workout("hc", WorkoutOrigin.HEALTH_CONNECT, noon)) + val published = listOf(workout("pub", WorkoutOrigin.PUBLISHED, noon + 60)) + + val merged = TrainingLog.merge(hc, published) + + assertEquals(1, merged.size) + assertEquals(WorkoutOrigin.HEALTH_CONNECT, merged[0].origin) + assertTrue(merged[0].alreadyPublished) + } + + @Test + fun `a health connect workout that was never shared is not marked published`() { + val hc = listOf(workout("hc", WorkoutOrigin.HEALTH_CONNECT, noon)) + val published = listOf(workout("pub", WorkoutOrigin.PUBLISHED, noon + TrainingLog.DEDUPE_TOLERANCE_SECONDS + 1)) + + val merged = TrainingLog.merge(hc, published) + + assertEquals(2, merged.size) + assertFalse(merged.first { it.id == "hc" }.alreadyPublished) + assertTrue(merged.first { it.id == "pub" }.alreadyPublished) + } + + /** Only the matching session is flagged; an unrelated one in the same log is left alone. */ + @Test + fun `flagging one workout does not flag the rest of the log`() { + val hc = + listOf( + workout("shared", WorkoutOrigin.HEALTH_CONNECT, noon), + workout("private", WorkoutOrigin.HEALTH_CONNECT, noon - 86_400), + ) + val published = listOf(workout("pub", WorkoutOrigin.PUBLISHED, noon)) + + val merged = TrainingLog.merge(hc, published) + + assertEquals(2, merged.size) + assertTrue(merged.first { it.id == "shared" }.alreadyPublished) + assertFalse(merged.first { it.id == "private" }.alreadyPublished) + } + @Test fun `a published workout just outside the tolerance is kept as its own`() { val hc = listOf(workout("hc", WorkoutOrigin.HEALTH_CONNECT, noon))