Skip to content

fix: macOS persistence, MySQL UUID keys, file picker blackout, and settings card color#35

Merged
iamvirul merged 5 commits into
masterfrom
fix/macos-mysql-fixes
Jun 23, 2026
Merged

fix: macOS persistence, MySQL UUID keys, file picker blackout, and settings card color#35
iamvirul merged 5 commits into
masterfrom
fix/macos-mysql-fixes

Conversation

@iamvirul

@iamvirul iamvirul commented Jun 23, 2026

Copy link
Copy Markdown
Member

Summary

Four fixes bundled together.

1. EULA and license key not persisting across launches

flutter_secure_storage was failing silently on macOS causing password prompts, EULA reset, and license cleared on every restart.

  • Added keychain-access-groups entitlement to DebugProfile.entitlements and Release.entitlements so the license JWT persists correctly
  • Switched EULA acceptance from flutter_secure_storage to a plain marker file under applicationSupportDirectory - EULA state is not sensitive data and does not belong in the Keychain

2. MySQL sync fails with error 1170

SyncColumnType.text mapped to LONGTEXT in DDL. MySQL rejects TEXT columns as primary keys without a key length prefix.

  • Added SyncColumnType.uuid mapping to VARCHAR(36)
  • Updated all 20 sync table descriptors to use uuid for primary keys and all UUID foreign key columns

3. Window goes black when opening Import Database file picker

NSOpenPanel opened before Flutter finished repainting after the confirmation dialog closed.

  • Added 100ms delay between dialog dismiss and FilePicker.pickFiles() to let the renderer settle

4. Backend settings card has a different background color than the text fields inside it

