fix: DB path, import file picker blackout, and sync registry columns#36
Merged
Conversation
…tion dialog - Store bms_local.sqlite in getApplicationSupportDirectory() instead of Documents/; drift_flutter defaults to Documents on desktop which is wrong for app data. Uses LazyDatabase + NativeDatabase with explicit path. - Invert import DB flow: open NSOpenPanel first, then show confirmation dialog with the selected filename. The previous dialog→NSOpenPanel sequence caused the macOS window to go black and become unresponsive because NSOpenPanel captured Metal render focus mid-animation.
suppliers: removed credit_limit (does not exist in the Drift schema), added payment_terms (nullable text) and is_active (integer/bool). customers: added missing is_active column; kept credit_limit which exists as creditLimit -> credit_limit in SQLite.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR updates database connection setup, refactors the settings import flow to use a preselected file, corrects sync table mappings for customers and suppliers, and adds a new ChangesDatabase, import, sync, and labels
Possibly Related PRs
Suggested Reviewers
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/data/database/app_database.dart`:
- Around line 181-184: Before creating the NativeDatabase in the LazyDatabase
lambda, add a one-time migration check to handle existing users. After obtaining
the directory path and creating the file reference for the new location, check
if a legacy database file exists at the old Documents location
(Documents/bms_local.sqlite). If it exists and the new file does not yet exist,
copy or move the legacy database file from Documents to the new Application
Support location. This ensures existing desktop users retain their data after
the upgrade instead of being booted into a fresh empty database.
In `@lib/data/sync/sync_tables_registry.dart`:
- Line 84: The SyncColumn definitions for is_active are incorrectly using the
_ts timestamp descriptor instead of an integer descriptor, which will cause
wrong serialization/parsing during sync operations. Find both occurrences of
SyncColumn('is_active', _ts) in the sync_tables_registry.dart file and replace
_ts with the appropriate integer descriptor that matches the Drift/MySQL
contract for is_active as an integer flag.
In `@lib/features/settings/presentation/settings_screen.dart`:
- Around line 186-187: The comment on line 186 of the settings_screen.dart file
contains an em-dash character that violates the repository's coding guidelines
for Dart files. Replace the em-dash (`—`) with a regular hyphen (`-`) in the
comment that reads "Open file picker first — avoids the dialog→NSOpenPanel
transition" to comply with the style guide requirement to use hyphens instead of
em-dashes.
- Line 195: The confirmation message in the settings_screen.dart file on line
195 contains a hardcoded English label `'File: '` within the Text widget
content. To maintain internationalization consistency, replace the hardcoded
`'File: '` string with a localized equivalent by accessing `context.l10n`
(similar to how `importDatabaseMessage` is already being localized). You will
need to add a corresponding localization key in your i18n resource files if one
does not already exist (such as `fileLabel` or `filePrefix`), then reference it
in the message construction to ensure all user-facing text is properly
localized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44ff4ab8-47d3-47a3-a1dd-4e562e911bde
📒 Files selected for processing (5)
CHANGELOG.mdlib/data/database/app_database.dartlib/data/sync/sync_tables_registry.dartlib/features/settings/presentation/settings_screen.dartlib/providers/settings_provider.dart
…te localization for file label
Contributor
Preview deployed
Updates automatically on every push. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/data/database/connection_native.dart`:
- Around line 14-19: The legacy migration in connection_native.dart only copies
the main bms_local.sqlite file, so update the migration logic around the
file.existsSync() block in the native database connection setup to also handle
the SQLite WAL sidecars from the legacy location. Use the existing File handling
with legacy and file to copy bms_local.sqlite-wal and bms_local.sqlite-shm when
they exist, or otherwise checkpoint/flush before moving so no committed changes
are lost during migration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 03ac7db3-ffc3-43d7-ad82-2d762b8b679e
📒 Files selected for processing (3)
lib/data/database/app_database.dartlib/data/database/connection_native.dartlib/data/database/connection_web.dart
✅ Files skipped from review due to trivial changes (1)
- lib/data/database/connection_web.dart
hiranyasemindi
approved these changes
Jun 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three fixes: database stored in the wrong directory on macOS, import DB causing a black screen, and MySQL sync failing due to wrong column definitions in the suppliers and customers tables.
Type
Changes
~/Documents/to~/Library/Application Support/lk.getbms.bms/usingLazyDatabase+NativeDatabase.createInBackground()withgetApplicationSupportDirectory()NSOpenPanel(file picker) first, then show confirmation dialog with selected filename - eliminates the dialog-to-native-panel transition that caused the macOS Metal renderer to go black and become unresponsive_supplierssync registry: removecredit_limit(does not exist in the DriftSupplierstable), addpayment_terms(nullable text) andis_active(integer)_customerssync registry: add missingis_activecolumn to match the Drift schemaScreenshots
N/A - no UI changes (import DB UX order changes but looks the same)
Test plan
Specific checks:
bms_local.sqlitecreated at~/Library/Application Support/lk.getbms.bms/bms_local.sqlite, not in~/Documents/no such column: credit_limiterroris_activeandcredit_limitcolumnsRelated issues
N/A
Summary by CodeRabbit
Release Notes
Bug Fixes
is_activeand supplier payment fields).Improvements