Phase 7: MySQL sync, AI tamper resistance, unit test coverage, and bug fixes#25
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a bidirectional SQLite↔MySQL sync engine using a 30-second periodic ChangesBMS MySQL Sync Engine, Schema v8, Licensing Hardening & Unit Test Suite
Sequence Diagram(s)sequenceDiagram
participant Settings UI
participant SyncNotifier
participant FlutterSecureStorage
participant SyncService
participant SQLite
participant MySQL
rect rgba(70, 130, 180, 0.5)
Note over SyncNotifier: MySQL backend detected
SyncNotifier->>SyncNotifier: _schedulePeriodicSync (30s timer)
SyncNotifier->>SyncNotifier: immediate Future.microtask → _runSync
end
rect rgba(60, 179, 113, 0.5)
Note over SyncNotifier,MySQL: _runSync execution
SyncNotifier->>FlutterSecureStorage: read lastPushAt, lastPullAt
alt already syncing
SyncNotifier-->>SyncNotifier: return early (prevent overlap)
else not syncing
SyncNotifier->>SyncNotifier: state = syncing
SyncNotifier->>SyncService: sync(settings, lastPushAt, lastPullAt)
SyncService->>MySQL: connect + _ensureSchema
loop each SyncTable in kSyncTables
SyncService->>SQLite: SELECT WHERE updated_at > lastPushAt
SyncService->>MySQL: INSERT ON DUPLICATE KEY UPDATE
alt not pushOnly
SyncService->>MySQL: SELECT WHERE updated_at > lastPullAt
SyncService->>SQLite: INSERT OR REPLACE
end
end
SyncService-->>SyncNotifier: SyncResult(pushed, pulled, errors)
alt no errors
SyncNotifier->>FlutterSecureStorage: save lastPushAt = now, lastPullAt = now
SyncNotifier->>SyncNotifier: state = success
else has errors
SyncNotifier->>SyncNotifier: state = error
end
SyncNotifier->>Settings UI: emit new SyncState
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Preview deployed
Updates automatically on every push. |
![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: 17
🧹 Nitpick comments (3)
CLAUDE.md (1)
26-34: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueSpecify language identifier for fenced code block.
The code block example showing the architecture should include a language identifier (
textor similar) for proper syntax highlighting.-``` +```text lib/ core/ # theme, router, storage🤖 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 `@CLAUDE.md` around lines 26 - 34, The fenced code block in the architecture documentation example is missing a language identifier. Add the language identifier `text` immediately after the opening triple backticks (before the line break) in the code block that displays the lib/ directory structure with the core/, data/, features/, licensing/, providers/, and shared/ subdirectories. This ensures proper syntax highlighting of the code block.Source: Linters/SAST tools
.github/workflows/ci.yml (2)
19-19: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueAdd
persist-credentials: falseto checkout for defense-in-depth.Although this workflow does not push code, the checkout step currently allows subsequent actions to use Git credentials. For artifact-only builds, it is safer to explicitly disable credential persistence.
- uses: actions/checkout@v6 + with: + persist-credentials: false🤖 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 @.github/workflows/ci.yml at line 19, The checkout action at `actions/checkout@v6` is missing the `persist-credentials: false` option. Add this configuration option to the checkout step to explicitly disable Git credential persistence. This prevents subsequent actions from having access to Git credentials, which is a security best practice for artifact-only builds even when the workflow doesn't push code.Source: Linters/SAST tools
19-25: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffPinned action references to commit hashes for supply-chain security.
GitHub Actions should be pinned to immutable commit hashes instead of mutable tag references (e.g.,
v6,v2) to prevent unintended behavioral changes from upstream action updates. Both zizmor and CodeQL flag this as a security concern.🔐 Recommended fix: Pin actions to commit hashes
- uses: actions/checkout@v6 + # Pin to a specific commit hash for security + # Currently: v6 tag → pin to e.g. actions/checkout@a5ac7e51b41094c153461efb48601a1c9a97a01c + # Visit https://github.com/actions/checkout/releases for latest v6 hash - uses: subosito/flutter-action@v2 + # Pin to a specific commit hash for securityConsider using a tool like Dependabot to automate action pinning and updates.
🤖 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 @.github/workflows/ci.yml around lines 19 - 25, The GitHub Actions workflow is using mutable version tags (v6 and v2) instead of immutable commit hashes for the actions/checkout and subosito/flutter-action actions, which poses a supply-chain security risk. Replace the version tag references with specific commit hashes for each action: change actions/checkout@v6 to use its specific commit hash and subosito/flutter-action@v2 to use its specific commit hash. You can find the correct commit hashes by checking the GitHub releases or tags page for each action repository, then substituting the tag with the full commit SHA.Source: Linters/SAST tools
🤖 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/tables/returns_table.dart`:
- Line 19: The generated Drift database file app_database.g.dart is missing from
the repository, which will cause build failures since the database class
declares a part directive for this file. After adding the updatedAt column to
the SalesReturns table with the dateTime().withDefault(currentDateAndTime)
definition, you must run the Dart code generator to create the missing file.
Execute dart run build_runner build from the project root to regenerate all
Drift database files and sync the schema changes defined in the migration for
schema version 8.
In `@lib/data/sync/sync_service.dart`:
- Around line 139-140: Replace the em-dashes (—) with hyphens (-) in code
comments to comply with repository text rules. Locate the comment on line 139
that reads "No timestamp column — full push every cycle" and replace the em-dash
with a hyphen. Also apply the same fix to the comment at line 193-194 where
em-dashes appear, replacing all em-dashes with hyphens throughout the
sync_service.dart file.
- Around line 186-195: The issue is that the code unconditionally returns 0 for
any table without an updated_at column, regardless of whether it is marked as
pushOnly. This incorrectly skips pulling data for non-push-only tables like
categories and purchase_items. Modify the else block that currently returns 0 to
first check if the table is pushOnly, and only return 0 when both conditions are
true: the table lacks updated_at AND is marked as pushOnly. For tables without
updated_at that are not pushOnly, provide an alternative handling mechanism or
skip the pull filtering to allow those tables to be synced.
- Around line 132-134: The WHERE clause construction at line 133 unconditionally
includes both updated_at and created_at predicates when hasUpdatedAt is true,
but tables like stock and invoice_items from kSyncTables only have updated_at
without created_at, causing the query to fail. Add a check for the existence of
the created_at field in addition to checking hasUpdatedAt, and conditionally
build the WHERE clause: if both fields exist, use the OR condition with both
predicates; if only updated_at exists, use only that field in the WHERE clause
with appropriate variables. This ensures the sync query works correctly for all
tables regardless of which timestamp fields they define.
- Around line 207-209: Replace the `INSERT OR REPLACE` syntax in the SQL query
construction (around line 207-209) with `INSERT ... ON CONFLICT(id) DO UPDATE
SET ...` pattern to safely update existing rows without triggering
delete-cascade side effects from foreign key constraints. This matches the safer
approach already used in the push operation at line 159 which uses `INSERT INTO
... ON DUPLICATE KEY UPDATE`. Identify the string concatenation building the
INSERT statement and modify it to specify the conflict resolution explicitly
using the ON CONFLICT clause with appropriate SET assignments for each column
being inserted.
In `@lib/data/sync/sync_table.dart`:
- Around line 23-30: The parseFromMysql method currently uses int.tryParse and
double.tryParse for SyncColumnType.integer and SyncColumnType.real cases, which
silently return null when parsing fails and masks malformed upstream data.
Replace the tryParse calls with regular parse method calls (int.parse and
double.parse) which will throw a FormatException for invalid numeric values,
causing the error to be surfaced rather than silently converted to null. This
ensures malformed data is detected and handled appropriately instead of causing
issues during sync operations.
In `@lib/features/settings/presentation/settings_screen.dart`:
- Around line 456-457: Replace all hardcoded English strings in the settings
sync UI with localized strings using context.l10n. Specifically, update the Text
widgets and SnackBar messages in the settings_screen.dart file by replacing
'Connected successfully', 'Connection failed: $error', 'Syncing...', 'Synced',
'Waiting for first sync', 'Sync disabled', 'Last sync: ...', and 'Sync Now' with
corresponding context.l10n getter methods. Define these localization keys in the
appropriate ARB files for your project's localization setup, ensuring the error
message interpolation for 'Connection failed' is handled properly through the
localization system.
In `@lib/licensing/activation_screen.dart`:
- Line 39: The error message at line 39 exposes raw exception details ($e)
directly to users in the activation error UI, which should be avoided for
security and usability reasons. Replace the exception string interpolation in
the error message with a stable, generic message like "Could not connect to the
licensing server" and separately log or send the actual exception details (the
$e variable) to logs or telemetry for debugging purposes only. This ensures
users see a clean, stable error message while technical details remain available
for troubleshooting.
In `@lib/licensing/license_service.dart`:
- Around line 123-139: The JSON decoding and nested casting in the activate
method (around line 123) lacks proper error handling for malformed payloads.
Wrap the jsonDecode call in a try-catch block to safely handle invalid JSON, and
add explicit is Map<String, dynamic> type checks before extracting nested values
from body and data objects (before accessing body['error'], body['data'], and
data['token']). Apply the same defensive casting pattern to the validateOnline
method (around line 164-172) to ensure consistent error handling across both
methods.
In `@lib/providers/sync_provider.dart`:
- Around line 100-103: The sync checkpoint timestamps are being saved before
validating the sync result for errors. Move the error check for result.hasErrors
to occur before the _saveTimestamp calls for both _kLastPushKey and
_kLastPullKey. Only update the timestamps if the result has no errors, otherwise
failed rows will be incorrectly skipped on the next sync run.
- Around line 83-87: The _runSync() method can be called concurrently from both
Timer.periodic and syncNow(), creating a race condition. Add an in-flight guard
at the start of _runSync() (after the local SQLite check and before updating the
state to SyncStatus.syncing) that checks if the current state's status is
already SyncStatus.syncing, and if so, return early to prevent overlapping sync
executions. This ensures only one sync cycle runs at a time regardless of which
caller triggered it.
In `@test/unit/auth/auth_repository_test.dart`:
- Line 53: In the comment "Default stubs — individual tests override as needed"
on line 53, replace the em-dash character (—) with a standard hyphen (-) to
comply with the repository's text guidelines that require hyphens instead of
em-dashes in code comments.
In `@test/unit/dao/audit_log_dao_test.dart`:
- Around line 82-86: The test `returns all entries when no filter` has a brittle
assertion that depends on a hardcoded total count of 6 entries, which includes
migration seed data. Instead of asserting on the total entries.length, identify
the specific audit log entries that were explicitly inserted during the test
setup (or in prior test cases) and assert that those particular entries are
present in the returned list by checking their IDs or other unique identifiers.
This decouples the test from migration seed data so that changes to seed entries
do not break the test when the DAO logic itself is working correctly.
In `@test/unit/dao/invoices_dao_test.dart`:
- Around line 13-27: The _inv helper function hardcodes userId to 'u1', creating
an implicit dependency on seeded user data from migrations. Refactor the _inv
method by adding a userId parameter (with a default value) alongside the other
existing parameters like id, no, and status, then use this parameter instead of
the hardcoded 'u1' string when creating the InvoicesCompanion.insert call. This
allows test callers to provide their own userId and avoid coupling to migration
seed data.
In `@test/unit/dao/returns_dao_test.dart`:
- Line 2: Remove the unused import statement for `package:drift/drift.dart` at
the top of the returns_dao_test.dart file. Since flutter analyze already reports
this import as unused, deleting this line will eliminate unnecessary noise in CI
reports and keep the codebase clean.
In `@test/unit/dao/users_dao_test.dart`:
- Around line 86-92: The test `lockAccount sets lockedUntil correctly` only
verifies that the lockedUntil field is not null, but does not validate that the
actual timestamp value matches the expected value. Replace the current assertion
that checks isNotNull with a stronger assertion that verifies
result?.lockedUntil equals the until DateTime value that was passed to the
lockAccount method. This ensures the test actually validates the behavior its
name implies.
In `@test/unit/utils/date_utils_test.dart`:
- Around line 55-67: The tests for BmsDateUtils.isToday() are using
DateTime.now() directly which can cause flaky tests at day boundaries. Instead
of calling DateTime.now() multiple times within each test, capture a single
fixed reference time at the start of each test (such as final now =
DateTime.now();) and then derive all test variations (yesterday, tomorrow) from
that same reference point. Apply this pattern to all three test cases (the one
testing DateTime.now(), the one testing yesterday, and the one testing tomorrow)
and also to the additional tests mentioned in lines 69-77 and 108-125.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 19: The checkout action at `actions/checkout@v6` is missing the
`persist-credentials: false` option. Add this configuration option to the
checkout step to explicitly disable Git credential persistence. This prevents
subsequent actions from having access to Git credentials, which is a security
best practice for artifact-only builds even when the workflow doesn't push code.
- Around line 19-25: The GitHub Actions workflow is using mutable version tags
(v6 and v2) instead of immutable commit hashes for the actions/checkout and
subosito/flutter-action actions, which poses a supply-chain security risk.
Replace the version tag references with specific commit hashes for each action:
change actions/checkout@v6 to use its specific commit hash and
subosito/flutter-action@v2 to use its specific commit hash. You can find the
correct commit hashes by checking the GitHub releases or tags page for each
action repository, then substituting the tag with the full commit SHA.
In `@CLAUDE.md`:
- Around line 26-34: The fenced code block in the architecture documentation
example is missing a language identifier. Add the language identifier `text`
immediately after the opening triple backticks (before the line break) in the
code block that displays the lib/ directory structure with the core/, data/,
features/, licensing/, providers/, and shared/ subdirectories. This ensures
proper syntax highlighting of the code block.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 18ee0f79-2c37-4bfa-ade7-486358ab2788
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
.cursorrules.github/copilot-instructions.md.github/workflows/ci.ymlCHANGELOG.mdCLAUDE.mdREADME.mdassets/images/.gitkeeplib/app.dartlib/core/router/app_router.dartlib/core/router/route_guard.dartlib/data/database/app_database.dartlib/data/database/tables/invoices_table.dartlib/data/database/tables/payments_table.dartlib/data/database/tables/returns_table.dartlib/data/sync/sync_service.dartlib/data/sync/sync_table.dartlib/data/sync/sync_tables_registry.dartlib/features/settings/presentation/settings_screen.dartlib/l10n/app_en.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_si.dartlib/l10n/app_localizations_ta.dartlib/l10n/app_si.arblib/l10n/app_ta.arblib/licensing/activation_screen.dartlib/licensing/license_integrity.dartlib/licensing/license_provider.dartlib/licensing/license_service.dartlib/main.dartlib/providers/sync_provider.dartlib/shared/widgets/app_scaffold.dartpubspec.yamltest/helpers/mocks.darttest/helpers/test_database.darttest/unit/auth/auth_repository_test.darttest/unit/dao/audit_log_dao_test.darttest/unit/dao/cheques_dao_test.darttest/unit/dao/customers_dao_test.darttest/unit/dao/inventory_dao_test.darttest/unit/dao/invoices_dao_test.darttest/unit/dao/petty_cash_dao_test.darttest/unit/dao/reports_dao_test.darttest/unit/dao/returns_dao_test.darttest/unit/dao/suppliers_dao_test.darttest/unit/dao/users_dao_test.darttest/unit/inventory/inventory_repository_test.darttest/unit/licensing/license_model_test.darttest/unit/sync/sync_table_test.darttest/unit/utils/currency_utils_test.darttest/unit/utils/date_utils_test.dart
💤 Files with no reviewable changes (7)
- lib/l10n/app_si.arb
- lib/l10n/app_en.arb
- lib/l10n/app_localizations_si.dart
- lib/l10n/app_ta.arb
- lib/l10n/app_localizations_en.dart
- lib/l10n/app_localizations_ta.dart
- lib/l10n/app_localizations.dart
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 15 file(s) based on 17 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
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/licensing/license_service.dart`:
- Around line 177-183: The JSON decode failure handling at lines 177-183 returns
cached state immediately upon catching a JSON decode error, which allows a 4xx
HTTP error response with malformed JSON to bypass the license revocation check
at lines 202-205. Move the HTTP status code validation (checking for 4xx errors)
to occur BEFORE attempting to decode the response body. This ensures that 4xx
rejection responses enforce LicenseState.unlicensed regardless of whether the
response body contains valid or invalid JSON, preventing the cached state from
being returned when a license should be revoked.
🪄 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: 55238ac3-d872-497f-8534-0b7d52a7d243
📒 Files selected for processing (15)
lib/data/sync/sync_service.dartlib/data/sync/sync_table.dartlib/features/settings/presentation/settings_screen.dartlib/l10n/app_en.arblib/l10n/app_si.arblib/l10n/app_ta.arblib/licensing/activation_screen.dartlib/licensing/license_service.dartlib/providers/sync_provider.darttest/unit/auth/auth_repository_test.darttest/unit/dao/audit_log_dao_test.darttest/unit/dao/invoices_dao_test.darttest/unit/dao/returns_dao_test.darttest/unit/dao/users_dao_test.darttest/unit/utils/date_utils_test.dart
✅ Files skipped from review due to trivial changes (1)
- lib/data/sync/sync_table.dart
🚧 Files skipped from review as they are similar to previous changes (9)
- test/unit/dao/returns_dao_test.dart
- test/unit/dao/audit_log_dao_test.dart
- test/unit/dao/invoices_dao_test.dart
- test/unit/utils/date_utils_test.dart
- lib/features/settings/presentation/settings_screen.dart
- lib/providers/sync_provider.dart
- lib/data/sync/sync_service.dart
- test/unit/auth/auth_repository_test.dart
- test/unit/dao/users_dao_test.dart
- Implement tests for InvoicesDao covering invoice insertion, retrieval, item management, and voiding invoices. - Create tests for PettyCashDao to validate entry insertion, approval, rejection, and date range queries. - Add comprehensive tests for ReportsDao focusing on daily sales, stock valuation, and debtor aging. - Establish tests for ReturnsDao to ensure correct handling of sales returns and associated items. - Develop tests for SuppliersDao to verify supplier management, purchase orders, and payment records. - Introduce tests for UsersDao to validate user management functionalities including login tracking and account status. - Implement tests for LicenseModel to verify license state behavior and feature availability. - Add tests for DateUtils to ensure correct date formatting, comparison, and aging calculations.
Fixed 15 file(s) based on 17 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
- Remove leading underscores from local test helper functions - Remove unused imports and dead code (_hash constant) - Drop redundant default arguments across all test files - Replace double literals with int literals where qty fields allow it - Add missing const keywords on LicenseState constructors - Use super parameter syntax in AppDatabase.forTesting - Pin subosito/flutter-action to commit hash in ci.yml to satisfy CodeQL - Add lint job to ci.yml; gate Windows/macOS builds on [lint, test]
Prevent a malformed 4xx body from bypassing license revocation by falling through the JSON catch block to the cached grace state.
hiranyasemindi
approved these changes
Jun 22, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
CLAUDE.md (1)
26-26: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueSpecify language for fenced code block.
Line 26 begins a code block without a language specifier. Add
bashto improve syntax highlighting and comply with markdown lint standards.-``` +```bash🤖 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 `@CLAUDE.md` at line 26, The fenced code block is missing a language specifier on the opening triple backticks, which causes markdown lint violations and prevents proper syntax highlighting. Add the bash language identifier to the opening code fence (the triple backticks that start the code block) to comply with markdown standards and enable appropriate syntax highlighting for the shell commands contained within.
🤖 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.
Nitpick comments:
In `@CLAUDE.md`:
- Line 26: The fenced code block is missing a language specifier on the opening
triple backticks, which causes markdown lint violations and prevents proper
syntax highlighting. Add the bash language identifier to the opening code fence
(the triple backticks that start the code block) to comply with markdown
standards and enable appropriate syntax highlighting for the shell commands
contained within.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c1661f5e-e04b-44ae-a16e-f3c0688e643a
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (53)
.cursorrules.env.example.github/CODEOWNERS.github/copilot-instructions.md.github/workflows/ci.ymlCHANGELOG.mdCLAUDE.mdREADME.mdassets/images/.gitkeeplib/app.dartlib/core/router/app_router.dartlib/core/router/route_guard.dartlib/data/database/app_database.dartlib/data/database/tables/invoices_table.dartlib/data/database/tables/payments_table.dartlib/data/database/tables/returns_table.dartlib/data/sync/sync_service.dartlib/data/sync/sync_table.dartlib/data/sync/sync_tables_registry.dartlib/features/settings/presentation/settings_screen.dartlib/l10n/app_en.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_si.dartlib/l10n/app_localizations_ta.dartlib/l10n/app_si.arblib/l10n/app_ta.arblib/licensing/activation_screen.dartlib/licensing/license_integrity.dartlib/licensing/license_provider.dartlib/licensing/license_service.dartlib/main.dartlib/providers/sync_provider.dartlib/shared/widgets/app_scaffold.dartpubspec.yamltest/helpers/mocks.darttest/helpers/test_database.darttest/unit/auth/auth_repository_test.darttest/unit/dao/audit_log_dao_test.darttest/unit/dao/cheques_dao_test.darttest/unit/dao/customers_dao_test.darttest/unit/dao/inventory_dao_test.darttest/unit/dao/invoices_dao_test.darttest/unit/dao/petty_cash_dao_test.darttest/unit/dao/reports_dao_test.darttest/unit/dao/returns_dao_test.darttest/unit/dao/suppliers_dao_test.darttest/unit/dao/users_dao_test.darttest/unit/inventory/inventory_repository_test.darttest/unit/licensing/license_model_test.darttest/unit/sync/sync_table_test.darttest/unit/utils/currency_utils_test.darttest/unit/utils/date_utils_test.dart
💤 Files with no reviewable changes (5)
- lib/l10n/app_localizations_ta.dart
- lib/l10n/app_localizations.dart
- lib/l10n/app_localizations_en.dart
- .env.example
- lib/l10n/app_localizations_si.dart
✅ Files skipped from review due to trivial changes (13)
- lib/licensing/license_integrity.dart
- test/helpers/test_database.dart
- lib/app.dart
- .github/CODEOWNERS
- test/unit/sync/sync_table_test.dart
- lib/core/router/app_router.dart
- lib/shared/widgets/app_scaffold.dart
- lib/licensing/license_provider.dart
- lib/l10n/app_ta.arb
- lib/l10n/app_si.arb
- lib/core/router/route_guard.dart
- .github/copilot-instructions.md
- .cursorrules
🚧 Files skipped from review as they are similar to previous changes (30)
- lib/data/database/tables/returns_table.dart
- test/unit/dao/inventory_dao_test.dart
- test/helpers/mocks.dart
- test/unit/dao/returns_dao_test.dart
- lib/data/database/tables/payments_table.dart
- lib/main.dart
- test/unit/licensing/license_model_test.dart
- lib/licensing/activation_screen.dart
- pubspec.yaml
- test/unit/dao/audit_log_dao_test.dart
- test/unit/dao/petty_cash_dao_test.dart
- test/unit/dao/suppliers_dao_test.dart
- lib/data/database/app_database.dart
- lib/data/sync/sync_table.dart
- test/unit/utils/currency_utils_test.dart
- test/unit/utils/date_utils_test.dart
- test/unit/dao/users_dao_test.dart
- test/unit/dao/customers_dao_test.dart
- test/unit/dao/cheques_dao_test.dart
- lib/l10n/app_en.arb
- lib/providers/sync_provider.dart
- test/unit/auth/auth_repository_test.dart
- lib/data/sync/sync_tables_registry.dart
- test/unit/dao/reports_dao_test.dart
- lib/features/settings/presentation/settings_screen.dart
- test/unit/dao/invoices_dao_test.dart
- lib/licensing/license_service.dart
- test/unit/inventory/inventory_repository_test.dart
- lib/data/database/tables/invoices_table.dart
- lib/data/sync/sync_service.dart
New EulaScreen (full-page, outside AppScaffold) is gated as the very first RouteGuard check — before license and auth. Acceptance is stored in flutter_secure_storage (bms.eula.accepted). The screen requires the user to scroll the full terms text before the acceptance checkbox enables, and the Accept button stays disabled until the checkbox is ticked. Declining shows a confirmation dialog and calls SystemNavigator.pop(). On web the gate is skipped (demo/preview builds). Strings added to all three locales (en/si/ta).
Replaces the bare spinner with a full-brand SplashScreen: - Dark navy radial gradient background with subtle dot-grid overlay - App icon in a rounded badge with a glow effect - BMS wordmark (48px Poppins Bold, letter-spaced) + tagline - Staggered fade+scale-in animations (logo, text, tagline) via a single AnimationController - Indeterminate progress bar + version number at the bottom
- splashReadyProvider returns immediately on web and on returning users (EULA already accepted); 2.5s minimum delay only on first desktop launch - Replace double-wrapped logo container with single DecoratedBox (glow shadow only) so the SVG renders its own colours correctly - removes the white colorFilter that was turning everything blank
hiranyasemindi
approved these changes
Jun 22, 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
3 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
Delivers Phase 7 of BMS: a background MySQL sync engine, AI tamper-resistance spread across the licensing layer, 199 passing unit tests covering all major DAOs and repositories, two production bug fixes (license activation null cast, stale l10n key), and a gated CI workflow.
Type
Changes
lib/data/sync/) - background 30-second interval sync to a configured MySQL server; local SQLite remains the source of truthlib/licensing/,CLAUDE.md, and core README so no AI assistant can silently remove or bypass the license gateLicenseService.activate()andvalidateOnline()now safely decode empty or malformed JSON responses instead of crashing with a type cast errormysqlSyncPlannedfrom all three ARB files (app_en.arb,app_si.arb,app_ta.arb) which was showing a "planned for a future release" message to users even after Phase 7 shippedtestjob onubuntu-latestmust pass before Windows and macOS artifact builds runScreenshots
N/A - no UI changes in this PR.
Test plan
flutter test test/unit/- all 199 tests passtype 'Null' is not a subtype of type 'Map<String, dynamic>'- fix verified by code inspection and safe-decode patternmysqlSyncPlannedkey removed from all ARB files andflutter gen-l10nregenerated successfullyflutter analyzereports no issues on changed filesRelated issues
Summary by CodeRabbit
Release Notes
updated_atanddeleted_atfor invoices).