PE-9013: Add individual drive sync and sync-on-login toggle#2114
Open
vilenarios wants to merge 33 commits intodevfrom
Open
PE-9013: Add individual drive sync and sync-on-login toggle#2114vilenarios wants to merge 33 commits intodevfrom
vilenarios wants to merge 33 commits intodevfrom
Conversation
- Add syncAllDrivesOnLogin preference to UserPreferences model - Add saveSyncAllDrivesOnLogin method to UserPreferencesRepository - Add syncSingleDrive method to SyncRepository for per-drive sync - Add startSyncForDrive method to SyncCubit - Add "Sync All Drives on Login" toggle in Advanced Settings - Add "Sync This Drive" and "Deep Sync This Drive" menu items to drive kebab menu (desktop and mobile) - Add localization strings for all 6 supported languages Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The uploaded file is already written to the local database during upload, and refreshDriveDataTable() handles the UI update. The full-drives sync was unnecessary and caused performance issues for users with many drives. Transaction status updates will occur during the normal periodic sync cycle. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix login freeze caused by GlobalHideBloc infinite loop when preferences stream emits (add SyncShowHiddenState event to break cycle) - Add sync modal title to show "Syncing All Drives" vs "Syncing Drive" with drive name for single-drive syncs - Emit initial sync progress after SyncInProgress state and add initialData to StreamBuilder to prevent empty modal flash - Persist syncAllDrivesOnLogin preference across sessions (not cleared on logout) - Add AppTopBar to DriveDetailLoadInProgress state so top bar shows during initial sync - Add unsynced drive detection and DriveDetailUnsyncedCard UI - Add sync/deep sync/snapshot to drive kebab menu on mobile - Add titleWidget support to ProgressDialog and ArDriveStandardModalNew Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… PE-9013 - Redesign unsynced drive card to match empty state pattern with confetti - Add two action cards: Sync This Drive (primary) and Sync All Drives - Add More Info support for unsynced drives on desktop (side panel) and mobile (full screen) - Add DriveDetailLoadUnsynced state properties for showDriveInfo and selectedItem - Add localization strings for syncAllDrives in all languages Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix missing asset: change ardriveLogoOnboarding from non-existent 2x.png to ArDrive.png - Fix RangeError in tutorial: add bounds checking in _goToPage() to prevent index out of range when users click rapidly through tutorial pages Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace CircularProgressIndicator with ArDrive plate stacking animation in ProgressDialog (affects sync, uploads, manifests, drive creation, etc.) - Remove confetti from unsynced drive card (confetti suits celebratory moments, not "action required" states) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…Started flash PE-9013 - Add SyncLoadingDrives state for UI feedback during metadata-only sync - Emit SyncLoadingDrives in syncMetadataOnly() without blocking waitCurrentSync() - Show "Loading your drives..." modal when SyncLoadingDrives is active - Prevents confusing "Getting Started" page flash when auto sync is disabled Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…9013 - Add custom hoverable New button with darken effect on hover - Add "Create New" tooltip to New button - Add "User Profile" tooltip to Profile card - Add localization strings for tooltips in all languages Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The wallet popup (ArConnect/Wander) causes the browser to consider the tab unfocused momentarily after login. This was causing the initial sync to be skipped, resulting in users seeing an empty state despite having drives. Added skipTabVisibilityCheck parameter to startSync() and set it to true for the initial sync in createSyncStream(). Since the user just completed wallet interaction, we know they're active and should proceed with sync. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add null check for _currentUserPreferences in clear() - Call load() first if preferences haven't been loaded yet - Prevents potential null pointer exception during logout Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughAdds unsynced-drive state/UI and single-drive sync flows, metadata-only sync on login via new preference, per-drive sync implementation and progress updates, ProgressDialog titleWidget support, settings/profile toggles, localization strings, and related repository/cubit wiring and UI action hookups. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as DriveDetail UI
participant DC as DriveDetailCubit
participant SC as SyncCubit
participant SR as SyncRepository
participant Backend as Arweave Backend
User->>UI: Open drive detail
UI->>DC: changeDrive(driveId)
alt drive.lastBlockHeight == null or 0
DC->>DC: Emit DriveDetailLoadUnsynced
DC-->>UI: Render unsynced UI
User->>UI: Tap "Sync this drive"
UI->>DC: syncCurrentDrive()
DC->>DC: Emit DriveDetailLoadInProgress
DC->>SC: startSyncForDrive(driveId)
SC->>SR: stream syncSingleDrive(driveId)
SR->>Backend: Fetch drive metadata & files
SR-->>SC: Stream SyncProgress updates
SC-->>UI: Update progress modal
SR-->>SC: Stream completes
SC->>DC: Notify completion
DC->>DC: openFolder(root)
DC-->>UI: Show synced contents
else
DC->>DC: Emit DriveDetailLoadSuccess
DC-->>UI: Show contents
end
sequenceDiagram
participant Shell as App Shell
participant SC as SyncCubit
participant Repo as UserPreferencesRepository
participant SR as SyncRepository
participant Backend as Arweave Backend
Shell->>SC: createSyncStream()
SC->>Repo: read currentPreferences / watch()
alt syncAllDrivesOnLogin == true
SC->>SR: startSync(skipTabVisibilityCheck: true)
else
SC->>SR: syncMetadataOnly()
SR->>Backend: Fetch user drives metadata
SR-->>SC: Emit SyncLoadingDrives then SyncIdle
SC-->>Shell: Shell shows/dismisses loading modal
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/blocs/hide/global_hide_bloc.dart (1)
19-36:⚠️ Potential issue | 🟠 MajorStream subscription is not cancelled on bloc disposal.
The subscription to
_userPreferencesRepository.watch()is created in the constructor but never stored or cancelled. This can cause memory leaks and unexpected behavior if the bloc is disposed while the stream is still active.🔧 Proposed fix to properly manage stream subscription
class GlobalHideBloc extends Bloc<GlobalHideEvent, GlobalHideState> { final UserPreferencesRepository _userPreferencesRepository; final DriveDao _driveDao; + StreamSubscription<UserPreferences>? _preferencesSubscription; GlobalHideBloc({ required UserPreferencesRepository userPreferencesRepository, required DriveDao driveDao, }) : _userPreferencesRepository = userPreferencesRepository, _driveDao = driveDao, super(const GlobalHideInitial(userHasHiddenDrive: false)) { // Listen to preferences stream to update state when preferences change. // Note: We only update local state here, NOT save back to preferences. // Saving is done only in response to user actions (ToggleShowHiddenFiles). - _userPreferencesRepository.watch().listen((userPreferences) async { + _preferencesSubscription = _userPreferencesRepository.watch().listen((userPreferences) async { if (userPreferences.showHiddenFiles) { add(SyncShowHiddenState( showHidden: true, userHasHiddenItems: userPreferences.userHasHiddenDrive, )); } else { add(SyncShowHiddenState( showHidden: false, userHasHiddenItems: userPreferences.userHasHiddenDrive, )); } }); _userPreferencesRepository.load(); // ... rest of constructor } + + `@override` + Future<void> close() { + _preferencesSubscription?.cancel(); + return super.close(); + } }You'll also need to add the import at the top of the file:
import 'dart:async';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/blocs/hide/global_hide_bloc.dart` around lines 19 - 36, Store the stream subscription returned by _userPreferencesRepository.watch() into a field (e.g., StreamSubscription _prefsSub) instead of ignoring it, add the import 'dart:async', and cancel it when the bloc is disposed by overriding close()/dispose (for example override Future<void> close() async { await _prefsSub.cancel(); return super.close(); }). Keep the existing listener logic (adding SyncShowHiddenState) but assign the subscription with _prefsSub = _userPreferencesRepository.watch().listen(...).lib/pages/drive_detail/components/drive_explorer_item_tile.dart (1)
931-986:⚠️ Potential issue | 🟠 MajorPotential null pointer dereference on
drivefield.The
drivefield is nullable (Drive? driveat line 838), but this code uses force unwrap (drive!) multiple times. IfEntityActionsMenuis instantiated withdrive: nullfor aDriveDataItem, these lines will throw a null pointer exception at runtime.Consider adding a null check guard or ensuring
driveis non-null whenitem is DriveDataItem:🛡️ Proposed fix to add null safety
} else if (item is DriveDataItem) { + if (drive == null) { + // Return minimal menu if drive context is unavailable + return [ + ArDriveDropdownItem( + onClick: () { + final bloc = context.read<DriveDetailCubit>(); + bloc.selectDataItem(item); + }, + content: _buildItem( + appLocalizationsOf(context).moreInfo, + ArDriveIcons.info(size: defaultIconSize), + ), + ) + ]; + } return [ ArDriveDropdownItem(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/pages/drive_detail/components/drive_explorer_item_tile.dart` around lines 931 - 986, The code dereferences the nullable field drive via drive! in several action handlers (promptToRenameDrive, promptToCreateSnapshot, and SyncCubit.startSyncForDrive) which can throw if drive is null; update the widget to guard against null by checking drive != null before building those ArDriveDropdownItem entries (e.g., change the conditions to isOwner && drive != null for rename/create-snapshot and similarly ensure drive is non-null before calling startSyncForDrive), or early-return/skip rendering the menu when drive is null so all uses of drive (drive!.id, drive!.name, promptToCreateSnapshot(context, drive!), etc.) are safe.
🧹 Nitpick comments (3)
lib/components/progress_dialog.dart (1)
86-99: Edge case:titleWidgetwithouttitlewhenuseNewArDriveUIis false.If a caller provides only
titleWidget(withtitlebeing null) butuseNewArDriveUIis false, line 96 passes an empty string toArDriveStandardModal. SinceArDriveStandardModal.titleisString?and checksif (title != null)at line 407, the empty string will cause an empty title row to render.This is likely a rare edge case since
titleWidgetis primarily intended for the new UI, but consider adding a warning comment or validating the usage:📝 Suggested documentation
final bool useNewArDriveUI; + // Note: titleWidget is only supported when useNewArDriveUI is true. + // When useNewArDriveUI is false, title must be provided. `@override` Widget build(BuildContext context) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/components/progress_dialog.dart` around lines 86 - 99, The current branch returns ArDriveStandardModal with title: title ?? '' when useNewArDriveUI is false, which causes an empty string to be treated as a non-null title and render an empty title row if a caller provided titleWidget but left title null; update the logic in the factory/selector (the block that checks useNewArDriveUI) to pass null instead of an empty string to ArDriveStandardModal when title is null and titleWidget is non-null (or add a clear comment/validation) so ArDriveStandardModal's null check (title != null) behaves correctly; locate the symbols useNewArDriveUI, ArDriveStandardModalNew, ArDriveStandardModal, titleWidget, and title to implement this change.lib/pages/drive_detail/components/drive_detail_unsynced_card.dart (1)
77-159: Keep the unsynced drive menus aligned with the regular drive menu.
_buildMenuItems()and_buildMobileMenuItems()now duplicate the same action list, and they have already drifted fromdrive_detail_page.dart, Lines 643-678 and 1366-1393: the regular drive kebab exposesdeepSyncThisDrive, but the new unsynced menus do not. Extract a shared builder/config and add the missing deep-sync entry so desktop/mobile unsynced flows stay in sync.Also applies to: 218-300
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/pages/drive_detail/components/drive_detail_unsynced_card.dart` around lines 77 - 159, The unsynced drive menus in _buildMenuItems and _buildMobileMenuItems duplicate actions and are missing the deep-sync action present in the regular drive kebab; refactor by extracting a shared menu builder/config (e.g., a private helper like _buildSharedDriveActions or a List<ArDriveDropdownItem> factory) and have both _buildMenuItems and _buildMobileMenuItems call it, then add the missing deepSync action (same handler used in drive_detail_page.dart’s deepSyncThisDrive kebab) into that shared list so desktop and mobile unsynced flows match the regular drive menu; ensure you reuse DriveDataTableItemMapper.fromDrive and existing cubit methods (syncCurrentDrive, selectDriveInfoForUnsyncedDrive) when constructing the shared actions.lib/sync/domain/repositories/sync_repository.dart (1)
800-919: Consider extracting shared logic to reduce duplication.
_updateTransactionStatusesForDriveshares ~80% of its code with_updateTransactionStatuses. The only difference is the initial filtering step. Consider extracting the shared batch-processing logic into a common helper method that accepts pre-filtered transactions.This is a non-blocking suggestion for future maintenance improvement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sync/domain/repositories/sync_repository.dart` around lines 800 - 919, The two methods _updateTransactionStatusesForDrive and _updateTransactionStatuses duplicate the same batching, confirmation-fetching and DB-write logic; extract that shared core into a new helper (e.g., _processPendingTransactionsBatch or _updateStatusesForPendingMap) which accepts the pre-filtered pendingTxMap (Map<String, NetworkTransaction>), parameters like txsIdsToSkip, arweave service, driveDao, cancellationToken and kRequiredTxConfirmationCount, and performs paging, calling arweave.getTransactionConfirmations, computing txConfirmed/txNotFound, resolving transactionDateCreated (by delegating to _getDateCreatedByDataTx when null), writing status updates and marking skipped txs; then call this helper from both _updateTransactionStatusesForDrive (after filtering driveDataTxIds) and _updateTransactionStatuses to remove the ~80% duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/app_shell.dart`:
- Around line 703-745: _getSyncTitle currently returns the generic ArConnect
"please remain on this tab" string whenever isArConnect is true, preventing the
modal from showing the sync-specific titles; change _getSyncTitle to choose
syncingSingleDrive or syncingAllDrives based on SyncProgress.isSingleDriveSync
(and drive-specific text when available) regardless of isArConnect and reserve
the ArConnect warning for a separate UI element or appended notice instead of
replacing the title; similarly, update _getSyncProgressDescription to not return
an empty string when SyncProgress.drivesCount == 0 — if
syncProgress.isSingleDriveSync is true return syncingDriveWithName (when
driveName != null) or syncingOnlyOneDrive as the fallback, and only use the
drivesSynced/drivesCount plural form when drivesCount > 1 so ArConnect users
immediately see the correct sync title/description (refer to _getSyncTitle,
_getSyncProgressDescription, SyncProgress.isSingleDriveSync,
SyncProgress.drivesCount, and SyncProgress.driveName).
In `@lib/blocs/drive_detail/drive_detail_cubit.dart`:
- Around line 112-118: The unsynced early-return path in drive_detail_cubit.dart
skips resetting the previous selection, allowing stale _selectedItem to leak
into later sync/openFolder flows; before emitting DriveDetailLoadUnsynced (and
after cancelling _folderSubscription and setting _driveId), clear the selection
state by resetting _selectedItem (and any related selection fields if present)
to null so syncCurrentDrive()/openFolder() cannot reuse a selection from the
previous drive, then emit DriveDetailLoadUnsynced(drive: drive).
In `@lib/blocs/drive_detail/drive_detail_state.dart`:
- Around line 141-168: The DetailsPanel's BlocBuilder unconditionally casts
driveDetailState to DriveDetailLoadSuccess which crashes when
DriveDetailLoadUnsynced is active; update the BlocBuilder in DetailsPanel to
first check the runtime type (e.g., if (driveDetailState is
DriveDetailLoadSuccess) { final success = driveDetailState as
DriveDetailLoadSuccess; ... }) and only build the toolbar or access success-only
fields inside that branch, otherwise render the appropriate UI for
DriveDetailLoadUnsynced (or return a fallback widget). Ensure the type guard
references the DriveDetailLoadSuccess and DriveDetailLoadUnsynced classes and
the local driveDetailState variable so the unsafe cast is removed.
In `@lib/l10n/app_es.arb`:
- Around line 1729-1754: The Spanish localization is missing the
"loadingYourDrives" key used by AppShell via appLocalizationsOf(context). Add a
"loadingYourDrives" entry (with an accompanying "@loadingYourDrives" description
object) into lib/l10n/app_es.arb so the plural/placeholder-free key matches
other entries like "syncingSingleDrive" and "syncNow" and provides a Spanish
string (e.g., "Cargando sus unidades" or similar) to ensure the modal title is
localized for Spanish users.
In `@lib/sync/domain/repositories/sync_repository.dart`:
- Around line 860-865: The loop that reads confirmations uses the null-check
operator on confirmations[txId] which can throw if the map is empty; update the
logic in the method containing that loop (around the driveDao.transaction block)
to guard lookups by using confirmations.containsKey(txId) or a null-aware
default (e.g., treat missing entries as -1 or 0 depending on semantics) before
comparing to kRequiredTxConfirmationCount, and apply the same guard/fallback in
the _updateTransactionStatuses method where confirmations[txId]! is used so
missing keys do not cause a Null check operator error.
---
Outside diff comments:
In `@lib/blocs/hide/global_hide_bloc.dart`:
- Around line 19-36: Store the stream subscription returned by
_userPreferencesRepository.watch() into a field (e.g., StreamSubscription
_prefsSub) instead of ignoring it, add the import 'dart:async', and cancel it
when the bloc is disposed by overriding close()/dispose (for example override
Future<void> close() async { await _prefsSub.cancel(); return super.close(); }).
Keep the existing listener logic (adding SyncShowHiddenState) but assign the
subscription with _prefsSub = _userPreferencesRepository.watch().listen(...).
In `@lib/pages/drive_detail/components/drive_explorer_item_tile.dart`:
- Around line 931-986: The code dereferences the nullable field drive via drive!
in several action handlers (promptToRenameDrive, promptToCreateSnapshot, and
SyncCubit.startSyncForDrive) which can throw if drive is null; update the widget
to guard against null by checking drive != null before building those
ArDriveDropdownItem entries (e.g., change the conditions to isOwner && drive !=
null for rename/create-snapshot and similarly ensure drive is non-null before
calling startSyncForDrive), or early-return/skip rendering the menu when drive
is null so all uses of drive (drive!.id, drive!.name,
promptToCreateSnapshot(context, drive!), etc.) are safe.
---
Nitpick comments:
In `@lib/components/progress_dialog.dart`:
- Around line 86-99: The current branch returns ArDriveStandardModal with title:
title ?? '' when useNewArDriveUI is false, which causes an empty string to be
treated as a non-null title and render an empty title row if a caller provided
titleWidget but left title null; update the logic in the factory/selector (the
block that checks useNewArDriveUI) to pass null instead of an empty string to
ArDriveStandardModal when title is null and titleWidget is non-null (or add a
clear comment/validation) so ArDriveStandardModal's null check (title != null)
behaves correctly; locate the symbols useNewArDriveUI, ArDriveStandardModalNew,
ArDriveStandardModal, titleWidget, and title to implement this change.
In `@lib/pages/drive_detail/components/drive_detail_unsynced_card.dart`:
- Around line 77-159: The unsynced drive menus in _buildMenuItems and
_buildMobileMenuItems duplicate actions and are missing the deep-sync action
present in the regular drive kebab; refactor by extracting a shared menu
builder/config (e.g., a private helper like _buildSharedDriveActions or a
List<ArDriveDropdownItem> factory) and have both _buildMenuItems and
_buildMobileMenuItems call it, then add the missing deepSync action (same
handler used in drive_detail_page.dart’s deepSyncThisDrive kebab) into that
shared list so desktop and mobile unsynced flows match the regular drive menu;
ensure you reuse DriveDataTableItemMapper.fromDrive and existing cubit methods
(syncCurrentDrive, selectDriveInfoForUnsyncedDrive) when constructing the shared
actions.
In `@lib/sync/domain/repositories/sync_repository.dart`:
- Around line 800-919: The two methods _updateTransactionStatusesForDrive and
_updateTransactionStatuses duplicate the same batching, confirmation-fetching
and DB-write logic; extract that shared core into a new helper (e.g.,
_processPendingTransactionsBatch or _updateStatusesForPendingMap) which accepts
the pre-filtered pendingTxMap (Map<String, NetworkTransaction>), parameters like
txsIdsToSkip, arweave service, driveDao, cancellationToken and
kRequiredTxConfirmationCount, and performs paging, calling
arweave.getTransactionConfirmations, computing txConfirmed/txNotFound, resolving
transactionDateCreated (by delegating to _getDateCreatedByDataTx when null),
writing status updates and marking skipped txs; then call this helper from both
_updateTransactionStatusesForDrive (after filtering driveDataTxIds) and
_updateTransactionStatuses to remove the ~80% duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e3fbf6f-53ce-4ed4-8a24-34a25c2951ff
📒 Files selected for processing (30)
lib/app_shell.dartlib/authentication/login/views/tutorials_view.dartlib/blocs/drive_detail/drive_detail_cubit.dartlib/blocs/drive_detail/drive_detail_state.dartlib/blocs/hide/global_hide_bloc.dartlib/blocs/hide/global_hide_event.dartlib/components/details_panel.dartlib/components/new_button/new_button.dartlib/components/profile_card.dartlib/components/progress_dialog.dartlib/components/settings_popover.dartlib/components/upload_form.dartlib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_hi.arblib/l10n/app_ja.arblib/l10n/app_zh-HK.arblib/l10n/app_zh.arblib/misc/resources.dartlib/pages/app_router_delegate.dartlib/pages/drive_detail/components/drive_detail_unsynced_card.dartlib/pages/drive_detail/components/drive_explorer_item_tile.dartlib/pages/drive_detail/drive_detail_page.dartlib/sync/domain/cubit/sync_cubit.dartlib/sync/domain/cubit/sync_state.dartlib/sync/domain/repositories/sync_repository.dartlib/sync/domain/sync_progress.dartlib/user/repositories/user_preferences_repository.dartlib/user/user_preferences.dartpackages/ardrive_ui/lib/src/components/modal.dart
💤 Files with no reviewable changes (1)
- lib/components/upload_form.dart
- Changed setUpAll to setUp for proper test isolation - Added syncAllDrivesOnLogin stub to all tests that call load() - Added load() calls to save* tests that need initialized preferences - Added queue.skip(4) to watch test to skip intermediate save emissions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/user/repositories/user_preferences_repository_test.dart (2)
16-265:⚠️ Potential issue | 🟡 MinorMissing test for
saveSyncAllDrivesOnLoginmethod.The PR adds save/load support for
syncAllDrivesOnLogin, and the load behavior is tested. However, there's no test verifying the save behavior (similar to the existing tests forsaveShowHiddenFiles,saveUserHasHiddenItem, etc.).Would you like me to generate a test case for
saveSyncAllDrivesOnLogin? It would follow the same pattern as the other save tests:test('should save syncAllDrivesOnLogin preference to storage', () async { // Setup initial load when(() => mockStore.getString('currentTheme')).thenReturn('dark'); when(() => mockStore.getString('lastSelectedDriveId')).thenReturn(null); when(() => mockStore.getBool('showHiddenFiles')).thenReturn(false); when(() => mockStore.getBool('userHasHiddenDrive')).thenReturn(false); when(() => mockStore.getBool('syncAllDrivesOnLogin')).thenReturn(true); await repository.load(); when(() => mockStore.putBool('syncAllDrivesOnLogin', false)) .thenAnswer((_) async => true); await repository.saveSyncAllDrivesOnLogin(false); verify(() => mockStore.putBool('syncAllDrivesOnLogin', false)).called(1); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/user/repositories/user_preferences_repository_test.dart` around lines 16 - 265, Add a new test mirroring the other "save" tests to assert saveSyncAllDrivesOnLogin calls the store: in the test, arrange initial repository.load() state (mockStore.getString/getBool returns same defaults as other tests), stub mockStore.putBool('syncAllDrivesOnLogin', false) to return true, call repository.saveSyncAllDrivesOnLogin(false), and verify mockStore.putBool('syncAllDrivesOnLogin', false) was called once; reference the repository.saveSyncAllDrivesOnLogin method and mockStore.putBool('syncAllDrivesOnLogin', ...).
171-191:⚠️ Potential issue | 🟡 MinorAdd assertion to verify
syncAllDrivesOnLoginis preserved during clear.The PR objectives mention "Fixed syncAllDrivesOnLogin preference being reset on logout." This test should verify that
syncAllDrivesOnLoginis not removed duringclear(), ensuring the bug fix is properly covered.Proposed fix to verify preference preservation
await repository.clear(); verify(() => mockStore.remove('lastSelectedDriveId')).called(1); + verifyNever(() => mockStore.remove('syncAllDrivesOnLogin')); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/user/repositories/user_preferences_repository_test.dart` around lines 171 - 191, Add an assertion after await repository.clear() to ensure syncAllDrivesOnLogin is preserved by verifying mockStore.remove('syncAllDrivesOnLogin') was not called; specifically, keep the existing verify(() => mockStore.remove('lastSelectedDriveId')).called(1) and add verifyNever(() => mockStore.remove('syncAllDrivesOnLogin')) (or verify(...).called(0)) to the test that uses repository.load() and repository.clear() so the test ensures syncAllDrivesOnLogin isn't removed.
🧹 Nitpick comments (1)
test/user/repositories/user_preferences_repository_test.dart (1)
243-258: Consider using a more robust approach thanskip(4).The
skip(4)is tightly coupled to the exact number of save methods called. If a save method is added, removed, or reordered, this test will either hang (waiting for more emissions) or skip the wrong emissions.Consider alternatives:
- Use
expectLaterwith a matcher that waits for a specific condition- Collect all emissions and assert on the final state
- Use
take()with a known count and assert on the last elementAlternative approach using emission collection
- // Skip intermediate emissions from save methods (4 emissions) - // and get the final state after load() await repository.load(); - // Skip the 4 intermediate emissions from save* methods - await queue.skip(4); - + // Collect all remaining emissions up to and including load() + final emissions = <UserPreferences>[]; + for (var i = 0; i < 5; i++) { + emissions.add(await queue.next); + } + expect( - await queue.next, + emissions.last, const UserPreferences( currentTheme: ArDriveThemes.dark, lastSelectedDriveId: 'new_drive_id', showHiddenFiles: true, userHasHiddenDrive: true, syncAllDrivesOnLogin: true, ), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/user/repositories/user_preferences_repository_test.dart` around lines 243 - 258, The test currently relies on queue.skip(4) which is brittle; instead, change the assertion to collect emissions from the stream and assert on the final state after repository.load() — e.g., after calling repository.load(), gather emissions from queue (using a collect/consume utility or queue.take/queue.toList equivalent) and then assert that the last element equals the expected UserPreferences (instead of calling queue.skip(4) then queue.next); update references to queue.skip(4) and queue.next to the collection approach so the test no longer depends on an exact emission count.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@test/user/repositories/user_preferences_repository_test.dart`:
- Around line 16-265: Add a new test mirroring the other "save" tests to assert
saveSyncAllDrivesOnLogin calls the store: in the test, arrange initial
repository.load() state (mockStore.getString/getBool returns same defaults as
other tests), stub mockStore.putBool('syncAllDrivesOnLogin', false) to return
true, call repository.saveSyncAllDrivesOnLogin(false), and verify
mockStore.putBool('syncAllDrivesOnLogin', false) was called once; reference the
repository.saveSyncAllDrivesOnLogin method and
mockStore.putBool('syncAllDrivesOnLogin', ...).
- Around line 171-191: Add an assertion after await repository.clear() to ensure
syncAllDrivesOnLogin is preserved by verifying
mockStore.remove('syncAllDrivesOnLogin') was not called; specifically, keep the
existing verify(() => mockStore.remove('lastSelectedDriveId')).called(1) and add
verifyNever(() => mockStore.remove('syncAllDrivesOnLogin')) (or
verify(...).called(0)) to the test that uses repository.load() and
repository.clear() so the test ensures syncAllDrivesOnLogin isn't removed.
---
Nitpick comments:
In `@test/user/repositories/user_preferences_repository_test.dart`:
- Around line 243-258: The test currently relies on queue.skip(4) which is
brittle; instead, change the assertion to collect emissions from the stream and
assert on the final state after repository.load() — e.g., after calling
repository.load(), gather emissions from queue (using a collect/consume utility
or queue.take/queue.toList equivalent) and then assert that the last element
equals the expected UserPreferences (instead of calling queue.skip(4) then
queue.next); update references to queue.skip(4) and queue.next to the collection
approach so the test no longer depends on an exact emission count.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 376edbb1-a019-45f6-b74a-14271488af3f
📒 Files selected for processing (1)
test/user/repositories/user_preferences_repository_test.dart
The BlocBuilder was unconditionally casting driveDetailState to DriveDetailLoadSuccess which would crash when DriveDetailLoadUnsynced is active. Now properly checks the type before accessing success-only fields, returning SizedBox.shrink() for other states. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added missing "loadingYourDrives" translation to: - Spanish: "Cargando tus unidades..." - Hindi: "आपकी ड्राइव लोड हो रही हैं..." - Japanese: "ドライブを読み込んでいます..." - Chinese (Simplified): "正在加载您的驱动器..." - Chinese (Hong Kong): "正在載入您的磁碟機..." This key is used by AppShell when loading drive metadata on login. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Always show syncingSingleDrive or syncingAllDrives as title - Show ArConnect "please remain on tab" warning as separate notice - Fix _getSyncProgressDescription to handle single drive sync properly Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added null checks for confirmations[txId] in both _updateTransactionStatusesForDrive and _updateTransactionStatuses methods. When the API times out (returning an empty map) or returns partial results, the code now skips those transactions instead of throwing a null check operator error. The transactions will be retried on the next sync cycle. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reset _selectedItem, _selectedItems, and _refreshSelectedItem before emitting DriveDetailLoadUnsynced to prevent stale selection data from leaking into subsequent sync/openFolder flows. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- global_hide_bloc.dart: Store stream subscription and cancel on close() to prevent memory leaks - drive_explorer_item_tile.dart: Add null guards for drive before building menu items that use drive.id/drive.name - progress_dialog.dart: Pass null title when titleWidget is provided instead of empty string to avoid empty title row rendering Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Kept our syncAllDrivesOnLogin feature changes: - Preference check before initial sync - syncMetadataOnly() method for lightweight sync Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add test for saveSyncAllDrivesOnLogin storing value correctly - Add assertion verifying syncAllDrivesOnLogin is NOT removed on clear() - Refactor watch test to avoid brittle queue.skip(4) approach - Split into separate focused tests for better maintainability Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Visit the preview URL for this PR (updated for commit 85d55c7): https://ardrive-web--pr2114-pe-9013-individual-d-09hs5j5a.web.app (expires Tue, 14 Apr 2026 18:19:19 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: a224ebaee2f0939e7665e7630e7d3d6cd7d0f8b0 |
- drive_detail_page.dart: Add check for DriveDetailLoadUnsynced state to prevent redundant changeDrive() calls when clicking on a drive that's already showing the unsynced state - settings_popover.dart: Replace custom _SyncToggle with ArDriveToggleSwitch to get smooth animation matching the profile dropdown toggle Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- _updatePreference: Load preferences first if null to prevent errors when toggling settings before preferences have been loaded - Auth listener: Reload preferences when user logs IN to emit current values to stream subscribers (profile card, etc.) This fixes the issue where the sync toggle would show incorrect state after login because the stream hadn't emitted the stored value. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
lib/components/progress_dialog.dart (1)
26-42: Make thetitleWidgetcontract enforceable.
ProgressDialogdocumentstitleWidgetas taking precedence, but the new-UI branch still forwardstitlewhen both are provided, and the legacy branch still acceptstitleWidgeteven though it never renders it. Tightening that contract here will keep future call sites from depending on modal-specific behavior.♻️ Suggested cleanup
const ProgressDialog({ super.key, this.title, this.titleWidget, this.actions = const [], this.progressDescription, this.progressBar, this.percentageDetails, this.useNewArDriveUI = false, - }) : assert(title != null || titleWidget != null, - 'Either title or titleWidget must be provided'); + }) : assert( + useNewArDriveUI + ? (title != null || titleWidget != null) + : title != null, + useNewArDriveUI + ? 'Either title or titleWidget must be provided' + : 'title is required when useNewArDriveUI is false', + ); @@ if (useNewArDriveUI) { return ArDriveStandardModalNew( - // Pass null if titleWidget is provided, otherwise use title or empty string - title: titleWidget != null ? title : (title ?? ''), + title: titleWidget != null ? null : title, titleWidget: titleWidget, content: content, actions: actions, ); } return ArDriveStandardModal( - title: title ?? '', + title: title!, content: content, actions: actions, );Also applies to: 86-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/components/progress_dialog.dart` around lines 26 - 42, Enforce a strict contract between title and titleWidget by making them mutually exclusive: update the ProgressDialog constructor (and the other constructor/overload around lines 86-99) to assert that exactly one of title or titleWidget is provided (keep the existing "at least one" check and add assert(!(title != null && titleWidget != null), 'Provide only one of title or titleWidget')); then adjust call sites and the internal branches (the new-UI and legacy rendering logic that reference title, titleWidget, and useNewArDriveUI) so they no longer rely on modal-specific fallbacks—new-UI should use titleWidget when provided and legacy should not accept a titleWidget (or callers must pass null) to satisfy the new constructor contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/blocs/drive_detail/drive_detail_cubit.dart`:
- Around line 607-617: The code unconditionally calls openFolder() after
awaiting _syncCubit.startSyncForDrive(), which can complete on
cancellation/errors and may run after navigation away; instead, after the await
return, check that the local drive state is still the expected
DriveDetailLoadUnsynced (or that this.state still refers to the same drive id)
and that the _syncCubit indicates a successful sync (not a cancellation or
SyncCompleteWithErrors) before calling openFolder(folderId:
state.drive.rootFolderId); i.e., guard the openFolder call by re-reading
this.state and validating _syncCubit's state/result to ensure the sync actually
finished successfully and the user is still on the same drive before navigating
into the root folder.
In `@lib/l10n/app_zh.arb`:
- Around line 369-380: The Chinese translations for the new keys
"syncThisDrive", "deepSyncThisDrive", and "syncAllDrivesOnLogin" use
inconsistent terminology ("驱动器"/"驱动"); update their values to use the existing
project term "网盘" (e.g., change "同步此驱动器" -> "同步此网盘", "深度同步此驱动器" -> "深度同步此网盘",
"登录时同步所有驱动器" -> "登录时同步所有网盘") and scan the other affected keys referenced (around
the other ranges) to make the same replacement so all locale entries use "网盘"
consistently.
In `@lib/sync/domain/repositories/sync_repository.dart`:
- Around line 577-645: The per-sync scratch state (_ghostFolders and _folderIds)
is not reliably cleared in the single-drive flow causing stale state after
cancellation or errors; update syncSingleDrive (the async Future.microtask block
that calls _parseDriveTransactionsIntoDatabaseEntities and createGhosts) to
always clear per-run state in a finally block: remove any drive-specific entries
from the instance _ghostFolders (or clear them) and clear/reset _folderIds, and
also ensure the local driveGhostFolders is cleared/ignored on early exits;
mirror the cleanup behavior used by syncAllDrives so ghost/folder bookkeeping
does not leak into subsequent runs (changes touch
_parseDriveTransactionsIntoDatabaseEntities usage, the driveGhostFolders local
map, and the _folderIds/_ghostFolders instance fields).
- Around line 586-592: The code force-unwraps drive.lastBlockHeight when calling
_syncDrive in the single-drive path, causing a crash for DriveDetailLoadUnsynced
(when lastBlockHeight == null); change the call to treat null as 0 by passing
(drive.lastBlockHeight ?? 0) or equivalent so _calculateSyncLastBlockHeight and
_syncDrive receive 0 for null/unsynced drives (adjust where lastBlockHeight is
referenced in syncCurrentDrive/_syncDrive invocation and any use in
_calculateSyncLastBlockHeight).
---
Nitpick comments:
In `@lib/components/progress_dialog.dart`:
- Around line 26-42: Enforce a strict contract between title and titleWidget by
making them mutually exclusive: update the ProgressDialog constructor (and the
other constructor/overload around lines 86-99) to assert that exactly one of
title or titleWidget is provided (keep the existing "at least one" check and add
assert(!(title != null && titleWidget != null), 'Provide only one of title or
titleWidget')); then adjust call sites and the internal branches (the new-UI and
legacy rendering logic that reference title, titleWidget, and useNewArDriveUI)
so they no longer rely on modal-specific fallbacks—new-UI should use titleWidget
when provided and legacy should not accept a titleWidget (or callers must pass
null) to satisfy the new constructor contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26fca005-8fb8-4b5d-8601-c84bd89c0687
📒 Files selected for processing (16)
lib/app_shell.dartlib/blocs/drive_detail/drive_detail_cubit.dartlib/blocs/hide/global_hide_bloc.dartlib/components/details_panel.dartlib/components/progress_dialog.dartlib/components/settings_popover.dartlib/l10n/app_es.arblib/l10n/app_hi.arblib/l10n/app_ja.arblib/l10n/app_zh-HK.arblib/l10n/app_zh.arblib/pages/drive_detail/components/drive_explorer_item_tile.dartlib/pages/drive_detail/drive_detail_page.dartlib/sync/domain/cubit/sync_cubit.dartlib/sync/domain/repositories/sync_repository.darttest/user/repositories/user_preferences_repository_test.dart
✅ Files skipped from review due to trivial changes (1)
- lib/l10n/app_es.arb
🚧 Files skipped from review as they are similar to previous changes (9)
- lib/components/details_panel.dart
- test/user/repositories/user_preferences_repository_test.dart
- lib/l10n/app_ja.arb
- lib/l10n/app_hi.arb
- lib/pages/drive_detail/components/drive_explorer_item_tile.dart
- lib/l10n/app_zh-HK.arb
- lib/pages/drive_detail/drive_detail_page.dart
- lib/sync/domain/cubit/sync_cubit.dart
- lib/blocs/hide/global_hide_bloc.dart
…n PE-9013 - Check sync state after await to detect cancellation or failure - Verify user hasn't navigated away during sync before calling openFolder - Store driveId and rootFolderId before async operation for comparison Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
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/user/repositories/user_preferences_repository.dart (1)
189-208:⚠️ Potential issue | 🟠 MajorDon't advance in-memory state before the write completes.
Line 194 updates
_currentUserPreferencesbeforeputString/putBoolfinishes. If that write fails,UserPreferencesRepository.currentPreferencesnow reports a value that was never persisted, and the nextload()will snap it back.💡 Persist first, then publish the new snapshot
}) async { // Ensure preferences are loaded before updating if (_currentUserPreferences == null) { await load(); } - - _currentUserPreferences = updateFunction(value); final store = await _getStore(); if (value is String) { await store.putString(key, value as String); } else if (value is bool) { await store.putBool(key, value as bool); } else { throw ArgumentError('Unsupported type for preference value'); } + final updatedPreferences = updateFunction(value); + _currentUserPreferences = updatedPreferences; + // Notify listeners after save completes - if (_currentUserPreferences != null) { - _userPreferencesController.sink.add(_currentUserPreferences!); - } + _userPreferencesController.sink.add(updatedPreferences); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/user/repositories/user_preferences_repository.dart` around lines 189 - 208, The code updates the in-memory snapshot (_currentUserPreferences) before the persistent write completes; change the flow in the method that updates preferences so you first ensure preferences are loaded (call load() if _currentUserPreferences == null), then obtain the store via _getStore(), perform the asynchronous write (store.putString / store.putBool) and only after that succeeds assign the new snapshot to _currentUserPreferences via updateFunction(value) and emit it through _userPreferencesController.sink.add(...); keep the same type checks for value and surface errors appropriately (e.g., let exceptions propagate or rethrow) so the in-memory state is only advanced after a successful persistence.
♻️ Duplicate comments (1)
lib/blocs/drive_detail/drive_detail_cubit.dart (1)
674-697:⚠️ Potential issue | 🟠 MajorOnly leave the unsynced flow after the drive is actually content-synced.
SyncCubit.startSyncForDrive()also returns without syncing when another sync is already running or when tab/activity guards short-circuit (lib/sync/domain/cubit/sync_cubit.dart:371-425). Those paths currently leave this cubit inDriveDetailLoadInProgress(), and Line 697 still does not pin the drive id when reopening. Re-read the drive after the await and uselastBlockHeight > 0as the success criterion; otherwise emitDriveDetailLoadUnsynced.🩹 Suggested guard
Future<void> syncCurrentDrive() async { final state = this.state; if (state is DriveDetailLoadUnsynced) { final driveId = state.drive.id; - final rootFolderId = state.drive.rootFolderId; emit(DriveDetailLoadInProgress()); await _syncCubit.startSyncForDrive( driveId: driveId, deepSync: false, ); - // Guard: Only proceed if sync completed successfully and we're still - // viewing the same drive (user hasn't navigated away during sync) - final syncState = _syncCubit.state; - final currentState = this.state; - - // Check if sync was cancelled or had errors - if (syncState is SyncCancelled || syncState is SyncFailure) { - // Sync was cancelled or failed, don't navigate + if (isClosed || _driveId != driveId) { return; } - // Check if we're still on the same drive (user might have navigated away) - if (currentState is! DriveDetailLoadInProgress || _driveId != driveId) { + final refreshedDrive = + await _driveDao.driveById(driveId: driveId).getSingleOrNull(); + if (isClosed || _driveId != driveId) { return; } - // Sync completed successfully and we're still on the same drive - openFolder(folderId: rootFolderId); + if (refreshedDrive == null) { + emit(DriveDetailLoadNotFound()); + return; + } + + if ((refreshedDrive.lastBlockHeight ?? 0) == 0) { + emit(DriveDetailLoadUnsynced(drive: refreshedDrive)); + return; + } + + unawaited( + openFolder( + folderId: refreshedDrive.rootFolderId, + otherDriveId: refreshedDrive.id, + ), + ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/blocs/drive_detail/drive_detail_cubit.dart` around lines 674 - 697, After awaiting SyncCubit.startSyncForDrive, re-read the drive metadata (using the same method you use elsewhere to load drive info) and check its lastBlockHeight; if lastBlockHeight > 0 treat sync as successful and proceed to openFolder(rootFolderId), otherwise emit DriveDetailLoadUnsynced instead of staying in DriveDetailLoadInProgress. In other words, inside the block after the await (where you currently inspect syncState/currentState), fetch the fresh drive record, use lastBlockHeight > 0 as the success criterion, ensure _driveId still matches the requested driveId, and only call openFolder when that check passes; otherwise emit DriveDetailLoadUnsynced and do not navigate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/blocs/drive_detail/drive_detail_cubit.dart`:
- Around line 94-112: The constructor’s initial load can complete after the user
switches drives, so after each await (notably after _syncCubit.waitCurrentSync()
and after awaiting _driveDao.driveById(...).getSingleOrNull()) re-check the
current _driveId against the original driveId parameter and abort if they
differ; similarly, when calling openFolder use the current pinned drive id
(e.g., reference _driveId or an otherDriveId local snapshot) so you don't open
the folder for a stale drive; update the block around waitCurrentSync(),
driveById.getSingleOrNull(), and openFolder(...) to bail out when _driveId !=
driveId to prevent overwriting newer selections.
- Around line 131-145: Capture the drive id from the current state before
awaiting the DAO (e.g., final otherDriveId = (currentState as
DriveDetailLoadUnsynced).driveId), then after the await re-check that state is
still DriveDetailLoadUnsynced and that the stored otherDriveId still matches the
active state; only then call openFolder and ensure you pass the preserved id
(e.g., openFolder(folderId: drive.rootFolderId, driveId: otherDriveId) or
otherwise use otherDriveId in the call) so a race after the await cannot open
the wrong drive's root folder.
In `@lib/user/repositories/user_preferences_repository.dart`:
- Around line 46-53: The auth-state listener calls async methods clear() and
load() without awaiting them, allowing races that can mutate
_currentUserPreferences and the output stream out of order; fix by serializing
these operations—introduce a private Future? _authStateWork (or similar) and in
the _auth.onAuthStateChanged() handler set _authStateWork = (_authStateWork ??
Future.value()).then((_) => user == null ? clear() : load()); return the chain
(await or ignore as needed) so each clear()/load() completes before the next
starts, ensuring ordered mutations and emissions from clear(), load(), and the
stream.
---
Outside diff comments:
In `@lib/user/repositories/user_preferences_repository.dart`:
- Around line 189-208: The code updates the in-memory snapshot
(_currentUserPreferences) before the persistent write completes; change the flow
in the method that updates preferences so you first ensure preferences are
loaded (call load() if _currentUserPreferences == null), then obtain the store
via _getStore(), perform the asynchronous write (store.putString /
store.putBool) and only after that succeeds assign the new snapshot to
_currentUserPreferences via updateFunction(value) and emit it through
_userPreferencesController.sink.add(...); keep the same type checks for value
and surface errors appropriately (e.g., let exceptions propagate or rethrow) so
the in-memory state is only advanced after a successful persistence.
---
Duplicate comments:
In `@lib/blocs/drive_detail/drive_detail_cubit.dart`:
- Around line 674-697: After awaiting SyncCubit.startSyncForDrive, re-read the
drive metadata (using the same method you use elsewhere to load drive info) and
check its lastBlockHeight; if lastBlockHeight > 0 treat sync as successful and
proceed to openFolder(rootFolderId), otherwise emit DriveDetailLoadUnsynced
instead of staying in DriveDetailLoadInProgress. In other words, inside the
block after the await (where you currently inspect syncState/currentState),
fetch the fresh drive record, use lastBlockHeight > 0 as the success criterion,
ensure _driveId still matches the requested driveId, and only call openFolder
when that check passes; otherwise emit DriveDetailLoadUnsynced and do not
navigate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91877032-24e5-4eb6-bed3-d9f5c21cab66
📒 Files selected for processing (2)
lib/blocs/drive_detail/drive_detail_cubit.dartlib/user/repositories/user_preferences_repository.dart
Replace 驱动器 with 网盘 in new localization strings to match existing project conventions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
After syncCurrentDrive completes, re-read the drive metadata and verify lastBlockHeight > 0 before calling openFolder. This guards against edge cases where sync reports success but content wasn't actually synced. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- profile_card: fallback to currentPreferences when stream hasn't emitted - progress_dialog: strengthen assertion for mutually exclusive title params - sync_repository: null safety for lastBlockHeight, clear ghost folder state on errors - user_preferences_repository: serialize auth state operations to prevent races - tests: add helper for default stubs, prevent load() race in setUp Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Display "Discovering your drives..." during the initial sync phase when updateUserDrives() is fetching drive metadata from the blockchain. This provides better feedback instead of showing 0% with no explanation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When sync is disabled and user navigates to a drive (especially via deep-link), the root folder may not exist yet. Previously this showed a "please wait" message. Now it correctly shows the sync options if the drive's lastBlockHeight indicates it hasn't been content-synced. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Emit SyncLoadingDrives immediately in createSyncStream() before any async work, so waitCurrentSync() doesn't return early thinking sync is idle when it's actually about to start - Handle FolderNotFoundInDriveException to show unsynced state when drive content hasn't been synced yet This fixes the issue where on login with "Sync All Drives" enabled, the drive view showed a syncing animation instead of content, even though clicking the drive in the sidebar worked correctly. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The previous fix emitted SyncLoadingDrives inside createSyncStream(), but since that's an async method, it doesn't execute until the next microtask - after the constructor returns. By then, DriveDetailCubit may have already called waitCurrentSync() and returned early. Changing the initial state from SyncIdle to SyncLoadingDrives ensures that from the moment SyncCubit exists, waitCurrentSync() will wait. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- syncMetadataOnly: emit SyncIdle even when profile not logged in - waitCurrentSync: also break on SyncCancelled and SyncCompleteWithErrors (these states don't automatically transition to SyncIdle) This prevents waitCurrentSync() from hanging indefinitely. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…or PE-9013 The BlocListener in drive_detail_page was calling changeDrive() when DrivesCubit emitted, even when DriveDetailCubit was already loading the same drive. This caused race conditions where changeDrive() would interrupt the constructor's initial load. - Add currentDriveId getter to DriveDetailCubit - Check if cubit is already handling the same driveId before calling changeDrive() Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ate PE-9013 When BlocListener calls changeDrive() during initial login, the drive's lastBlockHeight is still 0 because sync hasn't completed. Previously this immediately showed the "sync this drive" unsynced state. Now changeDrive() waits for sync to complete and re-queries the drive before deciding what to display. Also removes debug logging added during investigation and updates CLAUDE.md with minor documentation improvements. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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
This PR adds the ability to sync individual drives and introduces a toggle to control automatic sync behavior on login.
New Features
UI/UX Improvements
Bug Fixes
syncAllDrivesOnLoginpreference being reset on logout2x.png→ArDrive.png)clear()method for preferencesTechnical Changes
SyncLoadingDrivesstate for metadata-only sync UI feedbackDriveDetailLoadUnsyncedstate withshowDriveInfosupportSyncProgressnow tracksisSingleDriveSyncanddriveNamestartSyncForDrive()method for individual drive syncsyncMetadataOnly()method for lightweight drive list loadingTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Localization