Om/logic correction#74
Open
Omprakash-48 wants to merge 15 commits into
Open
Conversation
… code Consolidate repeated patterns into single sources of truth so future changes are made in one place instead of 10-20. New helpers: - utils/date_helpers.dart — parseDateTime, parseTime, formatDurationSeconds shared by date/datetime/duration fields and form_builder patch normaliser - utils/frappe_json_utils.dart — parseBool: canonical 0/1/'true'/bool coercion for every model fromJson - utils/sql_row_utils.dart — retryCountFrom, lastAttemptAtFrom, utcMillisFrom, parseEnumByName for sqflite row factories - ui/widgets/fields/field_helpers.dart — requiredValidator, baseFieldDecoration, fieldErrorText shared by every Form*Field - ui/widgets/screen_helpers.dart — showStatusSnackBar + showConfirmDialog replace ad-hoc SnackBar/Dialog construction across screens Schema/query additions: - system_columns: systemSyncMetadataColumnNames + linkCompanionColumnDDL consumed by PayloadAssembler, PayloadSerializer, parent/child schema builders and runtime migration so strip rules cannot diverge - table_name: stripDocsPrefix inverse of normalizeDoctypeTableName - query_result: QueryResult.ofRows derives hasMore + returnedCount for online/offline parity in UnifiedResolver No behaviour change. flutter analyze + dart format clean across the package.
… UX polish - DoctypeService.list: guard non-List Frappe message shape → treat as empty page - RestHelper.get/getPublic: expose per-call timeout and maxRetries overrides so boot/splash callers can fast-fail instead of burning the default 30s × 3-retry budget - PermissionService.syncFromApi: accept timeout param; remove silent catch so callers own error handling - MobileHomeScreen: suppress full-screen spinner on background syncComplete$ reloads — only show it on first paint when the list is empty - FieldStyle: default showLabel/showDescription to false - Tests: Cursor.start round-trip, initial-sync limit_start advance vs incremental modified-filter path, stall guard cursor pre-seeding
- FrappeSDK._runUpgradeClosurePull: call ensureSchemaForClosure before the closure pull so per-doctype mirror tables exist (closes the "no such table" hole that previously required SNF's runSnfPostSdkSync to re-pull) - FrappeSDK._initialMetaAndDataSync: bracket the boot pull with isInitialSync=true so host UI subscribed to state$ can render a syncing indicator; reset on every exit path - FrappeSDK: surface boot failures via SyncStateNotifier.lastError (code: 'network'|'permissions'), expose retryInitialMetaAndDataSync(), clear lastError on logout - parent/child schema DDL: CREATE TABLE/INDEX IF NOT EXISTS so SNF's belt-and-suspenders ensureSchemaForClosure and the SDK's eager pull can re-enter safely after a partial failure - PullEngine: optional SchemaReconcilerFn wired to OfflineRepository.reconcileParentTableForMeta so ALTER ADD COLUMN runs before applying a page (closes the meta-cache race) - PullEngine stall guard now scoped to incremental (complete=true); initial-sync uses limit_start offset which always advances - PushEngine: optional PayloadTransformerFn between assembly and HTTP dispatch (e.g. host auto-submit on sync); exception-safe - FrappeFormBuilder: onValidationFailed callback so parents can stop loading indicators when submit is rejected - FrappeFormBuilder: ??=-inject linkOptionService / linkFieldCoordinator on host-supplied customFieldFactory so Link pickers aren't half-configured - SyncStateNotifier: recordLastError / clearLastError bypass copyWith (cannot express "set to null") - SDK barrel: export PayloadTransformerFn + ChildTableFormBuilder - Tests: PayloadTransformerFn smoke; onValidationFailed fires on invalid submit and not on valid
…logic_correction
Conflict resolution favoured HEAD (om/logic_correction): kept the
`stripDocsPrefix` helper, the `tableNameFor` docblock, `_assertInitialized`,
`_buildPullableDoctypes`, the `payloadTransformer` parameter wiring, and the
`closure_result.dart` import. The incoming `3cbb9e6` ("Port of
om/logic_correction fixes to om/offline_improvement") was an earlier
snapshot of the same work, so HEAD's refactored versions are the canonical
copy. Also picked up the `.gitignore` trailing-newline strip from the
incoming branch.
…-mobile-sdk into om/logic_correction
# Conflicts: # lib/src/database/app_database.dart # lib/src/database/daos/outbox_dao.dart # lib/src/screens/mobile_home_screen.dart # lib/src/ui/document_list_screen.dart # lib/src/ui/widgets/form_builder.dart
Combines the offline-push correctness fixes with logic_correction's
payload-transformer / DDL-helper refactors. Conflict resolutions:
- push_engine.dart / sync_engine_builder.dart: kept BOTH logic's
PayloadTransformerFn (field + re-export) AND offline's deadlock retry
(DeadlockError, isDeadlockApiException, _withJitter, _dispatchUnits).
- system_columns.dart: linkCompanionColumnDDL now quotes the column
("name__is_local") — keeps logic's single-source helper while honoring
offline's "quote identifiers in DDL" fix (parent/child schema use it).
- offline_repository.dart: kept offline's per-iteration try/catch +
_reconcileParentTableSchema healing, using logic's _executeDDL helper.
- form_builder.dart (from prior develop merge): submit path keeps the
hidden-field strip; _buildCompleteFormData retained for dirty detection.
Full suite green (1304 tests); analyzer clean.
…cripts/) These were swept in by `git add -A` during the merges; they are local developer tooling, not part of the package. Untracked and gitignored.
…mable pull, pre-flight manifest - dhwani-ris#43: PullApply matches a pulled parent by mobile_uuid when server_name is not yet stamped locally, preventing duplicate rows during an in-flight push INSERT; reconcile (no false conflict) when local server_name is NULL. Extend the hasActivePushFor defer to SyncService pull paths (parity with PullEngine) via SyncStatus.deferredActivePush. - dhwani-ris#47: SyncMode {full, pushOnly, pullOnly} on SyncController.syncNow (default full = existing behavior). - dhwani-ris#64: checkpoint the PullEngine cursor (complete:false) after every applied page so an app-kill mid initial-sync resumes from the last page, not page 0. - dhwani-ris#49: /sync_details pre-flight client (DoctypeService.getSyncDetails) + pure doctypesToSkip; wired into _runUpgradeClosurePull to skip unchanged INCREMENTAL doctypes, with graceful full-pull fallback on any failure. Note: sdk_log.dart and the sdkLog lines in sync_controller/sync_service are pre-existing refactor bits that ride along because these files are shared; the rest of that print->sdkLog refactor is intentionally left uncommitted.
Route stray print() diagnostics through sdkLog (a no-op in release builds) and drop the `// ignore: avoid_print` suppressions, importing sdk_log where needed. No behavioral change.
Resolve conflicts in auth.dart, mobile_home_screen.dart, frappe_sdk.dart, and sync_service.dart: - Adopt sdkLog over debugPrint/print per the offline-sync logging sweep. - Keep both screen_helpers and sdk_log imports in mobile_home_screen. - forTesting ctor: keep httpClient param + payloadTransformer init. - Closure pull: use the dhwani-ris#49 sync_details 'allowed' (skip-unchanged) set; fix List/Set mismatch since the refactor made _buildPullableDoctypes return a List (allowed = pullable.toSet(); allowed.difference(skip)). - sync_service: keep refactored _loadMetaFromDao, preserve the new pullOneInternalForTest helper, drop vestigial parse-catch tails. flutter analyze: clean. flutter test: 1333 passed.
FrappeSDK.dispose() closes the shared notifier, but an in-flight PushEngine/PullEngine/SyncController (all wired to the same instance by SyncEngineBuilder) can resume from an await and write notifier.value afterwards, hitting StreamController.add on a closed controller and throwing StateError during logout/shutdown. dispose() only drained _pendingDrain, not controller-driven syncs. Guard the setter with an isClosed check so a late write from any writer is a safe no-op. Update the notifier test to the new no-throw contract and add a regression test for the teardown race.
Move the per-fieldtype normalization switch and the multiselect helper out of the FrappeFormBuilder widget into a pure, headless FieldNormalizer so the rules can be unit-tested directly. Behavior unchanged; the widget delegates at both call sites. Adds 22 characterization tests covering every branch.
_handleLoginSuccess and build() re-instantiated MetaService, OfflineRepository, SyncService and a hand-rolled UnifiedResolver that diverged from the SDK's own wiring. Those blocks only ran when initialize() failed, but build() returns the error screen first, so they were unreachable. Remove them; rely on sdk.meta/repository/sync/ linkOptions with a "not initialized" guard.
6 tasks
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
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
Brings
om/logic_correctionup to date with the offline-first work. This PRbundles four threads of work that landed together on this branch:
(no behavior change).
sync resilient and observable.
callback, retry/error surfaces).
Plus a logging cleanup (
print()→sdkLog).Net: 83 files, +2635 / −1365, 18 test files added/changed.
1. Refactor — shared helpers (no behavior change)
Consolidates repeated patterns into single sources of truth:
utils/date_helpers.dart—parseDateTime/parseTime/formatDurationSecondsshared by date/datetime/duration fields and theform-builder patch normaliser.
utils/frappe_json_utils.dart—parseBool: canonical0/1/'true'/boolcoercion for every model
fromJson.utils/sql_row_utils.dart—retryCountFrom,lastAttemptAtFrom,utcMillisFrom,parseEnumByNamefor sqflite row factories.ui/widgets/fields/field_helpers.dart—requiredValidator,baseFieldDecoration,fieldErrorTextshared by everyForm*Field.ui/widgets/screen_helpers.dart—showStatusSnackBar+showConfirmDialogreplace ad-hoc SnackBar/Dialog construction.
database/schema/system_columns.dart—systemSyncMetadataColumnNames+linkCompanionColumnDDLconsumed by PayloadAssembler/Serializer and theschema builders so strip rules can't diverge.
query_result.dart—QueryResult.ofRowsderiveshasMore/returnedCountfor online/offline parity in
UnifiedResolver.2. Initial-sync hardening + schema-gap fix
_runUpgradeClosurePullnow callsensureSchemaForClosurebefore theclosure pull, closing the "no such table" hole that previously forced SNF's
runSnfPostSdkSyncto re-pull.isInitialSync=trueso host UI subscribed tostate$can render a syncing indicator (reset on every exit path).SyncStateNotifier.lastError(
code: 'network' | 'permissions'); addedretryInitialMetaAndDataSync();lastErrorcleared on logout.CREATE TABLE/INDEX IF NOT EXISTSso eager pulland SNF's belt-and-suspenders reconcile can re-enter safely after a partial
failure.
PullEnginestall guard scoped to incremental (complete=true);initial sync uses
limit_startoffset pagination, which always advances.RestHelper.get/getPublicexpose per-calltimeout/maxRetriesoverrides instead of burning the default 30s × 3.MobileHomeScreensuppresses the full-screen spinner on backgroundsyncComplete$reloads — only shown on first paint when the list is empty.3. New host-facing extension points
PushEngine— optionalPayloadTransformerFnbetween assembly and HTTPdispatch (e.g. host auto-submit on sync); exception-safe. Exported from the
barrel.
FrappeFormBuilder.onValidationFailed— lets parents stop loadingindicators when a submit is rejected.
FrappeFormBuilder??=-injectslinkOptionService/linkFieldCoordinatoron a host-supplied
customFieldFactoryso Link pickers aren'thalf-configured.
PullEngine— optionalSchemaReconcilerFnwired toOfflineRepository.reconcileParentTableForMetasoALTER ADD COLUMNrunsbefore applying a page (closes the meta-cache race).
4. Offline-sync correctness fixes
PullApplymatches a pulled parent bymobile_uuidwhenserver_nameisn't stamped locally yet, preventingduplicate rows during an in-flight push INSERT (reconcile, no false conflict).
The
hasActivePushFordefer is extended toSyncServicepull paths viaSyncStatus.deferredActivePush(parity withPullEngine).SyncMode {full, pushOnly, pullOnly}onSyncController.syncNow(defaultfull= existing behavior).PullEnginecheckpoints its cursor(
complete:false) after every applied page, so an app-kill mid initial-syncresumes from the last page instead of page 0.
DoctypeService.getSyncDetails+ puredoctypesToSkip, wired into_runUpgradeClosurePullto skip unchangedINCREMENTAL doctypes, with graceful full-pull fallback on any failure.
5. Logging cleanup
Routes stray
print()diagnostics throughsdkLog(a no-op in release builds)and drops the
// ignore: avoid_printsuppressions. No behavioral change.Testing
flutter analyze— clean.flutter test— 1333 passing.doctypesToSkip), SyncMode,active-push defer, pull_apply mobile_uuid matching, cursor resume,
payload-transform, validation-failed callback, reserved-word column DDL, and
field-style defaults.
Notes for reviewers
be skimmed; review attention is best spent on sections 2–4.
om/offline_improvementanddevelopmultipletimes; conflict resolutions favored the refactored single-source helpers
while preserving the offline correctness fixes (deadlock retry, DDL
identifier quoting, schema healing).