mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 11:18:24 +00:00
fix: don't offer to re-share a workout shared from Health Connect
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ec8a5fa573
commit
0debf8a023
+5
-5
@@ -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)
|
||||
}
|
||||
|
||||
+10
@@ -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
|
||||
|
||||
+17
-1
@@ -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<DetectedWorkout>,
|
||||
published: List<DetectedWorkout>,
|
||||
): List<DetectedWorkout> {
|
||||
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,
|
||||
)
|
||||
}
|
||||
|
||||
+3
@@ -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. */
|
||||
|
||||
+49
@@ -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))
|
||||
|
||||
Reference in New Issue
Block a user