From a7418a4d8f5284d9c0751eb99d48eb52ec6f5fe3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 29 Jun 2026 11:39:32 +0000 Subject: [PATCH] Fix three period entry bugs: backup export labels, delete cleanup, flow save Bug 1: Full backup export was missing scaleLabels and slider-scale status for numeric slider categories. Added scaleLabels param to addCategory() and updateCategoryFullSettings() in TrackingRepository, and wired it through the import path in SettingsViewModel.importCategoryConfig(). Bug 2: Deleting a period did not remove its associated tracking logs (flow, symptoms, pinned category logs). Added deleteLogsForCategoryInRange() and deleteLogsForCategoryOnDate() to TrackingLogDao, a deleteLogsForPeriod() helper to TrackingRepository, and called it from both delete paths: LogPeriodViewModel.delete() and HistoryViewModel.stageDeletion(). The HistoryViewModel.Factory now accepts and forwards a TrackingRepository. Bug 3: LogPeriodViewModel.save() was writing flowLevel = "" instead of state.selectedFlowLabel to PeriodEntry, breaking export and doctor-visit display. Fixed the save call. Added a one-time reverse migration (runFlowLevelRestoreIfNeeded) that reads the flow TrackingLog for each period with a blank flowLevel and writes the resolved label back, repairing historic data. Guarded by a new flowLevelRestoreDone preference flag. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01SYUviWXvsmCQkRGLrRjgD1 --- LESSONS.md | 3 ++ .../java/com/mapgie/goflo/GoFloApplication.kt | 39 +++++++++++++++++++ .../java/com/mapgie/goflo/MainActivity.kt | 2 +- .../goflo/data/database/dao/TrackingLogDao.kt | 8 ++++ .../data/preferences/ReminderPreferences.kt | 12 ++++++ .../goflo/data/repository/PeriodRepository.kt | 5 +++ .../data/repository/TrackingRepository.kt | 27 +++++++++++++ .../ui/screens/history/HistoryViewModel.kt | 10 ++++- .../ui/screens/log/LogPeriodViewModel.kt | 6 ++- .../ui/screens/settings/SettingsViewModel.kt | 2 + changelog/unreleased/period-entry-bugs.json | 9 +++++ 11 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 changelog/unreleased/period-entry-bugs.json diff --git a/LESSONS.md b/LESSONS.md index f9426d8..2e3275a 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -122,6 +122,9 @@ In a Compose Navigation graph with a bottom nav bar, navigating between tabs des **SQL `:param IS NULL` in a parameterised `OR` condition matches every row when the param is null** A Room DAO query like `AND (:id IS NULL OR col = :id)` evaluates to `AND TRUE` when `:id` is null, returning all rows rather than only those where `col IS NULL`. This silently broadens the result set in ways that are hard to spot in testing. Fix by splitting into separate query methods — one for the null case and one for the non-null case — or handle the branch in application code before calling the DAO. +**When a save bug corrupts a primary field, use its mirror store as the repair source** +If a field is mirrored to a secondary store (e.g. `PeriodEntry.flowLevel` is also written to `TrackingLog`), a bug that writes a blank value to the primary record leaves the secondary intact. The forward backfill that populates the secondary from the primary will skip blank rows, so the secondary stays correct. A one-time reverse migration — reading the secondary and writing back to the primary — is the right repair: it is idempotent (skip if already non-blank), guarded by a preference flag, and requires no schema change. When mirroring data across stores, make sure the secondary write path is independent of the primary so it succeeds even when the primary is corrupted. + **Non-reactive suspend calls inside a `combine` lambda silently break reactivity** Calling a `suspend` DAO function inside a `combine { }` transform reads data once at emission time and never again. If the queried table changes, the outer flow won't re-emit. Fix: promote the query to a `Flow` and include it as an additional `combine` argument so the pipeline re-fires on every table change. diff --git a/app/src/main/java/com/mapgie/goflo/GoFloApplication.kt b/app/src/main/java/com/mapgie/goflo/GoFloApplication.kt index 936bdec..5d70078 100644 --- a/app/src/main/java/com/mapgie/goflo/GoFloApplication.kt +++ b/app/src/main/java/com/mapgie/goflo/GoFloApplication.kt @@ -61,6 +61,7 @@ class GoFloApplication : Application() { appScope.launch { runFlowBackfillIfNeeded() } appScope.launch { runSymptomsBackfillIfNeeded() } appScope.launch { runPeriodOverlapMergeIfNeeded() } + appScope.launch { runFlowLevelRestoreIfNeeded() } } /** @@ -129,6 +130,44 @@ class GoFloApplication : Application() { preferencesStore.setSymptomsBackfillDone(true) } + /** + * One-time reverse migration: reads each period's flow TrackingLog entry and writes + * the resolved label back into PeriodEntry.flowLevel for periods that were saved with + * an empty flowLevel (due to the save bug that wrote "" instead of the selected label). + * + * Numeric slider values ("1"–"4") are mapped back to their text labels + * ("Spotting"/"Light"/"Medium"/"Heavy") so PeriodEntry.flowLevel is always a label. + * Periods that already have a non-blank flowLevel are skipped. + */ + private suspend fun runFlowLevelRestoreIfNeeded() { + val prefs = preferencesStore.preferences.first() + if (prefs.flowLevelRestoreDone) return + + val flowCategory = trackingRepository.getSystemCategoryByKey("flow") ?: run { + preferencesStore.setFlowLevelRestoreDone(true) + return + } + + val periods = repository.getAllPeriodsOnce() + for (period in periods) { + if (period.flowLevel.isNotBlank()) continue + val startDate = runCatching { LocalDate.parse(period.startDate) }.getOrNull() ?: continue + val logValue = trackingRepository.getExistingLog(startDate, flowCategory.id) + ?.values?.firstOrNull() ?: continue + val label = when (logValue) { + "1" -> "Spotting" + "2" -> "Light" + "3" -> "Medium" + "4" -> "Heavy" + else -> logValue + } + if (label.isBlank()) continue + repository.updateFlowLevel(period, label) + } + + preferencesStore.setFlowLevelRestoreDone(true) + } + /** * One-time data fixup: merges period entries whose date ranges overlap * (e.g. a new period logged for a date already covered by an ongoing period diff --git a/app/src/main/java/com/mapgie/goflo/MainActivity.kt b/app/src/main/java/com/mapgie/goflo/MainActivity.kt index c13d57a..2d515b6 100644 --- a/app/src/main/java/com/mapgie/goflo/MainActivity.kt +++ b/app/src/main/java/com/mapgie/goflo/MainActivity.kt @@ -298,7 +298,7 @@ private fun MainNavHost(app: GoFloApplication, currentTheme: AppTheme, pendingCa } composable(Screen.History.route) { - val vm: HistoryViewModel = viewModel(factory = HistoryViewModel.Factory(app.repository, app)) + val vm: HistoryViewModel = viewModel(factory = HistoryViewModel.Factory(app.repository, app, app.trackingRepository)) HistoryScreen(viewModel = vm, onNavigate = { navController.navigate(it) }) } diff --git a/app/src/main/java/com/mapgie/goflo/data/database/dao/TrackingLogDao.kt b/app/src/main/java/com/mapgie/goflo/data/database/dao/TrackingLogDao.kt index e3ff25e..9d3f4ee 100644 --- a/app/src/main/java/com/mapgie/goflo/data/database/dao/TrackingLogDao.kt +++ b/app/src/main/java/com/mapgie/goflo/data/database/dao/TrackingLogDao.kt @@ -110,6 +110,14 @@ interface TrackingLogDao { @Query("SELECT MAX(date) FROM tracking_logs") suspend fun getLatestLogDate(): String? + /** Removes all logs for [categoryId] whose date falls within the inclusive range [startDate]..[endDate]. Values cascade via FK. */ + @Query("DELETE FROM tracking_logs WHERE categoryId = :categoryId AND date >= :startDate AND date <= :endDate") + suspend fun deleteLogsForCategoryInRange(categoryId: Long, startDate: String, endDate: String) + + /** Removes the log(s) for [categoryId] on exactly [date]. Values cascade via FK. */ + @Query("DELETE FROM tracking_logs WHERE categoryId = :categoryId AND date = :date") + suspend fun deleteLogsForCategoryOnDate(categoryId: Long, date: String) + /** Permanently removes every tracking log row (values cascade via FK). */ @Query("DELETE FROM tracking_logs") suspend fun deleteAllLogs() diff --git a/app/src/main/java/com/mapgie/goflo/data/preferences/ReminderPreferences.kt b/app/src/main/java/com/mapgie/goflo/data/preferences/ReminderPreferences.kt index a48e0d9..3560006 100644 --- a/app/src/main/java/com/mapgie/goflo/data/preferences/ReminderPreferences.kt +++ b/app/src/main/java/com/mapgie/goflo/data/preferences/ReminderPreferences.kt @@ -102,6 +102,12 @@ data class AppPreferences( val heatmapZoomLevel: Int = 1, /** True once the new-user onboarding banner has been dismissed. */ val onboardingBannerDismissed: Boolean = false, + /** + * True once the one-time reverse migration that restores PeriodEntry.flowLevel from + * TrackingLog data has been completed. Repairs periods saved while the flow-save bug + * was active (flowLevel was being written as ""). + */ + val flowLevelRestoreDone: Boolean = false, /** Hue (0–360°) for the primary colour in the custom theme. */ val customPrimaryHue: Float = 0f, /** Hue (0–360°) for the secondary colour in the custom theme. */ @@ -174,6 +180,7 @@ class AppPreferencesStore(private val context: Context) { val HEATMAP_AGGREGATION = stringPreferencesKey("heatmap_aggregation") val HEATMAP_ZOOM_LEVEL = intPreferencesKey("heatmap_zoom_level") val ONBOARDING_BANNER_DISMISSED = booleanPreferencesKey("onboarding_banner_dismissed") + val FLOW_LEVEL_RESTORE_DONE = booleanPreferencesKey("flow_level_restore_done") val CUSTOM_PRIMARY_HUE = floatPreferencesKey("custom_primary_hue") val CUSTOM_SECONDARY_HUE = floatPreferencesKey("custom_secondary_hue") val CUSTOM_TERTIARY_HUE = floatPreferencesKey("custom_tertiary_hue") @@ -219,6 +226,7 @@ class AppPreferencesStore(private val context: Context) { heatmapAggregation = prefs[Keys.HEATMAP_AGGREGATION] ?: "AVERAGE", heatmapZoomLevel = prefs[Keys.HEATMAP_ZOOM_LEVEL] ?: 1, onboardingBannerDismissed = prefs[Keys.ONBOARDING_BANNER_DISMISSED] ?: false, + flowLevelRestoreDone = prefs[Keys.FLOW_LEVEL_RESTORE_DONE] ?: false, customPrimaryHue = prefs[Keys.CUSTOM_PRIMARY_HUE] ?: 0f, customSecondaryHue = prefs[Keys.CUSTOM_SECONDARY_HUE] ?: 200f, customTertiaryHue = prefs[Keys.CUSTOM_TERTIARY_HUE] ?: 330f, @@ -397,6 +405,10 @@ class AppPreferencesStore(private val context: Context) { context.dataStore.edit { it[Keys.ONBOARDING_BANNER_DISMISSED] = dismissed } } + suspend fun setFlowLevelRestoreDone(done: Boolean) { + context.dataStore.edit { it[Keys.FLOW_LEVEL_RESTORE_DONE] = done } + } + suspend fun setCustomPrimaryHue(hue: Float) { context.dataStore.edit { it[Keys.CUSTOM_PRIMARY_HUE] = hue } } 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 209bb36..98ac1bb 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 @@ -65,6 +65,11 @@ class PeriodRepository( periodDao.deletePeriod(entry) } + /** Updates only the flowLevel field of a period without touching its symptoms. */ + suspend fun updateFlowLevel(period: PeriodEntry, flowLevel: String) { + periodDao.updatePeriod(period.copy(flowLevel = flowLevel)) + } + /** * 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/data/repository/TrackingRepository.kt b/app/src/main/java/com/mapgie/goflo/data/repository/TrackingRepository.kt index 3b484e0..87b6d15 100644 --- a/app/src/main/java/com/mapgie/goflo/data/repository/TrackingRepository.kt +++ b/app/src/main/java/com/mapgie/goflo/data/repository/TrackingRepository.kt @@ -62,6 +62,7 @@ class TrackingRepository( numericMax: Float = 10f, allowDecimals: Boolean = false, numericUnit: String = "", + scaleLabels: String = "", allowMultiple: Boolean = false, showInLogPeriod: Boolean = false, trackAgainstTime: Boolean = false, @@ -80,6 +81,7 @@ class TrackingRepository( numericMax = numericMax, allowDecimals = allowDecimals, numericUnit = numericUnit, + scaleLabels = scaleLabels, allowMultiple = allowMultiple, showInLogPeriod = showInLogPeriod, trackAgainstTime = trackAgainstTime, @@ -120,6 +122,7 @@ class TrackingRepository( numericMax: Float, allowDecimals: Boolean, numericUnit: String = "", + scaleLabels: String = "", allowMultiple: Boolean = false, showInLogPeriod: Boolean = false, trackAgainstTime: Boolean = false, @@ -136,6 +139,7 @@ class TrackingRepository( numericMax = numericMax, allowDecimals = allowDecimals, numericUnit = numericUnit, + scaleLabels = scaleLabels, allowMultiple = allowMultiple, showInLogPeriod = showInLogPeriod, trackAgainstTime = trackAgainstTime, @@ -509,6 +513,29 @@ class TrackingRepository( categoryDao.unarchiveAllSystemCategories() } + /** + * Deletes all tracking logs associated with a period's date range. + * + * Flow logs are deleted for every day from [startDate] to [endDate] (inclusive) + * because the backfill migration may have created one per day. + * Symptoms and pinned-category logs are deleted only for [startDate], which is + * the only date the Log Period screen writes to for those categories. + */ + suspend fun deleteLogsForPeriod(startDate: LocalDate, endDate: LocalDate?) { + val end = endDate ?: startDate + val flowCategory = getSystemCategoryByKey("flow") + val symptomsCategory = getSystemCategoryByKey("symptoms") + if (flowCategory != null) { + logDao.deleteLogsForCategoryInRange(flowCategory.id, startDate.toString(), end.toString()) + } + if (symptomsCategory != null) { + logDao.deleteLogsForCategoryOnDate(symptomsCategory.id, startDate.toString()) + } + for (cat in categoryDao.getShowInLogPeriodCategoriesOnce().filter { !it.isSystem }) { + logDao.deleteLogsForCategoryOnDate(cat.id, startDate.toString()) + } + } + /** * Ensures a TrackingLog + TrackingLogValue exists for each date in [dates] * under [flowCategoryId] with value label [flowLabel]. 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 77463b0..406acef 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 @@ -8,6 +8,8 @@ import com.mapgie.goflo.widget.GoFloWidget import com.mapgie.goflo.data.database.entities.PeriodEntry import com.mapgie.goflo.data.database.entities.SymptomEntry import com.mapgie.goflo.data.repository.PeriodRepository +import com.mapgie.goflo.data.repository.TrackingRepository +import java.time.LocalDate import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow @@ -25,6 +27,7 @@ data class PeriodWithSymptoms( class HistoryViewModel( private val repository: PeriodRepository, private val application: Application? = null, + private val trackingRepository: TrackingRepository? = null, ) : ViewModel() { // ── Pending-delete state ────────────────────────────────────────────────── @@ -68,6 +71,10 @@ class HistoryViewModel( val symptoms = repository.getSymptomsParsed(period.id) pendingUndo[period.id] = UndoData(period, symptoms) _pendingDeleteIds.update { it + period.id } + trackingRepository?.deleteLogsForPeriod( + LocalDate.parse(period.startDate), + period.endDate?.let { LocalDate.parse(it) } + ) repository.deletePeriod(period) application?.let { GoFloWidget.updateAllWidgets(it) } } @@ -99,10 +106,11 @@ class HistoryViewModel( class Factory( private val repository: PeriodRepository, private val application: Application? = null, + private val trackingRepository: TrackingRepository? = null, ) : ViewModelProvider.Factory { override fun create(modelClass: Class): T { @Suppress("UNCHECKED_CAST") - return HistoryViewModel(repository, application) as T + return HistoryViewModel(repository, application, trackingRepository) as T } } } diff --git a/app/src/main/java/com/mapgie/goflo/ui/screens/log/LogPeriodViewModel.kt b/app/src/main/java/com/mapgie/goflo/ui/screens/log/LogPeriodViewModel.kt index 3998671..e7860c3 100644 --- a/app/src/main/java/com/mapgie/goflo/ui/screens/log/LogPeriodViewModel.kt +++ b/app/src/main/java/com/mapgie/goflo/ui/screens/log/LogPeriodViewModel.kt @@ -270,7 +270,7 @@ class LogPeriodViewModel( id = state.existingId ?: 0, startDate = state.startDate.toString(), endDate = state.endDate?.toString(), - flowLevel = "", + flowLevel = state.selectedFlowLabel, notes = state.notes ) if (state.isEditing && state.existingId != null) { @@ -384,6 +384,10 @@ class LogPeriodViewModel( viewModelScope.launch { try { val period = repository.getPeriodById(id).first() ?: return@launch + trackingRepository?.deleteLogsForPeriod( + LocalDate.parse(period.startDate), + period.endDate?.let { LocalDate.parse(it) } + ) repository.deletePeriod(period) application?.let { GoFloWidget.updateAllWidgets(it) } _uiState.update { it.copy(deleted = true) } diff --git a/app/src/main/java/com/mapgie/goflo/ui/screens/settings/SettingsViewModel.kt b/app/src/main/java/com/mapgie/goflo/ui/screens/settings/SettingsViewModel.kt index 27c7def..7089ab1 100644 --- a/app/src/main/java/com/mapgie/goflo/ui/screens/settings/SettingsViewModel.kt +++ b/app/src/main/java/com/mapgie/goflo/ui/screens/settings/SettingsViewModel.kt @@ -860,6 +860,7 @@ class SettingsViewModel( numericMax = catObj.optDouble("numericMax", 10.0).toFloat(), allowDecimals = catObj.optBoolean("allowDecimals", false), numericUnit = catObj.optString("numericUnit", ""), + scaleLabels = catObj.optString("scaleLabels", ""), allowMultiple = catObj.optBoolean("allowMultiple", false), showInLogPeriod = catObj.optBoolean("showInLogPeriod", false), trackAgainstTime = catObj.optBoolean("trackAgainstTime", false), @@ -879,6 +880,7 @@ class SettingsViewModel( numericMax = catObj.optDouble("numericMax", category.numericMax.toDouble()).toFloat(), allowDecimals = catObj.optBoolean("allowDecimals", category.allowDecimals), numericUnit = catObj.optString("numericUnit", category.numericUnit), + scaleLabels = catObj.optString("scaleLabels", category.scaleLabels), allowMultiple = catObj.optBoolean("allowMultiple", category.allowMultiple), showInLogPeriod = catObj.optBoolean("showInLogPeriod", category.showInLogPeriod), trackAgainstTime = catObj.optBoolean("trackAgainstTime", category.trackAgainstTime), diff --git a/changelog/unreleased/period-entry-bugs.json b/changelog/unreleased/period-entry-bugs.json new file mode 100644 index 0000000..ff9192c --- /dev/null +++ b/changelog/unreleased/period-entry-bugs.json @@ -0,0 +1,9 @@ +{ + "bump": "patch", + "fixed": [ + "Full backup export now includes scale labels and slider-scale status for numeric slider categories", + "Deleting a period entry now also removes its associated flow, symptoms, and pinned category tracking logs", + "Flow level is now correctly saved to the period record when logging a period", + "Historic period entries with missing flow levels are repaired on app start using tracking log data" + ] +}