Card used surfaceVariant (#EEF2F7) while TextFields filled with surface (white), creating a visible color mismatch.

  • Switched card to surface + border so the background is uniform

Test plan

  • Fresh launch - EULA appears, accept it, close and reopen - no EULA prompt, no Mac password prompt
  • Enter license key, close and reopen - stays activated
  • Settings > MySQL - Test connection - no error 1170
  • Settings > Import Database - confirm dialog, file picker opens without black screen
  • Settings > Backend section - card background matches text field fill color

Summary by CodeRabbit

Release Notes

  • New Features
    • Added UUID support to synchronization schemas and generated SQL/SQLite handling.
  • Bug Fixes
    • Smoother settings database import flow with improved timing and safety checks.
    • Improved EULA acceptance persistence using a local acceptance marker; acceptance now only records after successful save.
  • Style
    • Refreshed the settings connection card styling with clearer borders.
  • Chores
    • Updated macOS entitlements and secure storage grouping for better secure handling.

iamvirul added 4 commits June 23, 2026 14:29
MySQL rejects TEXT/LONGTEXT as primary key columns without a key length
(error 1170). All id columns and UUID foreign keys (user_id, customer_id,
product_id, etc.) are always 36-char UUIDs, so map them to VARCHAR(36)
via a new SyncColumnType.uuid enum value instead of LONGTEXT.
EULA acceptance now writes a plain marker file under applicationSupportDirectory
instead of using flutter_secure_storage. The Keychain is overkill for non-sensitive
preference data and was causing silent failures (and password prompts) on macOS.

keychain-access-groups entitlement added to both Debug and Release entitlements
so flutter_secure_storage (used for the license JWT) persists correctly across
builds without prompting for the Mac login password each time.
NSOpenPanel captures window focus before Flutter can repaint after a
dialog closes, leaving the window black. A one-frame delay lets the
renderer settle before the native panel opens.
Backend card used surfaceVariant (#EEF2F7) while TextField widgets
filled white (surface), creating a visible colour mismatch inside
the same card. Switch to surface + border to match the field fill.
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Migrates EULA acceptance tracking from flutter_secure_storage to a local marker file, removes the now-unused eulaStorageKey constant, and adds macOS keychain access group entitlements. Configures secure storage providers to use the macOS keychain group. Adds a uuid column type to SyncColumnType and applies it to all ID/foreign-key columns across sync registry tables. Adds a 100ms import delay in SettingsScreen and updates DB connection tile styling.

Changes

EULA File-Based Persistence and macOS Keychain Configuration

Layer / File(s) Summary
Remove eulaStorageKey and add macOS keychain entitlements
lib/core/constants/app_constants.dart, macos/Runner/DebugProfile.entitlements, macos/Runner/Release.entitlements
Removes eulaStorageKey constant from AppConstants. Adds keychain-access-groups entitlement with $(AppIdentifierPrefix)lk.getbms.bms to both macOS entitlement files.
EulaNotifier file-based persistence
lib/providers/eula_provider.dart
Replaces flutter_secure_storage with dart:io/path_provider. build() checks for .eula_accepted marker file existence (existsSync); accept() writes a UTC ISO-8601 timestamp to that file and sets state to AsyncData(true). _markerFile() resolves path from getApplicationSupportDirectory().
Configure macOS keychain access group for secure storage
lib/providers/auth_provider.dart, lib/providers/sync_provider.dart
Updates secureStorageProvider and SyncNotifier._storage to configure MacOsOptions(groupId: 'lk.getbms.bms'), enabling shared keychain access across secure storage instances.

Sync Schema UUID Column Type

Layer / File(s) Summary
SyncColumnType.uuid variant
lib/data/sync/sync_table.dart
Adds uuid to the SyncColumnType enum, maps it to VARCHAR(36) for MySQL DDL, and returns the raw string value in parseFromMysql.
Apply uuid type across sync registry
lib/data/sync/sync_tables_registry.dart
Introduces _uuid column constant and changes _pk to use it. Updates all ID/foreign-key columns across 17 tables (products, stock, customer_payments, supplier_payments, purchase_orders, purchases, purchase_items, purchase_order_items, cheques, petty_cash, invoices, invoice_items, no_invoice_sales, sales_returns, return_items, audit_log, stock_movements).

Settings Screen Minor Fixes

Layer / File(s) Summary
Import delay and tile styling
lib/features/settings/presentation/settings_screen.dart
_importDb adds a 100ms Future.delayed with context.mounted guard before invoking the JSON import. DB connection tile container switches to AppColors.surface background with an explicit AppColors.border border.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • hiranyasemindi

Poem

🐇 Hop hop, no more keychain stress,
A tiny file seals the EULA's address.
UUIDs march through every table in line,
VARCHAR(36) — oh, how they shine!
The import waits a hundred ms more,
While this bunny tidies up the store. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main changes: macOS persistence fixes, MySQL UUID column mapping, file picker UI issue, and settings card color correction.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description check ✅ Passed PR description is comprehensive with clear summary, structured fixes, detailed test plan, and specific implementation details addressing all four issues.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/macos-mysql-fixes

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Preview deployed

URL https://getbms.github.io/bms/pr-35/
Commit 926f717
Status Live

Updates automatically on every push.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/data/sync/sync_table.dart (1)

17-22: 🩺 Stability & Availability | 🟠 Major

Add ALTER TABLE logic to migrate existing MySQL sync tables, or confirm CREATE TABLE always failed with old LONGTEXT schema.

The _ensureSchema method uses CREATE TABLE IF NOT EXISTS, which won't modify existing tables. If MySQL instances have these tables with the old LONGTEXT schema, the VARCHAR(36) fix won't apply on upgrade—FK/PK columns would remain LONGTEXT and the error would persist. The Drift (SQLite) database has proper versioning and migration logic, but the MySQL sync tables lack an equivalent ALTER path. Either:

  1. Add ALTER TABLE statements to upgrade existing tables, or
  2. Confirm that the original LONGTEXT configuration always caused CREATE TABLE to fail, ensuring no stale tables exist in production deployments.
🤖 Prompt for 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.

In `@lib/data/sync/sync_table.dart` around lines 17 - 22, The `mysqlType` getter
has been updated to return VARCHAR(36) for UUID columns, but the `_ensureSchema`
method uses CREATE TABLE IF NOT EXISTS which only creates new tables and doesn't
modify existing ones. To ensure the schema fix applies to existing MySQL sync
tables that may have the old LONGTEXT schema, you need to either add ALTER TABLE
migration logic in the `_ensureSchema` method to update the column types for
UUID, text, integer, and real columns from their old definitions to the new
types defined in the `mysqlType` getter, or verify and document that the
original LONGTEXT configuration always caused the CREATE TABLE operation to
fail, ensuring no stale tables exist in production.
🤖 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/providers/eula_provider.dart`:
- Around line 7-8: In the comment block for EULA acceptance in the
eula_provider.dart file, replace the em-dash (—) character with a regular hyphen
(-) in the phrase "EULA acceptance is not sensitive data — store as a plain
marker file rather". This aligns with the coding guideline that disallows
em-dashes in comments.
- Around line 11-20: Add error handling to the build() and accept() methods in
the EulaProvider class. In the build() method, wrap the file I/O operations (the
call to _markerFile() and existsSync()) in a try/catch block that returns false
when any exception occurs, treating storage failures as "not yet accepted"
rather than crashing. In the accept() method, wrap the _markerFile() call and
writeAsString() operation in a try/catch block to prevent state changes when the
file write fails, ensuring the caller can handle write failures appropriately
instead of silently succeeding with an inconsistent state.

In `@macos/Runner/Release.entitlements`:
- Around line 7-10: The keychain access group defined in the
Release.entitlements file is not being used by flutter_secure_storage, causing
Keychain operations to fail silently or with permission errors. In
auth_provider.dart, modify the MacOsOptions to include groupId: 'lk.getbms.bms'
(ensuring it matches the entitlements entry without the $(AppIdentifierPrefix)
prefix). In sync_provider.dart, update the FlutterSecureStorage() instantiation
to pass MacOsOptions with the same groupId. This ensures both the auth provider
and sync provider use the configured keychain access group consistently with the
entitlements definition.

---

Outside diff comments:
In `@lib/data/sync/sync_table.dart`:
- Around line 17-22: The `mysqlType` getter has been updated to return
VARCHAR(36) for UUID columns, but the `_ensureSchema` method uses CREATE TABLE
IF NOT EXISTS which only creates new tables and doesn't modify existing ones. To
ensure the schema fix applies to existing MySQL sync tables that may have the
old LONGTEXT schema, you need to either add ALTER TABLE migration logic in the
`_ensureSchema` method to update the column types for UUID, text, integer, and
real columns from their old definitions to the new types defined in the
`mysqlType` getter, or verify and document that the original LONGTEXT
configuration always caused the CREATE TABLE operation to fail, ensuring no
stale tables exist in production.
🪄 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: 8bac5965-1fc0-4886-b881-9f8d9174f2bc

📥 Commits

Reviewing files that changed from the base of the PR and between d26ca9b and 2153144.

📒 Files selected for processing (7)
  • lib/core/constants/app_constants.dart
  • lib/data/sync/sync_table.dart
  • lib/data/sync/sync_tables_registry.dart
  • lib/features/settings/presentation/settings_screen.dart
  • lib/providers/eula_provider.dart
  • macos/Runner/DebugProfile.entitlements
  • macos/Runner/Release.entitlements
💤 Files with no reviewable changes (1)
  • lib/core/constants/app_constants.dart

Comment thread lib/providers/eula_provider.dart Outdated
Comment thread lib/providers/eula_provider.dart
Comment thread macos/Runner/Release.entitlements
@iamvirul iamvirul requested a review from hiranyasemindi June 23, 2026 13:16
- Replace em-dash with hyphen in eula_provider.dart comment
- Add try/catch to EulaNotifier.build() and accept(); accept() no longer
  updates state when the file write fails
- Add groupId: 'lk.getbms.bms' to MacOsOptions in auth_provider.dart to
  match the keychain-access-groups entitlement
- Add MacOsOptions(groupId: 'lk.getbms.bms') to SyncNotifier._storage so
  sync provider uses the same keychain access group

Skipped: ALTER TABLE migration for UUID columns - CREATE TABLE with a
LONGTEXT primary key always threw error 1170, so no stale tables exist.
@iamvirul iamvirul merged commit a3f066d into master Jun 23, 2026
11 checks passed
@iamvirul iamvirul deleted the fix/macos-mysql-fixes branch June 23, 2026 15:31
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.

2 participants