From 5aa5183d1ce6d0678b656ce4ebd09ea1a573408d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Jul 2026 21:00:38 +0000 Subject: [PATCH 1/2] Fix fragmented period logging and add History merge action Logging consecutive days used to fragment into disjoint period entries once a period's end date became fixed (e.g. via the "no end date set" confirmation): the next day no longer fell within any existing range, so tapping it silently started a brand-new, disconnected period with the in-between day unloggable in either one. PeriodRepository.resolvePeriodForLogging now checks, before creating a new entry, whether the logged date is adjacent to an existing period's start or end and extends it, or bridges a one-day gap between two periods and merges them. HomeScreen routes calendar taps and the Log Period FAB through this resolver instead of the old "covers today"-only check. The History tab also gains a manual "Merge with previous period" action per card (via PeriodRepository.mergePeriods, now reusable outside the one-time migration) so users can fix up any already-fragmented history that predates this fix or has a gap too large to auto-detect. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01WUWr5zTAXGDw1EooCpHxfy --- LESSONS.md | 4 +- .../goflo/data/repository/PeriodRepository.kt | 70 +++++++++ .../goflo/ui/screens/history/HistoryScreen.kt | 97 +++++++++++-- .../ui/screens/history/HistoryViewModel.kt | 14 ++ .../goflo/ui/screens/home/HomeScreen.kt | 16 ++- .../goflo/ui/screens/home/HomeViewModel.kt | 13 ++ .../data/repository/PeriodRepositoryTest.kt | 135 ++++++++++++++++++ .../period-logging-fragmentation-fix.json | 9 ++ 8 files changed, 338 insertions(+), 20 deletions(-) create mode 100644 changelog/unreleased/period-logging-fragmentation-fix.json diff --git a/LESSONS.md b/LESSONS.md index 2e3275a..f43ac78 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -97,8 +97,8 @@ A prediction window (e.g. a 5-day expected period) should remain visible as long **Don't double-count with offset when SQLite immediately reflects inserts in aggregate queries** In a Room migration loop that inserts rows one-by-one, calling `MAX(displayOrder)+1` in a subquery correctly reflects all previously-inserted rows in the same transaction — SQLite is not a snapshot. Adding a separate `offset` counter on top of that result double-counts and produces gaps (e.g., displayOrder 7, 9, 11 instead of 7, 8, 9). Remove the offset variable and let the `MAX+1` subquery self-increment across the loop. -**Date-range entities with an "ongoing" (null end) state need an existence check before quick-log entry points create a new record** -When an entity represents a date range and a null end date means "ongoing, extends through today" (e.g. an active period), any UI shortcut that creates a *new* record for a tapped date (calendar tap, FAB, speed dial) must first check whether that date already falls within an existing record's range. Otherwise the user ends up with two overlapping "ongoing" records for what they consider a single continuous span. Add a shared `recordForDate(records, date)` helper that treats a null end as `today`, and route quick-log entry points through it: if a match exists, navigate to edit it; otherwise create new. +**Date-range entities with an "ongoing" (null end) state need an existence check before quick-log entry points create a new record — and an adjacency check once the range is fixed** +When an entity represents a date range and a null end date means "ongoing, extends through today" (e.g. an active period), any UI shortcut that creates a *new* record for a tapped date (calendar tap, FAB, speed dial) must first check whether that date already falls within an existing record's range. Otherwise the user ends up with two overlapping "ongoing" records for what they consider a single continuous span. Add a shared `recordForDate(records, date)` helper that treats a null end as `today`, and route quick-log entry points through it: if a match exists, navigate to edit it; otherwise create new. This "coverage" check alone is not enough: the moment a record's end date is fixed (set explicitly, or via any confirm-dialog default that isn't the date the user meant), logging the very next day no longer falls inside any range and silently starts a second, disconnected record — with the day in between unreachable in either one. Extend the same entry point with an adjacency check: a tapped date exactly one day after some record's end (or one day before another's start) should extend that record instead of creating a new one; a date that bridges a one-day gap between two records should merge them. Also give the user a manual "merge with adjacent record" action in the list/history view as a backstop for gaps the auto-adjacency logic can't infer (larger gaps, pre-existing fragmented data). **Insert/upsert flags need a separate edit-by-ID path** diff --git a/app/src/main/java/com/mapgie/goflo/data/repository/PeriodRepository.kt b/app/src/main/java/com/mapgie/goflo/data/repository/PeriodRepository.kt index 98ac1bb..18fedb7 100644 --- a/app/src/main/java/com/mapgie/goflo/data/repository/PeriodRepository.kt +++ b/app/src/main/java/com/mapgie/goflo/data/repository/PeriodRepository.kt @@ -70,6 +70,76 @@ class PeriodRepository( periodDao.updatePeriod(period.copy(flowLevel = flowLevel)) } + /** + * Resolves which period should be opened when logging [date], extending or + * merging adjacent entries so consecutive-day logging never fragments into + * disjoint periods. + * + * - A date already covered by an existing period (including an ongoing + * period, which extends through today) returns that period's id as-is. + * - A date exactly one day after one period's end AND one day before + * another period's start bridges the gap between them: the two periods + * are merged into one (see [mergePeriods]) and the merged id is returned. + * - A date exactly one day after a period's end extends that period's end + * date to include it. + * - A date exactly one day before a period's start pulls that period's + * start date back to include it. + * + * Returns null when none of the above apply — the caller should start a + * brand-new period entry for [date]. + */ + suspend fun resolvePeriodForLogging(date: LocalDate): Long? { + val periods = periodDao.getAllPeriodsOnce() + periodForDate(periods, date)?.let { return it.id } + + val dateStr = date.toString() + val before = periods.firstOrNull { it.endDate == date.minusDays(1).toString() } + val after = periods.firstOrNull { it.startDate == date.plusDays(1).toString() } + + return when { + before != null && after != null -> mergePeriods(before, after) + before != null -> { + periodDao.updatePeriod(before.copy(endDate = dateStr)) + before.id + } + after != null -> { + periodDao.updatePeriod(after.copy(startDate = dateStr)) + after.id + } + else -> null + } + } + + /** + * Merges [later] into [earlier], keeping [earlier]'s id. The merged end + * date is whichever of the two is later, with an ongoing (null) end date + * always winning over a fixed one. Notes are concatenated and symptoms + * from both periods are combined onto [earlier] before [later] is deleted. + * + * @return [earlier]'s id. + */ + suspend fun mergePeriods(earlier: PeriodEntry, later: PeriodEntry): Long { + val earlierEnd = earlier.endDate?.let { LocalDate.parse(it) } + val laterEnd = later.endDate?.let { LocalDate.parse(it) } + val mergedEnd = when { + earlierEnd == null || laterEnd == null -> null + laterEnd.isAfter(earlierEnd) -> laterEnd + else -> earlierEnd + } + + val earlierSymptoms = symptomDao.getSymptomsForPeriodOnce(earlier.id).map { it.symptomType }.toSet() + symptomDao.getSymptomsForPeriodOnce(later.id) + .map { it.symptomType } + .filter { it.isNotBlank() && it !in earlierSymptoms } + .forEach { symptomDao.insertSymptom(SymptomEntry(periodId = earlier.id, symptomType = it)) } + + periodDao.updatePeriod( + earlier.copy(endDate = mergedEnd?.toString(), notes = mergeNotes(earlier.notes, later.notes)) + ) + periodDao.deletePeriod(later) + return earlier.id + } + /** * One-time data fixup: merges period entries whose date ranges overlap into a * single entry. An ongoing period (endDate == null) is treated as extending diff --git a/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryScreen.kt b/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryScreen.kt index 08cdc0c..de16467 100644 --- a/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryScreen.kt +++ b/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryScreen.kt @@ -17,11 +17,15 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Delete +import androidx.compose.material.icons.filled.MoreVert import androidx.compose.material3.AlertDialog import androidx.compose.material3.Card import androidx.compose.material3.CardDefaults +import androidx.compose.material3.DropdownMenu +import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold import androidx.compose.material3.Snackbar @@ -141,12 +145,19 @@ fun HistoryScreen( if (days in 15..60) put(a.id, days) } } + // Maps a period to its immediately preceding (chronologically earlier) + // period, if any — used to offer a "merge with previous period" action + // so fragmented history can be fixed up from the list itself. + val previousPeriodMap = buildMap { + sortedByStart.zipWithNext { older, newer -> put(newer.id, older) } + } items(periods, key = { it.id }) { period -> SwipeablePeriodCard( - period = period, - cycleLength = cycleLengthMap[period.id], - onDelete = { + period = period, + cycleLength = cycleLengthMap[period.id], + previousPeriod = previousPeriodMap[period.id], + onDelete = { // Stage the deletion — card disappears from the list immediately. viewModel.stageDeletion(period) // Show snackbar with Undo action from the screen-level scope @@ -163,8 +174,9 @@ fun HistoryScreen( } } }, - onClick = { onNavigate(Screen.LogPeriod.withId(period.id)) }, - modifier = Modifier, + onMerge = { previous -> viewModel.mergePeriods(previous, period) }, + onClick = { onNavigate(Screen.LogPeriod.withId(period.id)) }, + modifier = Modifier, ) } item { Spacer(Modifier.height(4.dp)) } @@ -199,7 +211,9 @@ fun HistoryScreen( private fun SwipeablePeriodCard( period: PeriodEntry, cycleLength: Int?, + previousPeriod: PeriodEntry?, onDelete: () -> Unit, + onMerge: (PeriodEntry) -> Unit, onClick: () -> Unit, modifier: Modifier = Modifier, ) { @@ -265,17 +279,52 @@ private fun SwipeablePeriodCard( } }, ) { - PeriodCard(period = period, cycleLength = cycleLength, onClick = onClick) + PeriodCard( + period = period, + cycleLength = cycleLength, + previousPeriod = previousPeriod, + onMerge = onMerge, + onClick = onClick, + ) } } // ── Period card ─────────────────────────────────────────────────────────────── @Composable -private fun PeriodCard(period: PeriodEntry, cycleLength: Int? = null, onClick: () -> Unit) { +private fun PeriodCard( + period: PeriodEntry, + cycleLength: Int? = null, + previousPeriod: PeriodEntry? = null, + onMerge: (PeriodEntry) -> Unit = {}, + onClick: () -> Unit, +) { val start = LocalDate.parse(period.startDate) val end = period.endDate?.let { LocalDate.parse(it) } val duration = if (end != null) "${ChronoUnit.DAYS.between(start, end) + 1} days" else "Ongoing" + var showOverflowMenu by remember { mutableStateOf(false) } + var showMergeConfirm by remember { mutableStateOf(false) } + + if (showMergeConfirm && previousPeriod != null) { + val previousStart = LocalDate.parse(previousPeriod.startDate).format(displayFormat) + AlertDialog( + onDismissRequest = { showMergeConfirm = false }, + title = { Text("Merge periods?") }, + text = { + Text( + "This will combine this entry with the period starting $previousStart into a " + + "single period. This can't be undone." + ) + }, + confirmButton = { + TextButton(onClick = { showMergeConfirm = false; onMerge(previousPeriod) }) { + Text("Merge") + } + }, + dismissButton = { TextButton(onClick = { showMergeConfirm = false }) { Text("Cancel") } } + ) + } + Card( modifier = Modifier .fillMaxWidth() @@ -285,10 +334,36 @@ private fun PeriodCard(period: PeriodEntry, cycleLength: Int? = null, onClick: ( elevation = CardDefaults.cardElevation(defaultElevation = 2.dp) ) { Column(modifier = Modifier.padding(16.dp), verticalArrangement = Arrangement.spacedBy(6.dp)) { - Text( - text = start.format(displayFormat), - style = MaterialTheme.typography.titleMedium - ) + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.SpaceBetween, + verticalAlignment = Alignment.CenterVertically + ) { + Text( + text = start.format(displayFormat), + style = MaterialTheme.typography.titleMedium + ) + if (previousPeriod != null) { + Box { + IconButton(onClick = { showOverflowMenu = true }) { + Icon( + imageVector = Icons.Default.MoreVert, + contentDescription = "More options", + tint = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + DropdownMenu( + expanded = showOverflowMenu, + onDismissRequest = { showOverflowMenu = false } + ) { + DropdownMenuItem( + text = { Text("Merge with previous period") }, + onClick = { showOverflowMenu = false; showMergeConfirm = true } + ) + } + } + } + } Text( text = if (end != null) "Until ${end.format(displayFormat)} · $duration" else "Ongoing", style = MaterialTheme.typography.bodyMedium, diff --git a/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryViewModel.kt b/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryViewModel.kt index 406acef..5c304e3 100644 --- a/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryViewModel.kt +++ b/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryViewModel.kt @@ -103,6 +103,20 @@ class HistoryViewModel( _pendingDeleteIds.update { it - period.id } } + // ── Manual merge ─────────────────────────────────────────────────────────── + + /** + * Merges [older] and [newer] into a single period entry, fixing up a + * fragmented history (e.g. two entries that should have been one + * continuous period). Keeps [older]'s id; [newer] is deleted. + */ + fun mergePeriods(older: PeriodEntry, newer: PeriodEntry) { + viewModelScope.launch { + repository.mergePeriods(older, newer) + application?.let { GoFloWidget.updateAllWidgets(it) } + } + } + class Factory( private val repository: PeriodRepository, private val application: Application? = null, diff --git a/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeScreen.kt b/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeScreen.kt index 4b38753..09a7bd7 100644 --- a/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeScreen.kt +++ b/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeScreen.kt @@ -63,7 +63,6 @@ import androidx.compose.ui.semantics.role import androidx.compose.ui.semantics.semantics import androidx.compose.ui.unit.dp import com.mapgie.goflo.BuildConfig -import com.mapgie.goflo.data.repository.PeriodRepository import com.mapgie.goflo.ui.components.CalendarGrid import com.mapgie.goflo.ui.components.DayLogSheet import com.mapgie.goflo.ui.navigation.Screen @@ -113,13 +112,16 @@ fun HomeScreen( // If [date] already falls within an existing period's range (including an // ongoing period, which extends through today), edit that period instead of - // creating a new, overlapping entry. + // creating a new, overlapping entry. Dates immediately adjacent to an + // existing period extend it instead of starting a new, disconnected one; + // a date that bridges a one-day gap between two periods merges them. fun navigateToLogPeriod(date: LocalDate) { - val existing = PeriodRepository.periodForDate(state.periods, date) - if (existing != null) { - onNavigate(Screen.LogPeriod.withId(existing.id)) - } else { - onNavigate(Screen.LogPeriod.newEntryForDate(date)) + viewModel.resolveLogPeriodTarget(date) { existingId -> + if (existingId != null) { + onNavigate(Screen.LogPeriod.withId(existingId)) + } else { + onNavigate(Screen.LogPeriod.newEntryForDate(date)) + } } } diff --git a/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeViewModel.kt b/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeViewModel.kt index 9921343..f31c4c7 100644 --- a/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeViewModel.kt +++ b/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeViewModel.kt @@ -187,6 +187,19 @@ class HomeViewModel( fun selectDay(date: LocalDate) { _selectedDay.value = date } fun clearSelectedDay() { _selectedDay.value = null } + /** + * Resolves which period id logging [date] should target, extending or + * merging adjacent entries as needed (see [PeriodRepository.resolvePeriodForLogging]), + * then hands the result to [onResolved]. A null result means no + * existing/adjacent period applies, so the caller should start a + * brand-new entry for [date]. + */ + fun resolveLogPeriodTarget(date: LocalDate, onResolved: (Long?) -> Unit) { + viewModelScope.launch { + onResolved(repository.resolvePeriodForLogging(date)) + } + } + // ── Quick increment (Plus One categories) ─────────────────────────────────── /** Transient confirmation message after an instant increment; null when none pending. */ diff --git a/app/src/test/java/com/mapgie/goflo/data/repository/PeriodRepositoryTest.kt b/app/src/test/java/com/mapgie/goflo/data/repository/PeriodRepositoryTest.kt index ea17ac3..58dc512 100644 --- a/app/src/test/java/com/mapgie/goflo/data/repository/PeriodRepositoryTest.kt +++ b/app/src/test/java/com/mapgie/goflo/data/repository/PeriodRepositoryTest.kt @@ -1,11 +1,72 @@ package com.mapgie.goflo.data.repository +import com.mapgie.goflo.data.database.dao.PeriodDao +import com.mapgie.goflo.data.database.dao.SymptomDao import com.mapgie.goflo.data.database.entities.PeriodEntry +import com.mapgie.goflo.data.database.entities.SymptomEntry +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.runBlocking import org.junit.Assert.assertEquals import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test import java.time.LocalDate +/** In-memory [PeriodDao] fake for exercising [PeriodRepository]'s suspend write paths. */ +private class FakePeriodDao : PeriodDao { + val periods = mutableListOf() + private var nextId = 1L + + override fun getAllPeriods(): Flow> = flowOf(periods.sortedByDescending { it.startDate }) + override suspend fun getAllPeriodsOnce(): List = periods.sortedBy { it.startDate } + override fun getPeriodById(id: Long): Flow = flowOf(periods.firstOrNull { it.id == id }) + + override suspend fun insertPeriod(period: PeriodEntry): Long { + val id = if (period.id != 0L) period.id else nextId++ + periods.removeAll { it.id == id } + periods.add(period.copy(id = id)) + return id + } + + override suspend fun updatePeriod(period: PeriodEntry) { + val index = periods.indexOfFirst { it.id == period.id } + if (index >= 0) periods[index] = period + } + + override suspend fun deletePeriod(period: PeriodEntry) { + periods.removeAll { it.id == period.id } + } + + override suspend fun deleteAllPeriods() { periods.clear() } + override suspend fun countPeriods(): Int = periods.size +} + +/** In-memory [SymptomDao] fake for exercising [PeriodRepository]'s suspend write paths. */ +private class FakeSymptomDao : SymptomDao { + val symptoms = mutableListOf() + private var nextId = 1L + + override fun getSymptomsForPeriod(periodId: Long): Flow> = + flowOf(symptoms.filter { it.periodId == periodId }) + override suspend fun getSymptomsForPeriodOnce(periodId: Long): List = + symptoms.filter { it.periodId == periodId } + override suspend fun insertSymptom(symptom: SymptomEntry) { + symptoms.add(symptom.copy(id = nextId++)) + } + override suspend fun deleteSymptomsByPeriodId(periodId: Long) { + symptoms.removeAll { it.periodId == periodId } + } + override suspend fun deleteAllSymptoms() { symptoms.clear() } + override suspend fun getAllSymptoms(): List = symptoms.toList() + override fun getAllSymptomsFlow(): Flow> = flowOf(symptoms.toList()) + override suspend fun bulkRenameSymptoms(oldLabel: String, newLabel: String) { + val renamed = symptoms.map { if (it.symptomType == oldLabel) it.copy(symptomType = newLabel) else it } + symptoms.clear() + symptoms.addAll(renamed) + } +} + class PeriodRepositoryTest { private fun entry(startDate: String, endDate: String? = null, id: Long = 0L) = @@ -132,4 +193,78 @@ class PeriodRepositoryTest { fun `activePeriod returns null for empty list`() { assertNull(PeriodRepository.activePeriod(emptyList())) } + + // ── resolvePeriodForLogging / mergePeriods ─────────────────────────────── + + private fun buildRepository(vararg seed: PeriodEntry): Triple { + val periodDao = FakePeriodDao() + val symptomDao = FakeSymptomDao() + seed.forEach { periodDao.periods.add(it) } + return Triple(PeriodRepository(periodDao, symptomDao), periodDao, symptomDao) + } + + @Test + fun `resolvePeriodForLogging returns id of period already covering the date`() = runBlocking { + val (repo, _, _) = buildRepository(entry("2024-01-01", "2024-01-05", id = 1)) + assertEquals(1L, repo.resolvePeriodForLogging(LocalDate.of(2024, 1, 3))) + } + + @Test + fun `resolvePeriodForLogging extends a period's end date to the following day`() = runBlocking { + val (repo, dao, _) = buildRepository(entry("2024-01-01", "2024-01-05", id = 1)) + val result = repo.resolvePeriodForLogging(LocalDate.of(2024, 1, 6)) + assertEquals(1L, result) + assertEquals("2024-01-06", dao.periods.first { it.id == 1L }.endDate) + } + + @Test + fun `resolvePeriodForLogging pulls a period's start date back to the preceding day`() = runBlocking { + val (repo, dao, _) = buildRepository(entry("2024-01-05", "2024-01-08", id = 1)) + val result = repo.resolvePeriodForLogging(LocalDate.of(2024, 1, 4)) + assertEquals(1L, result) + assertEquals("2024-01-04", dao.periods.first { it.id == 1L }.startDate) + } + + @Test + fun `resolvePeriodForLogging merges two periods when the date bridges a one-day gap`() = runBlocking { + // Reproduces the reported bug: a period ending 28 June and another starting + // 30 June should be joined into one continuous period by logging 29 June. + val (repo, dao, _) = buildRepository( + entry("2024-06-26", "2024-06-28", id = 1), + entry("2024-06-30", null, id = 2), + ) + val result = repo.resolvePeriodForLogging(LocalDate.of(2024, 6, 29)) + assertEquals(1L, result) + assertEquals(1, dao.periods.size) + val merged = dao.periods.single() + assertEquals("2024-06-26", merged.startDate) + assertNull(merged.endDate) // the later (ongoing) period's null end date wins + } + + @Test + fun `resolvePeriodForLogging returns null when no period is adjacent or covering`() = runBlocking { + val (repo, _, _) = buildRepository(entry("2024-01-01", "2024-01-05", id = 1)) + assertNull(repo.resolvePeriodForLogging(LocalDate.of(2024, 1, 10))) + } + + @Test + fun `mergePeriods keeps the earlier id, combines notes and symptoms, and deletes the later entry`() = runBlocking { + val earlier = entry("2024-06-26", "2024-06-28", id = 1).copy(notes = "cramps") + val later = entry("2024-06-30", "2024-07-01", id = 2).copy(notes = "headache") + val (repo, dao, symptomDao) = buildRepository(earlier, later) + symptomDao.symptoms.add(SymptomEntry(periodId = 1, symptomType = "Cramps")) + symptomDao.symptoms.add(SymptomEntry(periodId = 2, symptomType = "Fatigue")) + + val mergedId = repo.mergePeriods(earlier, later) + + assertEquals(1L, mergedId) + assertEquals(1, dao.periods.size) + val merged = dao.periods.single() + assertEquals("2024-06-26", merged.startDate) + assertEquals("2024-07-01", merged.endDate) + assertTrue(merged.notes.contains("cramps")) + assertTrue(merged.notes.contains("headache")) + val mergedSymptoms = symptomDao.symptoms.filter { it.periodId == 1L }.map { it.symptomType }.toSet() + assertEquals(setOf("Cramps", "Fatigue"), mergedSymptoms) + } } diff --git a/changelog/unreleased/period-logging-fragmentation-fix.json b/changelog/unreleased/period-logging-fragmentation-fix.json new file mode 100644 index 0000000..2fbc24b --- /dev/null +++ b/changelog/unreleased/period-logging-fragmentation-fix.json @@ -0,0 +1,9 @@ +{ + "bump": "minor", + "added": [ + "History entries can now be merged with the previous period to fix up fragmented history" + ], + "fixed": [ + "Logging consecutive days no longer splits a single period into disconnected entries with an unloggable gap in between" + ] +} From e987700629af42a864c3c14ec844975d44197fb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Jul 2026 11:34:53 +0000 Subject: [PATCH 2/2] Bridge the remaining single-day gap and stop merge from deleting data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on #166's fix (periodForDate day-after-end adjacency, deferred extension, one-time migration). Two gaps remained against the exact reported scenario (a period ending day N, an unloggable day N+1, and a new period starting day N+2): - periodForDate's day-after-end match only reopens the earlier period for a deferred extension; if a second period already starts the following day, extending the first just leaves them touching, requiring a further manual History merge. PeriodRepository.mergeGapAt detects this and merges both immediately when the tapped date bridges them, wired through HomeScreen/HomeViewModel. - mergeAdjacentPeriods (the one-time repair migration) only merged already-touching periods (zero-day gap), not periods separated by the single unlogged day this bug actually produces. Extended its gap check from "touching only" to "touching or one day apart". - Both the manual History merge and the migration were calling deleteLogsForPeriod on the absorbed period — a helper written for actual period deletion — silently discarding its flow/symptom tracking-log history instead of preserving it. Removed those calls; the data is still valid, dated history and isn't orphaned by merging. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01WUWr5zTAXGDw1EooCpHxfy --- LESSONS.md | 14 +- .../java/com/mapgie/goflo/GoFloApplication.kt | 20 +-- .../goflo/data/repository/PeriodRepository.kt | 98 ++++--------- .../ui/screens/history/HistoryViewModel.kt | 16 +-- .../goflo/ui/screens/home/HomeScreen.kt | 9 +- .../goflo/ui/screens/home/HomeViewModel.kt | 12 +- .../data/repository/PeriodRepositoryTest.kt | 133 ++++++++++++++++++ .../period-logging-gap-merge-fix.json | 7 + 8 files changed, 203 insertions(+), 106 deletions(-) create mode 100644 changelog/unreleased/period-logging-gap-merge-fix.json diff --git a/LESSONS.md b/LESSONS.md index 259c04d..6e336b8 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -97,11 +97,15 @@ A prediction window (e.g. a 5-day expected period) should remain visible as long **Don't double-count with offset when SQLite immediately reflects inserts in aggregate queries** In a Room migration loop that inserts rows one-by-one, calling `MAX(displayOrder)+1` in a subquery correctly reflects all previously-inserted rows in the same transaction — SQLite is not a snapshot. Adding a separate `offset` counter on top of that result double-counts and produces gaps (e.g., displayOrder 7, 9, 11 instead of 7, 8, 9). Remove the offset variable and let the `MAX+1` subquery self-increment across the loop. -**Date-range entities with an "ongoing" (null end) state need an existence check before quick-log entry points create a new record — and an adjacency check once the range is fixed** -When an entity represents a date range and a null end date means "ongoing, extends through today" (e.g. an active period), any UI shortcut that creates a *new* record for a tapped date (calendar tap, FAB, speed dial) must first check whether that date already falls within an existing record's range. Otherwise the user ends up with two overlapping "ongoing" records for what they consider a single continuous span. Add a shared `recordForDate(records, date)` helper that treats a null end as `today`, and route quick-log entry points through it: if a match exists, navigate to edit it; otherwise create new. This "coverage" check alone is not enough: the moment a record's end date is fixed (set explicitly, or via any confirm-dialog default that isn't the date the user meant), logging the very next day no longer falls inside any range and silently starts a second, disconnected record — with the day in between unreachable in either one. Extend the same entry point with an adjacency check: a tapped date exactly one day after some record's end (or one day before another's start) should extend that record instead of creating a new one; a date that bridges a one-day gap between two records should merge them. Also give the user a manual "merge with adjacent record" action in the list/history view as a backstop for gaps the auto-adjacency logic can't infer (larger gaps, pre-existing fragmented data). - -**"Ongoing" should mean unbounded, not "through today" — and adjacency needs its own check, separate from containment** -A `recordForDate` helper that treats a null end date as `LocalDate.now()` only works while "today" is on or after the date being checked, and only reads clean while a session stays open past midnight without new writes. Reopen the app on a later date, or check a future date, and the same ongoing record silently stops matching — a fresh "new record" branch fires instead of "extend existing," fragmenting what the user considers one continuous span. Treat the ongoing case as having no upper bound at all (any date `>= start` matches) instead of substituting `now()`. Separately, containment (`start <= date <= end`) only catches dates *inside* a closed record's range — it does nothing for the day immediately *after* a closed record's end, which is the single most common way these entities fragment in practice (a user who closes a date-range record explicitly each day, rather than leaving it open, and logs again the next day). Add `date == end.plusDays(1)` as an explicit adjacency match alongside containment; don't assume containment implies "continues." +**Date-range entities with an "ongoing" (null end) state need containment, adjacency, and gap-bridging checks — not just containment — before a quick-log entry point creates a new record** +When an entity represents a date range and a null end date means "ongoing" (e.g. an active period), any UI shortcut that creates a *new* record for a tapped date (calendar tap, FAB, speed dial) must check more than plain containment (`start <= date <= end`), or the same fragmentation bug resurfaces in layers: +1. Treat "ongoing" as unbounded (any date `>= start` matches), not as extending only through `LocalDate.now()`. A `now()` substitute stops matching the moment "today" moves past the last check — reopening the app on a later date, or checking a future date, silently fires "create new" instead of "extend existing." +2. Containment alone still misses the day immediately *after* a closed record's end — the single most common real-world fragmentation path (a user who sets an explicit end date each day rather than leaving it open, then logs again tomorrow). Add `date == end.plusDays(1)` as an explicit adjacency match. +3. Adjacency in isolation can just shift the fragmentation by one row: extending the earlier record to include the day-after-end is a no-op if a *second*, separate record already starts the day after that — the two are now merely touching, not merged. Detect this (a tapped date bridging a one-day gap between two distinct records) and merge them immediately rather than leaving a second manual step. +4. Ship a one-time migration that finds already-fragmented records at these same distances (touching, or a single day apart) and merges them, guarded by a preference flag — the entry-point fix alone never touches data that's already broken on existing installs. +5. Give the user a manual "merge with adjacent record" action in the list/history view too, as a backstop for gaps the auto-detection can't infer (larger gaps, ambiguous cases). + +When implementing the merge itself: don't reach for a `deleteLogsForPeriod`-style helper that was written for actual record *deletion*. Merging is not deleting — the absorbed record's own day-specific data (flow, symptoms, any per-day mirror log) is still valid history and must be preserved, not wiped out on the assumption that it would otherwise "linger orphaned." **Insert/upsert flags need a separate edit-by-ID path** diff --git a/app/src/main/java/com/mapgie/goflo/GoFloApplication.kt b/app/src/main/java/com/mapgie/goflo/GoFloApplication.kt index aa8286c..4961ed6 100644 --- a/app/src/main/java/com/mapgie/goflo/GoFloApplication.kt +++ b/app/src/main/java/com/mapgie/goflo/GoFloApplication.kt @@ -196,14 +196,14 @@ class GoFloApplication : Application() { } /** - * One-time data fixup: merges period entries that sit exactly one day apart - * (e.g. a period closed with an explicit end date, then a new period logged - * for the very next day, before the entry-point fix existed) into a single - * entry. + * One-time data fixup: merges period entries that are touching or sit a + * single unlogged day apart (e.g. a period closed with an explicit end + * date, then a new period logged for the next day or the day after, + * before the entry-point fix existed) into a single entry. * - * Each absorbed period's own tracking-log entries (flow, symptoms, pinned - * categories) are removed too, mirroring the cleanup performed on delete — - * otherwise they'd linger, orphaned, under a date no longer tied to any period. + * The absorbed periods' own tracking-log entries (flow, symptoms, pinned + * categories) are deliberately left in place — they're still valid, dated + * records of what was logged, not orphaned data to discard. * * Only runs once — guarded by the [periodAdjacencyMergeDone] preference flag. */ @@ -212,12 +212,6 @@ class GoFloApplication : Application() { if (prefs.periodAdjacencyMergeDone) return val absorbed = repository.mergeAdjacentPeriods() - for (period in absorbed) { - trackingRepository.deleteLogsForPeriod( - LocalDate.parse(period.startDate), - period.endDate?.let { LocalDate.parse(it) } - ) - } if (absorbed.isNotEmpty()) { GoFloWidget.updateAllWidgets(this) } diff --git a/app/src/main/java/com/mapgie/goflo/data/repository/PeriodRepository.kt b/app/src/main/java/com/mapgie/goflo/data/repository/PeriodRepository.kt index f9b5074..007e492 100644 --- a/app/src/main/java/com/mapgie/goflo/data/repository/PeriodRepository.kt +++ b/app/src/main/java/com/mapgie/goflo/data/repository/PeriodRepository.kt @@ -71,73 +71,25 @@ class PeriodRepository( } /** - * Resolves which period should be opened when logging [date], extending or - * merging adjacent entries so consecutive-day logging never fragments into - * disjoint periods. + * If [date] sits in the single-day gap between one period's end and the + * next period's start, merges the two into one continuous period. * - * - A date already covered by an existing period (including an ongoing - * period, which extends through today) returns that period's id as-is. - * - A date exactly one day after one period's end AND one day before - * another period's start bridges the gap between them: the two periods - * are merged into one (see [mergePeriods]) and the merged id is returned. - * - A date exactly one day after a period's end extends that period's end - * date to include it. - * - A date exactly one day before a period's start pulls that period's - * start date back to include it. + * `periodForDate` already treats the day right after an explicit end date + * as covered by that period (see its doc), so tapping that day alone + * opens the earlier period for a deferred end-date extension. But when a + * *second*, separate period already starts the following day, extending + * the first one in isolation just leaves it newly touching the second — + * still two rows, now requiring a manual History merge to finish the + * job. Detecting and merging both immediately closes the gap in one step. * - * Returns null when none of the above apply — the caller should start a - * brand-new period entry for [date]. + * @return the merged period if [date] bridged a gap, else null. */ - suspend fun resolvePeriodForLogging(date: LocalDate): Long? { + suspend fun mergeGapAt(date: LocalDate): PeriodEntry? { val periods = periodDao.getAllPeriodsOnce() - periodForDate(periods, date)?.let { return it.id } - - val dateStr = date.toString() val before = periods.firstOrNull { it.endDate == date.minusDays(1).toString() } val after = periods.firstOrNull { it.startDate == date.plusDays(1).toString() } - - return when { - before != null && after != null -> mergePeriods(before, after) - before != null -> { - periodDao.updatePeriod(before.copy(endDate = dateStr)) - before.id - } - after != null -> { - periodDao.updatePeriod(after.copy(startDate = dateStr)) - after.id - } - else -> null - } - } - - /** - * Merges [later] into [earlier], keeping [earlier]'s id. The merged end - * date is whichever of the two is later, with an ongoing (null) end date - * always winning over a fixed one. Notes are concatenated and symptoms - * from both periods are combined onto [earlier] before [later] is deleted. - * - * @return [earlier]'s id. - */ - suspend fun mergePeriods(earlier: PeriodEntry, later: PeriodEntry): Long { - val earlierEnd = earlier.endDate?.let { LocalDate.parse(it) } - val laterEnd = later.endDate?.let { LocalDate.parse(it) } - val mergedEnd = when { - earlierEnd == null || laterEnd == null -> null - laterEnd.isAfter(earlierEnd) -> laterEnd - else -> earlierEnd - } - - val earlierSymptoms = symptomDao.getSymptomsForPeriodOnce(earlier.id).map { it.symptomType }.toSet() - symptomDao.getSymptomsForPeriodOnce(later.id) - .map { it.symptomType } - .filter { it.isNotBlank() && it !in earlierSymptoms } - .forEach { symptomDao.insertSymptom(SymptomEntry(periodId = earlier.id, symptomType = it)) } - - periodDao.updatePeriod( - earlier.copy(endDate = mergedEnd?.toString(), notes = mergeNotes(earlier.notes, later.notes)) - ) - periodDao.deletePeriod(later) - return earlier.id + if (before == null || after == null) return null + return mergePeriods(before, after) } /** @@ -227,15 +179,20 @@ class PeriodRepository( } /** - * One-time data fixup: merges period entries that are adjacent — the day right - * after one period's explicit end date is exactly the start of the next — into - * a single entry. + * One-time data fixup: merges period entries that are at most one day + * apart — either touching (the day right after one period's explicit end + * date is exactly the start of the next) or separated by a single + * unlogged day (the next period starts two days after the previous one's + * end) — into a single entry. * - * This repairs periods fragmented by the entry-point bug where logging a day - * right after a period was closed with an explicit end date (rather than left - * ongoing) created a new, disconnected period instead of continuing the - * existing one. Only a one-day gap is merged; larger gaps are left alone since - * they represent a genuinely new cycle, not a fragmented one. + * This repairs periods fragmented by the entry-point bug where logging a + * day right after a period was closed with an explicit end date (rather + * than left ongoing) created a new, disconnected period instead of + * continuing the existing one — including the case where a third day was + * then logged before the gap day ever was, leaving that day orphaned + * between two separate entries. Only gaps of zero or one day are merged; + * larger gaps are left alone since they represent a genuinely new cycle, + * not a fragmented one. * * Periods are processed in start-date order, reusing [mergePeriods] for each * adjacent pair found. @@ -253,7 +210,8 @@ class PeriodRepository( for (next in periods.drop(1)) { val currentEnd = current.endDate?.let { LocalDate.parse(it) } val nextStart = LocalDate.parse(next.startDate) - if (currentEnd != null && nextStart == currentEnd.plusDays(1)) { + val gapDays = currentEnd?.let { ChronoUnit.DAYS.between(it, nextStart) } + if (gapDays != null && gapDays in 1..2) { absorbed.add(next) current = mergePeriods(current, next) } else { diff --git a/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryViewModel.kt b/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryViewModel.kt index 63c76ca..a56b505 100644 --- a/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryViewModel.kt +++ b/app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryViewModel.kt @@ -105,19 +105,17 @@ class HistoryViewModel( /** * Merges [first] and [second] into a single continuous period, combining their - * notes and symptoms. The tracking-log entries (flow, symptoms, pinned - * categories) belonging to whichever period is absorbed are also removed, - * mirroring the cleanup performed on delete — otherwise its flow/symptom - * entry would linger, orphaned, under a date no longer tied to any period. + * notes and symptoms. The absorbed period's own tracking-log entries (flow, + * symptoms, pinned categories) are deliberately left alone — they're still + * valid, dated records of what was logged that day, not orphaned data. They + * were previously deleted here on the assumption that they'd otherwise linger + * unattached, but `deleteLogsForPeriod` was written for actual period + * *deletion* and silently discarded real history that the user asked to + * combine, not remove. */ fun mergePeriods(first: PeriodEntry, second: PeriodEntry) { viewModelScope.launch { - val absorbed = if (first.startDate <= second.startDate) second else first repository.mergePeriods(first, second) - trackingRepository?.deleteLogsForPeriod( - LocalDate.parse(absorbed.startDate), - absorbed.endDate?.let { LocalDate.parse(it) } - ) application?.let { GoFloWidget.updateAllWidgets(it) } } } diff --git a/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeScreen.kt b/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeScreen.kt index 330fe13..2fbddb1 100644 --- a/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeScreen.kt +++ b/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeScreen.kt @@ -113,11 +113,16 @@ fun HomeScreen( // If [date] already falls within an existing period's range (including an // ongoing period, which extends through today), edit that period instead of - // creating a new, overlapping entry. + // creating a new, overlapping entry. If [date] is also the single unlogged + // day between that period's end and a separate, later period's start, + // merge the two immediately so the gap doesn't need a second, manual + // History merge to close. fun navigateToLogPeriod(date: LocalDate) { val existing = PeriodRepository.periodForDate(state.periods, date) if (existing != null) { - onNavigate(Screen.LogPeriod.withId(existing.id, date)) + viewModel.mergeGapAt(date, fallbackId = existing.id) { targetId -> + onNavigate(Screen.LogPeriod.withId(targetId, date)) + } } else { onNavigate(Screen.LogPeriod.newEntryForDate(date)) } diff --git a/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeViewModel.kt b/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeViewModel.kt index f31c4c7..b19816c 100644 --- a/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeViewModel.kt +++ b/app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeViewModel.kt @@ -188,15 +188,13 @@ class HomeViewModel( fun clearSelectedDay() { _selectedDay.value = null } /** - * Resolves which period id logging [date] should target, extending or - * merging adjacent entries as needed (see [PeriodRepository.resolvePeriodForLogging]), - * then hands the result to [onResolved]. A null result means no - * existing/adjacent period applies, so the caller should start a - * brand-new entry for [date]. + * If [date] bridges a one-day gap between two existing periods, merges + * them (see [PeriodRepository.mergeGapAt]) and hands the merged id to + * [onResolved]; otherwise hands back [fallbackId] unchanged. */ - fun resolveLogPeriodTarget(date: LocalDate, onResolved: (Long?) -> Unit) { + fun mergeGapAt(date: LocalDate, fallbackId: Long, onResolved: (Long) -> Unit) { viewModelScope.launch { - onResolved(repository.resolvePeriodForLogging(date)) + onResolved(repository.mergeGapAt(date)?.id ?: fallbackId) } } diff --git a/app/src/test/java/com/mapgie/goflo/data/repository/PeriodRepositoryTest.kt b/app/src/test/java/com/mapgie/goflo/data/repository/PeriodRepositoryTest.kt index f30a5b8..c6f42a8 100644 --- a/app/src/test/java/com/mapgie/goflo/data/repository/PeriodRepositoryTest.kt +++ b/app/src/test/java/com/mapgie/goflo/data/repository/PeriodRepositoryTest.kt @@ -1,11 +1,72 @@ package com.mapgie.goflo.data.repository +import com.mapgie.goflo.data.database.dao.PeriodDao +import com.mapgie.goflo.data.database.dao.SymptomDao import com.mapgie.goflo.data.database.entities.PeriodEntry +import com.mapgie.goflo.data.database.entities.SymptomEntry +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.runBlocking import org.junit.Assert.assertEquals import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test import java.time.LocalDate +/** In-memory [PeriodDao] fake for exercising [PeriodRepository]'s suspend write paths. */ +private class FakePeriodDao : PeriodDao { + val periods = mutableListOf() + private var nextId = 1L + + override fun getAllPeriods(): Flow> = flowOf(periods.sortedByDescending { it.startDate }) + override suspend fun getAllPeriodsOnce(): List = periods.sortedBy { it.startDate } + override fun getPeriodById(id: Long): Flow = flowOf(periods.firstOrNull { it.id == id }) + + override suspend fun insertPeriod(period: PeriodEntry): Long { + val id = if (period.id != 0L) period.id else nextId++ + periods.removeAll { it.id == id } + periods.add(period.copy(id = id)) + return id + } + + override suspend fun updatePeriod(period: PeriodEntry) { + val index = periods.indexOfFirst { it.id == period.id } + if (index >= 0) periods[index] = period + } + + override suspend fun deletePeriod(period: PeriodEntry) { + periods.removeAll { it.id == period.id } + } + + override suspend fun deleteAllPeriods() { periods.clear() } + override suspend fun countPeriods(): Int = periods.size +} + +/** In-memory [SymptomDao] fake for exercising [PeriodRepository]'s suspend write paths. */ +private class FakeSymptomDao : SymptomDao { + val symptoms = mutableListOf() + private var nextId = 1L + + override fun getSymptomsForPeriod(periodId: Long): Flow> = + flowOf(symptoms.filter { it.periodId == periodId }) + override suspend fun getSymptomsForPeriodOnce(periodId: Long): List = + symptoms.filter { it.periodId == periodId } + override suspend fun insertSymptom(symptom: SymptomEntry) { + symptoms.add(symptom.copy(id = nextId++)) + } + override suspend fun deleteSymptomsByPeriodId(periodId: Long) { + symptoms.removeAll { it.periodId == periodId } + } + override suspend fun deleteAllSymptoms() { symptoms.clear() } + override suspend fun getAllSymptoms(): List = symptoms.toList() + override fun getAllSymptomsFlow(): Flow> = flowOf(symptoms.toList()) + override suspend fun bulkRenameSymptoms(oldLabel: String, newLabel: String) { + val renamed = symptoms.map { if (it.symptomType == oldLabel) it.copy(symptomType = newLabel) else it } + symptoms.clear() + symptoms.addAll(renamed) + } +} + class PeriodRepositoryTest { private fun entry(startDate: String, endDate: String? = null, id: Long = 0L) = @@ -177,4 +238,76 @@ class PeriodRepositoryTest { val periods = listOf(entry("2024-01-01", "2024-01-05", id = 1)) assertNull(PeriodRepository.periodForDate(periods, LocalDate.of(2024, 1, 10))) } + + // ── mergeGapAt / mergeAdjacentPeriods ───────────────────────────────────── + + private fun buildRepository(vararg seed: PeriodEntry): Pair { + val periodDao = FakePeriodDao() + seed.forEach { periodDao.periods.add(it) } + return PeriodRepository(periodDao, FakeSymptomDao()) to periodDao + } + + @Test + fun `mergeGapAt merges the two periods flanking a one-day gap`() = runBlocking { + // Reproduces the reported bug: a period ending 28 June and another starting + // 30 June leave the 29th orphaned. Logging the 29th should join them. + val (repo, dao) = buildRepository( + entry("2024-06-26", "2024-06-28", id = 1), + entry("2024-06-30", null, id = 2), + ) + val merged = repo.mergeGapAt(LocalDate.of(2024, 6, 29)) + assertEquals(1L, merged?.id) + assertEquals(1, dao.periods.size) + val survivor = dao.periods.single() + assertEquals("2024-06-26", survivor.startDate) + assertNull(survivor.endDate) // the later (ongoing) period's null end date wins + } + + @Test + fun `mergeGapAt returns null when the date only touches one period`() = runBlocking { + val (repo, dao) = buildRepository(entry("2024-01-01", "2024-01-05", id = 1)) + assertNull(repo.mergeGapAt(LocalDate.of(2024, 1, 6))) + assertEquals(1, dao.periods.size) // untouched — no second period to bridge to + } + + @Test + fun `mergeGapAt returns null when no period precedes the date`() = runBlocking { + val (repo, _) = buildRepository(entry("2024-06-30", null, id = 2)) + assertNull(repo.mergeGapAt(LocalDate.of(2024, 6, 29))) + } + + @Test + fun `mergeAdjacentPeriods merges periods that are touching`() = runBlocking { + val (repo, dao) = buildRepository( + entry("2024-06-01", "2024-06-05", id = 1), + entry("2024-06-06", "2024-06-08", id = 2), + ) + val absorbed = repo.mergeAdjacentPeriods() + assertEquals(listOf(2L), absorbed.map { it.id }) + assertEquals(1, dao.periods.size) + assertEquals("2024-06-08", dao.periods.single().endDate) + } + + @Test + fun `mergeAdjacentPeriods merges periods separated by a single unlogged day`() = runBlocking { + val (repo, dao) = buildRepository( + entry("2024-06-26", "2024-06-28", id = 1), + entry("2024-06-30", "2024-07-02", id = 2), + ) + val absorbed = repo.mergeAdjacentPeriods() + assertEquals(listOf(2L), absorbed.map { it.id }) + assertEquals(1, dao.periods.size) + assertEquals("2024-07-02", dao.periods.single().endDate) + } + + @Test + fun `mergeAdjacentPeriods leaves periods with a gap of more than one day alone`() = runBlocking { + val (repo, dao) = buildRepository( + entry("2024-06-01", "2024-06-05", id = 1), + entry("2024-06-08", "2024-06-12", id = 2), // 2-day gap — a genuinely new cycle + ) + val absorbed = repo.mergeAdjacentPeriods() + assertTrue(absorbed.isEmpty()) + assertEquals(2, dao.periods.size) + } } diff --git a/changelog/unreleased/period-logging-gap-merge-fix.json b/changelog/unreleased/period-logging-gap-merge-fix.json new file mode 100644 index 0000000..c16c152 --- /dev/null +++ b/changelog/unreleased/period-logging-gap-merge-fix.json @@ -0,0 +1,7 @@ +{ + "bump": "patch", + "fixed": [ + "Logging the single unlogged day between two period entries now joins them into one continuous period instead of leaving the gap unfixable without a separate manual merge", + "The one-time period-repair migration and manual History merge no longer delete the absorbed period's own flow/symptom history" + ] +}