Skip to content

Deleting a medication keeps its dose history - #29

Merged
SusieeTheLinuxUser merged 1 commit into
masterfrom
fix/keep-history-on-delete
Nov 7, 2026
Merged

SusieeTheLinuxUser merged 1 commit into
masterfrom
fix/keep-history-on-delete

Conversation

@SusieeTheLinuxUser

@SusieeTheLinuxUser SusieeTheLinuxUser commented Nov 7, 2026 •

Copy link
Copy Markdown
Owner

What

The delete dialog says "Its upcoming alarms will be cancelled. Past history is kept." It wasn't true: deleteMedication did a hard DELETE, and doses has ON DELETE CASCADE with foreign keys on, so every taken/missed dose, fired_at and the adherence-grid history for that medication was erased. The next automatic backup then saved the loss.

Now deleting is a soft delete:

  • medications.deleted column, DB version 2 → 3 (ALTER TABLE ... ADD COLUMN, default 0). deleteMedication sets deleted = 1, active = 0 (lib/data/database.dart).
  • medications() hides deleted ones unless includeDeleted: true. Backups use it, and write/read the flag. Old backup files without it still load as not deleted (backup_format.dart, backup_service.dart).
  • After deleting, the medications page runs scheduler.sync(). The orphan pass then cancels and removes the med's upcoming pending doses, which the cascade used to do. Past doses stay.
  • The "doses missed" notification skips deleted meds. The Today screen hides them, so that notification could never be cleared otherwise (scheduler.dart).
  • AGENTS.md notes the soft delete.

Merges cleanly with #28 (checked locally).

Tests

  • New: keeps a soft-deleted medication and its dose history through a round-trip, a backup from before the deleted flag decodes as not deleted (test/backup_test.dart).
  • TZ=Europe/Warsaw flutter test: 38 passed. flutter analyze: clean.
  • No database test setup exists (sqflite runs on a device only), so the delete itself and the v2 → v3 upgrade aren't unit-tested. Worth one try on a phone or emulator before merging: update over the old version, delete a med that has taken doses, check the history grid still counts them.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Deleted medications are now hidden from medication lists while their dose history is retained.
    • Backups preserve deleted medications and their dose history, including when restored from older backups.
  • Bug Fixes

    • Notifications no longer include missed doses for medications that have been deleted.
    • Scheduled reminders are refreshed after a medication is deleted.

The confirm dialog promised "Past history is kept", but deleteMedication
did a hard DELETE and ON DELETE CASCADE removed every dose with it:
taken/missed history, fired_at and the adherence grid, saved into the
next backup too.

Now it's a soft delete: medications.deleted (DB v3), set together with
active = 0. Deleted meds are hidden from the user-facing lists, kept in
backups (old files without the flag still load), and their upcoming
pending doses are still cancelled through the scheduler's orphan pass.
The missed-dose notification skips deleted meds, since the Today screen
can't clear those.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-07T00:59:59.711942Z
We couldn’t complete this review. Comment @codex review to try again.
5e8e305 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Medication deletion now marks records inactive and deleted while retaining dose history. Default medication queries exclude deleted records, and backups preserve deleted medications and their dose history. Scheduler synchronization follows deletion and filters overdue doses whose medication is absent.

Changes: Medication Retention

Layer / File(s) Summary
Soft-delete storage and reconciliation
lib/models/medication.dart, lib/data/database.dart, lib/ui/medications_page.dart, lib/services/scheduler.dart, AGENTS.md
The medication model and database store a deletion flag. Deletion marks a medication inactive and deleted instead of removing its row. Default queries exclude deleted records. The medications page synchronizes the scheduler after deletion, and the scheduler excludes overdue doses whose medication is absent.
Backup and restore deleted records
lib/services/backup_format.dart, lib/services/backup_service.dart, lib/data/database.dart, test/backup_test.dart
Backups include deleted medications, and restore preserves their deletion flags and dose history. Decoding defaults a missing or null deletion flag to false. Tests cover round-trip preservation and older backups without the flag.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant MedicationsPage
  participant DoseDatabase
  participant DoseScheduler
  MedicationsPage->>DoseDatabase: Mark medication deleted and inactive
  MedicationsPage->>DoseScheduler: Synchronize after deletion
  DoseScheduler->>DoseDatabase: Load medications and overdue doses
Loading

Possibly related PRs

Suggested reviewers: claude

Merge Risk: Minimal · up to 5e8e3

No confirmed issue blocks merging. Validate the database upgrade and deletion flow on a device or emulator as planned.

Security architecture risk: Low · up to 5e8e3

The change intentionally preserves sensitive medication history through deletion and backup. The identified compatibility risk is that restoring a new backup into an older app loses the deletion marker. Restore remains user-confirmed, and normally deleted medications remain inactive. Interrupted or overlapping alarm reconciliation still needs device-level validation.

Retained concerns

  • Low · architecture · inferred: New backups still advertise format version 1, which the PR-base app accepts without reading the deletion marker. A user-confirmed restore into that older app therefore makes normally deleted medications visible again as inactive records. This loses the new lifecycle state during cross-version restore; whether older-reader compatibility is supported is unspecified.
Security review details

Security Blast Radius

  • observed — The changed retained-data scope covers the local medication database and complete backups written through the existing user-selected folder flow. Future snapshots continue carrying deleted records rather than allowing ordinary deletion to remove them from subsequent backups.

Trust Boundaries and Controls

  • observed — Selected backup text crosses into authoritative local state only through decoding and confirmed whole-database restore. Decoding checks structure, medication references, and the deletion flag's boolean type, but does not authenticate file contents. A modified file can change deletion or activity state; editable schedules and history were already trusted by this restore flow before the PR.

Resilience and Maintainability Implications

  • inferred — Database state and native alarm cancellation are separate transition owners. Cancellation precedes the database update, and synchronization arms alarms from an earlier medication snapshot without serialization. Both patterns existed at base; this PR adds immediate reconciliation but does not make the transition atomic. Startup and resume refresh provide eventual reconciliation, not proof of correctness under every interruption or overlap.

Hardening Proposals

  • proposed — If backups may be restored into older app versions, define a format-version or compatibility policy that prevents silent loss of deletion state. Separately, serializing medication lifecycle changes with alarm reconciliation would strengthen cancellation guarantees; targeted interruption and overlap checks could validate that no deleted owner regains an alarm.
Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main change: medication deletion preserves dose history.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (8 skipped: 8 unsupported.)

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a2937c8d-630d-4e70-93ad-ecdaa0c76635
Commits

Reviewing files that changed from the base of the PR and between 6fd5787 and 5e8e305.

Files selected for processing: 8

  • AGENTS.md
  • lib/data/database.dart
  • lib/models/medication.dart
  • lib/services/backup_format.dart
  • lib/services/backup_service.dart
  • lib/services/scheduler.dart
  • lib/ui/medications_page.dart
  • test/backup_test.dart

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

@SusieeTheLinuxUser
SusieeTheLinuxUser merged commit 4d58f9e into master Nov 7, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant