From 49ae712f5eb9ed1d326821f17c4c3f318b2d6ebe Mon Sep 17 00:00:00 2001 From: ayushya Date: Sun, 20 Sep 2026 17:20:14 -0400 Subject: [PATCH] Fix device sync: folder status badges, remove-deletes-local, multi-select, header summary - Folders now show the synced status badge themselves, not just their nested files - device-sync state never tracked folders individually, so syncStatusFor derives a folder's badge from whether its path falls within sync scope instead. - Turning sync off for a path now deletes its local mirror and clears its native sync-state entries (SyncEngine.removeLocalSync), instead of just stopping future updates and leaving the downloaded copy behind. - "Sync to device" in Files' selection toolbar now works over the whole selection at once instead of being gated to a single selected item. - The sync header's expanded panel shows real folder/item counts instead of a static "Synced" label that stayed accurate-sounding even before anything had actually synced. - Confirmation SnackBars on starting/stopping sync from both the selection toolbar and Settings' Device Sync card. Co-Authored-By: Claude Sonnet 5 --- .claude/context/server.md | 59 +++++++--- .claude/context/styling.md | 9 +- .../kotlin/dev/ayushya/noo/MainActivity.kt | 19 ++++ .../main/kotlin/dev/ayushya/noo/SyncEngine.kt | 29 +++++ lib/providers/sync_status_controller.dart | 102 +++++++++++++++--- lib/services/sync_service.dart | 17 +++ lib/views/account_view.dart | 10 +- lib/views/files_view.dart | 55 +++++++--- lib/widgets/synced_header_scaffold.dart | 24 ++++- 9 files changed, 273 insertions(+), 51 deletions(-) diff --git a/.claude/context/server.md b/.claude/context/server.md index 580cb5d..ab44d70 100644 --- a/.claude/context/server.md +++ b/.claude/context/server.md @@ -306,7 +306,15 @@ the whole engine is plain Kotlin using Android's WorkManager directly. chip/panel, replacing what used to be the WebDAV-refresh-loading indicator there) and `syncStatusFor(item)` (none/syncing/synced/conflict - drives the small corner badge on Files' tiles, `SyncStatusBadge`) are - both computed from this state, not fetched per-item. + both computed from this state, not fetched per-item. Folders never get + their own entry in the native state map (`diffFolder` only ever tracks + individual files - `if (entry.isFolder) continue`), so `syncStatusFor` + derives a folder's badge differently than a file's: `synced` once the + folder's own path (or an ancestor of it) is in sync scope + (`_isPathInSyncScope`, shared with `localSyncedFilePath`), `syncing` + while any sync pass is running, `conflict` if a pending conflict's + `remotePath` falls under it - not from `syncedFileIds`, which only ever + contains individual files. - **In-app conflict resolution reuses the exact same enqueue path as the notification actions** - `ConflictResolveWorker.enqueue(...)` is a shared companion function; `SyncConflictReceiver` (the notification @@ -326,31 +334,48 @@ the whole engine is plain Kotlin using Android's WorkManager directly. to actually push the local copy up or pull the server copy down and refresh that file's recorded state. - `MainActivity.kt`'s `dev.ayushya.noo/sync_service` channel - (`reschedule`/`cancel`/`syncNow`) is the only bridge from Dart: a periodic - `WorkRequest`'s input `Data` and `Constraints` are fixed at enqueue time, - so changing the synced-folder list, the active account, or the Wi-Fi-only - setting means cancelling and re-enqueueing, not updating in place. - [`SyncService`](../../lib/services/sync_service.dart) (Dart) wraps this - - `SyncStatusController` calls `SyncService.reschedule` after every - successful login/account switch and every synced-folder/`syncOnCellular` - change, and `SyncService.cancel` on logout/last-account-removed. The - network constraint is `NetworkType.UNMETERED` by default + (`reschedule`/`cancel`/`syncNow`/`removeLocalSync`) is the only bridge + from Dart: a periodic `WorkRequest`'s input `Data` and `Constraints` are + fixed at enqueue time, so changing the synced-folder list, the active + account, or the Wi-Fi-only setting means cancelling and re-enqueueing, + not updating in place. [`SyncService`](../../lib/services/sync_service.dart) + (Dart) wraps this - `SyncStatusController` calls `SyncService.reschedule` + after every successful login/account switch and every synced-folder/ + `syncOnCellular` change, and `SyncService.cancel` on logout/last-account- + removed. The network constraint is `NetworkType.UNMETERED` by default (`!syncOnCellular`, Wi-Fi only) or `NetworkType.CONNECTED` if the user's - opted into cellular sync. + opted into cellular sync. Turning sync off for a path + (`SyncStatusController.removeSyncedPath`) also calls + `SyncService.removeLocalSync`, which runs `SyncEngine.removeLocalSync` on + a background thread (deletes the local mirror files under that path + *and* their entries in the native sync-state map - clearing state too, + not just the files, matters because a bare "file's gone but the server + hasn't changed" without a state reset reads as a user-initiated local + deletion to `diffFolder`, so a later re-add wouldn't re-download + anything). - Synced-path list (`SyncStatusController.syncedPaths` - files or folders, not just folders despite the name of the underlying pref/native `Data` key, which stayed `ui_synced_folders`/`folders` to avoid a storage-key migration for a rename) follows the standard per-account-pref pattern - (JSON-encoded string list, in `AccountStore.perAccountPrefKeys`); so does - `syncEverything` (`ui_sync_everything`, per account) - when on, - `SyncService` sends `['/']` as the path list instead of `syncedPaths`, - mirroring the whole account rather than requiring per-item opt-in. - `syncOnCellular` is a plain global pref. All three are managed from + (JSON-encoded string list, in `AccountStore.perAccountPrefKeys`); a + parallel `_syncedPathTypes` map (path -> isFolder, `ui_synced_folder_types`) + tracks which of those paths are folders vs individual files for the sync + header's folder/item counts, defaulting missing entries to folder (the + common case, and what any path added before this map existed will look + like). `syncEverything` (`ui_sync_everything`, per account) works the + same way - when on, `SyncService` sends `['/']` as the path list instead + of `syncedPaths`, mirroring the whole account rather than requiring + per-item opt-in. `syncOnCellular` is a plain global pref. All three are + managed from Settings → Device Sync (a "Sync everything" switch, the path list with remove buttons - hidden while "Sync everything" is on - the cellular toggle, and a manual "Sync now"); individual files or folders are additionally toggled from Files' selection toolbar ("Sync to device", - single-selection, either item type). + works over the whole selection at once - either item type, folders or + files - not just a single item; the action reads as "stop syncing" only + once every selected item is already synced, otherwise it syncs whichever + ones aren't yet, and either direction ends with a confirmation + SnackBar). - **`android/app/proguard-rules.pro` exists specifically for this feature, and keeps `androidx.work.**` wholesale rather than naming individual classes.** Flutter's own Gradle plugin auto-enables R8 minification for diff --git a/.claude/context/styling.md b/.claude/context/styling.md index bc66919..f7f484b 100644 --- a/.claude/context/styling.md +++ b/.claude/context/styling.md @@ -63,12 +63,17 @@ widgets. Key points: - [`SyncedHeaderScaffold`](../../lib/widgets/synced_header_scaffold.dart) — the pull-to-sync `CustomScrollView` header shared by 5 of the 6 tabs (see `architecture.md`); also where the pull-to-refresh gesture thresholds and - the classic Material refresh spinner live. Its persistent chip/panel + the classic Material refresh spinner live. Its persistent compact chip (icon + "Sync off"/"Syncing…"/"Synced"/"Sync issue") reflects device-sync status (`SyncStatusController.syncHeaderStatus`), not the WebDAV-refresh loading state the pull gesture itself triggers - that has its own, separate floating spinner bubble, so nothing was lost by handing the - persistent text/icon over. + persistent text/icon over. The expanded panel's headline is a separate, + more detailed string (`_syncSummary` in `synced_header_scaffold.dart`) - + actual folder/item counts ("2 folders & 5 items synced") rather than + just repeating the chip's generic label, which would otherwise read + "Synced" even when a folder's just been added and nothing's downloaded + yet. - [`SyncStatusBadge`](../../lib/widgets/sync_status_badge.dart) — the small corner badge over a thumbnail showing per-item device-sync status (`cloud_done`/`sync`, nothing for not-synced/conflict); used in Files' diff --git a/android/app/src/main/kotlin/dev/ayushya/noo/MainActivity.kt b/android/app/src/main/kotlin/dev/ayushya/noo/MainActivity.kt index b3ab514..95fefbd 100644 --- a/android/app/src/main/kotlin/dev/ayushya/noo/MainActivity.kt +++ b/android/app/src/main/kotlin/dev/ayushya/noo/MainActivity.kt @@ -136,6 +136,7 @@ class MainActivity : FlutterFragmentActivity() { "syncNow" -> syncNow(call, result) "getSyncStatus" -> result.success(syncStatusMap(SyncStatusBus.snapshot())) "resolveConflict" -> resolveConflict(call, result) + "removeLocalSync" -> removeLocalSync(call, result) else -> result.notImplemented() } } @@ -405,6 +406,24 @@ class MainActivity : FlutterFragmentActivity() { result.success(null) } + /// Deletes a path's local mirror once the user turns sync off for it - + /// see [SyncEngine.removeLocalSync]. Runs off the main thread since it + /// walks/deletes a directory tree; `result.success` is posted back via + /// [mainHandler] since MethodChannel results must be delivered on the + /// platform thread. + private fun removeLocalSync(call: MethodCall, result: MethodChannel.Result) { + val accountId = call.argument("accountId") + val path = call.argument("path") + if (accountId == null || path == null) { + result.error("bad_args", "Missing required arguments", null) + return + } + Thread { + SyncEngine.removeLocalSync(applicationContext, accountId, path) + mainHandler.post { result.success(null) } + }.start() + } + /// One-off immediate run (Settings' "Sync now"), independent of the /// periodic schedule. private fun syncNow(call: MethodCall, result: MethodChannel.Result) { diff --git a/android/app/src/main/kotlin/dev/ayushya/noo/SyncEngine.kt b/android/app/src/main/kotlin/dev/ayushya/noo/SyncEngine.kt index 377536b..b8ba6eb 100644 --- a/android/app/src/main/kotlin/dev/ayushya/noo/SyncEngine.kt +++ b/android/app/src/main/kotlin/dev/ayushya/noo/SyncEngine.kt @@ -356,6 +356,35 @@ object SyncEngine { return actions } + /** + * Deletes [path]'s local mirror (a single file, or a whole folder's + * worth of files) and removes its entries from the persisted sync-state + * map, called when the user turns sync off for that path - without + * clearing the state too, a later re-add would see the (now missing) + * local file as "deleted, server unchanged" and skip re-downloading it + * (see [diffFolder]'s `!localExists && !serverChanged` branch) instead + * of pulling it back down. + */ + fun removeLocalSync(context: Context, accountId: String, path: String) { + val root = syncRoot(context, accountId) + val state = loadState(context, accountId).toMutableMap() + + var cleanPath = path.trim() + if (cleanPath.startsWith("/")) cleanPath = cleanPath.substring(1) + cleanPath = cleanPath.trimEnd('/') + val prefix = if (cleanPath.isEmpty()) "" else "$cleanPath/" + + val toRemove = state.filterValues { s -> + s.relPath == cleanPath || (prefix.isNotEmpty() && s.relPath.startsWith(prefix)) + }.keys + for (fileId in toRemove) state.remove(fileId) + saveState(context, accountId, state) + + val target = if (cleanPath.isEmpty()) root else File(root, cleanPath) + if (target.exists()) target.deleteRecursively() + Log.d(TAG, "removeLocalSync($path) -> removed ${toRemove.size} state entries") + } + fun folderIsSyncedUnder(itemPath: String, syncedFolders: List): String? { val normalizedItem = itemPath.trimEnd('/') for (folder in syncedFolders) { diff --git a/lib/providers/sync_status_controller.dart b/lib/providers/sync_status_controller.dart index d574125..46b44f6 100644 --- a/lib/providers/sync_status_controller.dart +++ b/lib/providers/sync_status_controller.dart @@ -20,10 +20,15 @@ class SyncStatusController extends ChangeNotifier { final SessionController session; static const _prefSyncedPaths = 'ui_synced_folders'; + static const _prefSyncedPathTypes = 'ui_synced_folder_types'; static const _prefSyncEverything = 'ui_sync_everything'; static const _prefSyncOnCellular = 'ui_sync_on_cellular'; List _syncedPaths = []; + // path -> isFolder, keyed the same as [_syncedPaths] - missing entries + // (from before this map existed) default to folder, the overwhelmingly + // common case. + Map _syncedPathTypes = {}; bool _syncEverything = false; bool _syncOnCellular = false; @@ -47,6 +52,16 @@ class SyncStatusController extends ChangeNotifier { bool get isSyncingNow => _isSyncingNow; List get syncConflicts => List.unmodifiable(_syncConflicts); + /// How many of [syncedPaths] are folders (as opposed to individual + /// files) - used by the sync header's expanded summary. + int get syncedFolderCount => + _syncedPaths.where((p) => _syncedPathTypes[p] ?? true).length; + + /// How many individual files have actually been mirrored locally so far + /// - distinct from [syncedFolderCount]/[syncedPaths], which are just the + /// configured *targets*, not what's actually landed on disk yet. + int get syncedItemCount => _syncedFileIds.length; + SyncHeaderStatus get syncHeaderStatus { if (_syncConflicts.isNotEmpty) return SyncHeaderStatus.alert; if (_isSyncingNow) return SyncHeaderStatus.syncing; @@ -54,7 +69,47 @@ class SyncStatusController extends ChangeNotifier { return SyncHeaderStatus.done; } + /// True if [path] itself, or an ancestor of it, is covered by device + /// sync - either explicitly (one of [_syncedPaths]) or via + /// [_syncEverything]. Shared by [syncStatusFor] (folders) and + /// [localSyncedFilePath] (files). + bool _isPathInSyncScope(String path) { + if (_syncEverything) return true; + final normalized = path.endsWith('/') + ? path.substring(0, path.length - 1) + : path; + return _syncedPaths.any((folder) { + final f = folder.endsWith('/') + ? folder.substring(0, folder.length - 1) + : folder; + return normalized == f || normalized.startsWith('$f/'); + }); + } + + /// True if any pending conflict falls under [folderPath]. + bool _folderHasConflict(String folderPath) { + final normalized = folderPath.endsWith('/') + ? folderPath.substring(0, folderPath.length - 1) + : folderPath; + return _syncConflicts.any((c) { + final p = c.remotePath.endsWith('/') + ? c.remotePath.substring(0, c.remotePath.length - 1) + : c.remotePath; + return p == normalized || p.startsWith('$normalized/'); + }); + } + + /// Folders don't get their own entry in [_syncedFileIds] (only individual + /// files do - see `SyncEngine.diffFolder`'s `if (entry.isFolder) continue`), + /// so a folder's status is derived from whether it's in sync scope at all + /// rather than tracked per-item like a file's is. SyncItemStatus syncStatusFor(NextcloudItem item) { + if (item.isFolder) { + if (!_isPathInSyncScope(item.path)) return SyncItemStatus.none; + if (_folderHasConflict(item.path)) return SyncItemStatus.conflict; + if (_isSyncingNow) return SyncItemStatus.syncing; + return SyncItemStatus.synced; + } if (_syncingFileIds.contains(item.id)) return SyncItemStatus.syncing; if (_syncConflicts.any((c) => c.fileId == item.id)) { return SyncItemStatus.conflict; @@ -71,6 +126,7 @@ class SyncStatusController extends ChangeNotifier { void _onAccountCleared() { _syncedPaths = []; + _syncedPathTypes = {}; _syncEverything = false; _isSyncingNow = false; _syncingFileIds = {}; @@ -95,6 +151,19 @@ class SyncStatusController extends ChangeNotifier { } else { _syncedPaths = []; } + final typesJson = prefs.getString(k(_prefSyncedPathTypes)); + if (typesJson != null) { + try { + _syncedPathTypes = (jsonDecode(typesJson) as Map).map( + (k, v) => MapEntry(k as String, v as bool), + ); + } catch (e) { + debugPrint('[SyncStatusController] Synced path types restore failed: $e'); + _syncedPathTypes = {}; + } + } else { + _syncedPathTypes = {}; + } _syncEverything = prefs.getBool(k(_prefSyncEverything)) ?? false; notifyListeners(); } @@ -117,30 +186,40 @@ class SyncStatusController extends ChangeNotifier { void _persistSyncedPaths() { final id = session.activeAccountId; if (id == null) return; - session.prefsFuture.then( - (p) => p.setString( + session.prefsFuture.then((p) { + p.setString( session.accountStore.accountPrefKey(id, _prefSyncedPaths), jsonEncode(_syncedPaths), - ), - ); + ); + p.setString( + session.accountStore.accountPrefKey(id, _prefSyncedPathTypes), + jsonEncode(_syncedPathTypes), + ); + }); } bool isPathSynced(String path) => _syncedPaths.contains(path); - void addSyncedPath(String path) { + void addSyncedPath(String path, {required bool isFolder}) { if (_syncedPaths.contains(path)) return; _syncedPaths = [..._syncedPaths, path]; + _syncedPathTypes = {..._syncedPathTypes, path: isFolder}; notifyListeners(); _persistSyncedPaths(); unawaited(SyncService.reschedule(session, this)); } + /// Unsyncs [path] and deletes its already-downloaded local mirror (see + /// `SyncService.removeLocalSync`) - stopping sync alone would leave + /// whatever had already been downloaded sitting on disk indefinitely. void removeSyncedPath(String path) { if (!_syncedPaths.contains(path)) return; _syncedPaths = _syncedPaths.where((f) => f != path).toList(); + _syncedPathTypes = {..._syncedPathTypes}..remove(path); notifyListeners(); _persistSyncedPaths(); unawaited(SyncService.reschedule(session, this)); + unawaited(SyncService.removeLocalSync(session, path)); } void setSyncEverything(bool value) { @@ -192,18 +271,7 @@ class SyncStatusController extends ChangeNotifier { Future localSyncedFilePath(NextcloudItem item) async { final id = session.activeAccountId; if (id == null) return null; - final itemPath = item.path.endsWith('/') - ? item.path.substring(0, item.path.length - 1) - : item.path; - final isSynced = - _syncEverything || - _syncedPaths.any((folder) { - final f = folder.endsWith('/') - ? folder.substring(0, folder.length - 1) - : folder; - return itemPath == f || itemPath.startsWith('$f/'); - }); - if (!isSynced) return null; + if (!_isPathInSyncScope(item.path)) return null; return SyncService.localSyncedFilePath(id, item.path); } diff --git a/lib/services/sync_service.dart b/lib/services/sync_service.dart index 40e8dfc..79b68df 100644 --- a/lib/services/sync_service.dart +++ b/lib/services/sync_service.dart @@ -149,6 +149,23 @@ class SyncService { }); } + /// Deletes [path]'s local mirror and its recorded sync state - called + /// when the user turns sync off for a path, so the on-device copy + /// actually goes away instead of just stopping future updates. See + /// `SyncEngine.removeLocalSync`'s doc comment for why the state also has + /// to be cleared, not just the files. + static Future removeLocalSync( + SessionController session, + String path, + ) async { + final accountId = session.activeAccountId; + if (accountId == null) return; + await _channel.invokeMethod('removeLocalSync', { + 'accountId': accountId, + 'path': path, + }); + } + /// The deterministic local mirror path for [remoteItemPath] under /// account [accountId] - mirrors `SyncEngine.kt#syncRoot`'s /// `/sync//...` layout exactly, so this diff --git a/lib/views/account_view.dart b/lib/views/account_view.dart index e4ff0fa..267a398 100644 --- a/lib/views/account_view.dart +++ b/lib/views/account_view.dart @@ -823,7 +823,15 @@ class _DeviceSyncCardState extends State<_DeviceSyncCard> { trailing: IconButton( icon: const Icon(Icons.close_rounded), tooltip: 'Stop syncing', - onPressed: () => sync.removeSyncedPath(folder), + onPressed: () { + sync.removeSyncedPath(folder); + ScaffoldMessenger.of(context).showSnackBar( + const SnackBar( + content: Text('Removed from device sync'), + behavior: SnackBarBehavior.floating, + ), + ); + }, ), ), const Divider(height: 1, indent: 16, endIndent: 16), diff --git a/lib/views/files_view.dart b/lib/views/files_view.dart index 120a587..2df04bd 100644 --- a/lib/views/files_view.dart +++ b/lib/views/files_view.dart @@ -292,18 +292,15 @@ class _FilesViewState extends State label: 'Rename', onTap: () => _renameItem(selected.single), ), - if (selected.length == 1) - SelectionAction( - icon: sync.isPathSynced(selected.single.path) - ? Icons.sync_rounded - : Icons.sync_outlined, - label: sync.isPathSynced(selected.single.path) - ? 'Stop syncing to device' - : 'Sync to device', - onTap: () => sync.isPathSynced(selected.single.path) - ? sync.removeSyncedPath(selected.single.path) - : sync.addSyncedPath(selected.single.path), - ), + SelectionAction( + icon: selected.every((i) => sync.isPathSynced(i.path)) + ? Icons.sync_rounded + : Icons.sync_outlined, + label: selected.every((i) => sync.isPathSynced(i.path)) + ? 'Stop syncing to device' + : 'Sync to device', + onTap: () => _toggleSyncSelected(context, sync, selected), + ), if (selected.length == 1) SelectionAction( icon: Icons.info_outline_rounded, @@ -807,6 +804,40 @@ class _FilesViewState extends State _clearSelection(); } + /// Toggles device sync for every selected item at once - if they're all + /// already synced this stops syncing all of them (and deletes their local + /// mirrors, see `SyncStatusController.removeSyncedPath`), otherwise it + /// starts syncing whichever ones aren't synced yet. + void _toggleSyncSelected( + BuildContext context, + SyncStatusController sync, + List items, + ) { + final allSynced = items.every((i) => sync.isPathSynced(i.path)); + if (allSynced) { + for (final item in items) { + sync.removeSyncedPath(item.path); + } + } else { + for (final item in items) { + if (!sync.isPathSynced(item.path)) { + sync.addSyncedPath(item.path, isFolder: item.isFolder); + } + } + } + _clearSelection(); + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text( + allSynced + ? 'Removed ${items.length} item(s) from device sync' + : 'Syncing ${items.length} item(s) to this device', + ), + behavior: SnackBarBehavior.floating, + ), + ); + } + /// Hands the whole batch off to `DownloadService.kt` (see its doc /// comment) rather than downloading each item in Dart then prompting /// `file_saver` per file - same reasoning as `UploadService`/ diff --git a/lib/widgets/synced_header_scaffold.dart b/lib/widgets/synced_header_scaffold.dart index b6045ab..530547b 100644 --- a/lib/widgets/synced_header_scaffold.dart +++ b/lib/widgets/synced_header_scaffold.dart @@ -21,6 +21,27 @@ import '../providers/sync_status_controller.dart'; }; } +/// The expanded sync panel's headline - actual folder/item counts instead +/// of just repeating the compact chip's generic "Synced" label, which reads +/// as true even when nothing has actually finished syncing yet (e.g. right +/// after adding a folder, before the first pass completes). +String _syncSummary(SyncStatusController sync) { + if (sync.syncConflicts.isNotEmpty) return 'Sync issue'; + if (sync.isSyncingNow) return 'Syncing…'; + final items = sync.syncedItemCount; + if (sync.syncEverything) { + return items > 0 ? '$items item${items == 1 ? '' : 's'} synced' : 'Sync off'; + } + final folders = sync.syncedFolderCount; + if (folders == 0) return 'Sync off'; + final folderWord = folders == 1 ? 'folder' : 'folders'; + if (items == 0) { + return '$folders $folderWord selected — nothing synced yet'; + } + final itemWord = items == 1 ? 'item' : 'items'; + return '$folders $folderWord & $items $itemWord synced'; +} + String formatBytes(int bytes) { if (bytes <= 0) return '0 B'; if (bytes < 1024) return '$bytes B'; @@ -361,8 +382,7 @@ class _SyncedStretchPanel extends StatelessWidget { // Reaches 1.0 (title fully hidden) at 50px of pull — comfortably // before the 100px lock threshold. final progress = forceVisible ? 1.0 : (stretch / 50).clamp(0.0, 1.0); - final status = syncStatus.syncHeaderStatus; - final (_, statusLabel) = _syncHeaderDisplay(status); + final statusLabel = _syncSummary(syncStatus); final conflicts = syncStatus.syncConflicts; return Stack(