feat: album info sync #21103
feat: album info sync #21103
Conversation
alextran1502
force-pushed
the
album-info-sync
branch
from
4bda30f to
02c2e76
Compare
alextran1502
force-pushed
the
album-info-sync
branch
3 times, most recently
from
8611858 to
69c24fe
Compare
alextran1502
force-pushed
the
album-info-sync
branch
from
69c24fe to
68354ef
Compare
alextran1502
force-pushed
the
album-info-sync
branch
from
38e5b14 to
f082fc8
Compare
alextran1502
force-pushed
the
album-info-sync
branch
from
f4addbb to
18059ac
Compare
alextran1502
force-pushed
the
album-info-sync
branch
from
c2e0bac to
56f9b2c
Compare
|
|
||
| final syncLinkedAlbumProvider = NotifierProvider<SyncLinkedAlbumNotifier, bool>(SyncLinkedAlbumNotifier.new); | ||
|
|
||
| class SyncLinkedAlbumNotifier extends Notifier<bool> { |
There was a problem hiding this comment.
This should just be a service and a simple provider can be used to locate the service
| return _db.managers.localAssetEntity.filter((e) => e.checksum.isNull().not()).count(); | ||
| } | ||
|
|
||
| Future<List<LocalAlbum>> getAlbumsForAsset(String assetId) async { |
| syncAlbums() async { | ||
| setState(() { | ||
| isAlbumSyncInProgress = true; | ||
| }); | ||
| try { | ||
| ref.read(syncLinkedAlbumProvider.notifier).syncLinkedAlbums(); | ||
| } catch (_) { | ||
| } finally { | ||
| Future.delayed(const Duration(seconds: 1), () { | ||
| setState(() { | ||
| isAlbumSyncInProgress = false; | ||
| }); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Why not throw the loading state inside a provider and handle it properly than just showing the indicator for 1 second always? Also, it feels weird that we have to enable the "Sync Albums" option inside the album selection page to have the sync albums show up in the settings page.
There was a problem hiding this comment.
This is for manually running the sync process, especially for cases where you later decide to use the sync feature. In the album selection, the linked album aren't created until you are navigate away from the page, meaning the selected albums are finalized
There was a problem hiding this comment.
I think we could instead do the following:
- Either have a section in the backup settings for sync albums and allow the user to enable or disable it there, along with a button to manually run it on demand
- Always show the manual sync button but keep it disabled until the sync albums setting is enabled (This can be coupled with the above suggestion)
In both cases, wouldn't it also be better if we showed the actual progress and prevent the user from tapping the button multiple times instead of a 1s indicator
| return _db.managers.remoteAlbumEntity.count(); | ||
| } | ||
|
|
||
| Future<List<String>> getLinkedAssetIds(String userId, String localAlbumId, String remoteAlbumId) async { |
There was a problem hiding this comment.
Should this be called the other way? i.e, getUnLinkedIds as we are returning the assets to be linked and not those that are already linked?
There was a problem hiding this comment.
technically, it is for "getting linked album assets that has not been added" lol. Told you the hardest part of the PR is naming :P
mobile/lib/infrastructure/repositories/remote_album.repository.dart
Outdated
Show resolved
Hide resolved
| Future<void> syncLinkedAlbums(String userId, List<LocalAlbum> selectedAlbums) async { | ||
| await Future.wait( | ||
| selectedAlbums.map((localAlbum) async { | ||
| final linkedRemoteAlbumId = localAlbum.linkedRemoteAlbumId; | ||
| if (linkedRemoteAlbumId == null) { | ||
| return; | ||
| } | ||
|
|
||
| final remoteAlbum = await get(linkedRemoteAlbumId); | ||
| if (remoteAlbum == null) { | ||
| return; | ||
| } | ||
|
|
||
| // get assets that are uploaded but not in the remote album | ||
| final assetIds = await _repository.getLinkedAssetIds(userId, localAlbum.id, linkedRemoteAlbumId); | ||
|
|
||
| if (assetIds.isNotEmpty) { | ||
| await addAssets(albumId: linkedRemoteAlbumId, assetIds: assetIds); | ||
| } | ||
| }), | ||
| ); | ||
| } |
There was a problem hiding this comment.
This is almost similar to the syncLinkedAlbums method on SyncLinkedAlbumService
There was a problem hiding this comment.
Good catch, this is an unsued method
Close #20341