From cb6d81771dbb8197322b2ef2568dc9b97fd1dd0c Mon Sep 17 00:00:00 2001 From: idubnori Date: Tue, 11 Nov 2025 04:25:43 +0900 Subject: [PATCH] fix(mobile): sync album and asset activity state when add/remove asset level activity (#23484) * fix; sync album-asset state when remove activity * make build * fix: support adding case * make build * Update mobile/lib/providers/activity.provider.dart Co-authored-by: shenlong <139912620+shenlong-tanwen@users.noreply.github.com> * fix: add missing import for collection package * make build --------- Co-authored-by: shenlong <139912620+shenlong-tanwen@users.noreply.github.com> --- mobile/lib/providers/activity.provider.dart | 53 ++++++++++-- mobile/lib/providers/activity.provider.g.dart | 2 +- .../activity/activity_provider_test.dart | 86 ++++++++++++++++--- 3 files changed, 118 insertions(+), 23 deletions(-) diff --git a/mobile/lib/providers/activity.provider.dart b/mobile/lib/providers/activity.provider.dart index a867a5a281..5e0e71d85d 100644 --- a/mobile/lib/providers/activity.provider.dart +++ b/mobile/lib/providers/activity.provider.dart @@ -1,3 +1,4 @@ +import 'package:collection/collection.dart'; import 'package:immich_mobile/models/activities/activity.model.dart'; import 'package:immich_mobile/providers/activity_service.provider.dart'; import 'package:immich_mobile/providers/activity_statistics.provider.dart'; @@ -16,13 +17,20 @@ class AlbumActivity extends _$AlbumActivity { Future removeActivity(String id) async { if (await ref.watch(activityServiceProvider).removeActivity(id)) { - final activities = state.valueOrNull ?? []; - final removedActivity = activities.firstWhere((a) => a.id == id); - activities.remove(removedActivity); - state = AsyncData(activities); - // Decrement activity count only for comments + final removedActivity = _removeFromState(id); + if (removedActivity == null) { + return; + } + + if (assetId != null) { + ref.read(albumActivityProvider(albumId).notifier)._removeFromState(id); + } + if (removedActivity.type == ActivityType.comment) { ref.watch(activityStatisticsProvider(albumId, assetId).notifier).removeActivity(); + if (assetId != null) { + ref.watch(activityStatisticsProvider(albumId).notifier).removeActivity(); + } } } } @@ -30,8 +38,10 @@ class AlbumActivity extends _$AlbumActivity { Future addLike() async { final activity = await ref.watch(activityServiceProvider).addActivity(albumId, ActivityType.like, assetId: assetId); if (activity.hasValue) { - final activities = state.asData?.value ?? []; - state = AsyncData([...activities, activity.requireValue]); + _addToState(activity.requireValue); + if (assetId != null) { + ref.read(albumActivityProvider(albumId).notifier)._addToState(activity.requireValue); + } } } @@ -41,8 +51,10 @@ class AlbumActivity extends _$AlbumActivity { .addActivity(albumId, ActivityType.comment, assetId: assetId, comment: comment); if (activity.hasValue) { - final activities = state.valueOrNull ?? []; - state = AsyncData([...activities, activity.requireValue]); + _addToState(activity.requireValue); + if (assetId != null) { + ref.read(albumActivityProvider(albumId).notifier)._addToState(activity.requireValue); + } ref.watch(activityStatisticsProvider(albumId, assetId).notifier).addActivity(); // The previous addActivity call would increase the count of an asset if assetId != null // To also increase the activity count of the album, calling it once again with assetId set to null @@ -51,6 +63,29 @@ class AlbumActivity extends _$AlbumActivity { } } } + + void _addToState(Activity activity) { + final activities = state.valueOrNull ?? []; + if (activities.any((a) => a.id == activity.id)) { + return; + } + state = AsyncData([...activities, activity]); + } + + Activity? _removeFromState(String id) { + final activities = state.valueOrNull; + if (activities == null) { + return null; + } + final activity = activities.firstWhereOrNull((a) => a.id == id); + if (activity == null) { + return null; + } + + final updated = [...activities]..remove(activity); + state = AsyncData(updated); + return activity; + } } /// Mock class for testing diff --git a/mobile/lib/providers/activity.provider.g.dart b/mobile/lib/providers/activity.provider.g.dart index dc927795f8..6ca99e4f72 100644 --- a/mobile/lib/providers/activity.provider.g.dart +++ b/mobile/lib/providers/activity.provider.g.dart @@ -6,7 +6,7 @@ part of 'activity.provider.dart'; // RiverpodGenerator // ************************************************************************** -String _$albumActivityHash() => r'3b0d7acee4d41c84b3f220784c3b904c83f836e6'; +String _$albumActivityHash() => r'154e8ae98da3efc142369eae46d4005468fd67da'; /// Copied from Dart SDK class _SystemHash { diff --git a/mobile/test/modules/activity/activity_provider_test.dart b/mobile/test/modules/activity/activity_provider_test.dart index 7964b43cad..84eba62b70 100644 --- a/mobile/test/modules/activity/activity_provider_test.dart +++ b/mobile/test/modules/activity/activity_provider_test.dart @@ -33,6 +33,7 @@ final _activities = [ void main() { late ActivityServiceMock activityMock; late ActivityStatisticsMock activityStatisticsMock; + late ActivityStatisticsMock albumActivityStatisticsMock; late ProviderContainer container; late AlbumActivityProvider provider; late ListenerMock>> listener; @@ -44,17 +45,23 @@ void main() { setUp(() async { activityMock = ActivityServiceMock(); activityStatisticsMock = ActivityStatisticsMock(); + albumActivityStatisticsMock = ActivityStatisticsMock(); + container = TestUtils.createContainer( overrides: [ activityServiceProvider.overrideWith((ref) => activityMock), activityStatisticsProvider('test-album', 'test-asset').overrideWith(() => activityStatisticsMock), + activityStatisticsProvider('test-album').overrideWith(() => albumActivityStatisticsMock), ], ); // Mock values + when(() => activityStatisticsMock.build(any(), any())).thenReturn(0); + when(() => albumActivityStatisticsMock.build(any())).thenReturn(0); when( () => activityMock.getAllActivities('test-album', assetId: 'test-asset'), ).thenAnswer((_) async => [..._activities]); + when(() => activityMock.getAllActivities('test-album')).thenAnswer((_) async => [..._activities]); // Init and wait for providers future to complete provider = albumActivityProvider('test-album', 'test-asset'); @@ -89,6 +96,10 @@ void main() { () => activityMock.addActivity('test-album', ActivityType.like, assetId: 'test-asset'), ).thenAnswer((_) async => AsyncData(like)); + final albumProvider = albumActivityProvider('test-album'); + container.read(albumProvider.notifier); + await container.read(albumProvider.future); + await container.read(provider.notifier).addLike(); verify(() => activityMock.addActivity('test-album', ActivityType.like, assetId: 'test-asset')); @@ -99,6 +110,11 @@ void main() { // Never bump activity count for new likes verifyNever(() => activityStatisticsMock.addActivity()); + verifyNever(() => albumActivityStatisticsMock.addActivity()); + + final albumActivities = container.read(albumProvider).requireValue; + expect(albumActivities, hasLength(5)); + expect(albumActivities, contains(like)); }); test('Like failed', () async { @@ -107,6 +123,10 @@ void main() { () => activityMock.addActivity('test-album', ActivityType.like, assetId: 'test-asset'), ).thenAnswer((_) async => AsyncError(Exception('Mock'), StackTrace.current)); + final albumProvider = albumActivityProvider('test-album'); + container.read(albumProvider.notifier); + await container.read(albumProvider.future); + await container.read(provider.notifier).addLike(); verify(() => activityMock.addActivity('test-album', ActivityType.like, assetId: 'test-asset')); @@ -114,6 +134,12 @@ void main() { final activities = await container.read(provider.future); expect(activities, hasLength(4)); expect(activities, isNot(contains(like))); + + verifyNever(() => albumActivityStatisticsMock.addActivity()); + + final albumActivities = container.read(albumProvider).requireValue; + expect(albumActivities, hasLength(4)); + expect(albumActivities, isNot(contains(like))); }); }); @@ -130,6 +156,7 @@ void main() { expect(activities, isNot(anyElement(predicate((Activity a) => a.id == '3')))); verifyNever(() => activityStatisticsMock.removeActivity()); + verifyNever(() => albumActivityStatisticsMock.removeActivity()); }); test('Remove Like failed', () async { @@ -140,6 +167,9 @@ void main() { final activities = await container.read(provider.future); expect(activities, hasLength(4)); expect(activities, anyElement(predicate((Activity a) => a.id == '3'))); + + verifyNever(() => activityStatisticsMock.removeActivity()); + verifyNever(() => albumActivityStatisticsMock.removeActivity()); }); test('Comment successfully removed', () async { @@ -151,23 +181,35 @@ void main() { expect(activities, isNot(anyElement(predicate((Activity a) => a.id == '1')))); verify(() => activityStatisticsMock.removeActivity()); + verify(() => albumActivityStatisticsMock.removeActivity()); + }); + + test('Removes activity from album state when asset scoped', () async { + when(() => activityMock.removeActivity('3')).thenAnswer((_) async => true); + when(() => activityMock.getAllActivities('test-album')).thenAnswer((_) async => [..._activities]); + + final albumProvider = albumActivityProvider('test-album'); + container.read(albumProvider.notifier); + await container.read(albumProvider.future); + + await container.read(provider.notifier).removeActivity('3'); + + final assetActivities = container.read(provider).requireValue; + final albumActivities = container.read(albumProvider).requireValue; + + expect(assetActivities, hasLength(3)); + expect(assetActivities, isNot(anyElement(predicate((Activity a) => a.id == '3')))); + + expect(albumActivities, hasLength(3)); + expect(albumActivities, isNot(anyElement(predicate((Activity a) => a.id == '3')))); + + verify(() => activityMock.removeActivity('3')); + verifyNever(() => activityStatisticsMock.removeActivity()); + verifyNever(() => albumActivityStatisticsMock.removeActivity()); }); }); group('addComment()', () { - late ActivityStatisticsMock albumActivityStatisticsMock; - - setUp(() { - albumActivityStatisticsMock = ActivityStatisticsMock(); - container = TestUtils.createContainer( - overrides: [ - activityServiceProvider.overrideWith((ref) => activityMock), - activityStatisticsProvider('test-album', 'test-asset').overrideWith(() => activityStatisticsMock), - activityStatisticsProvider('test-album').overrideWith(() => albumActivityStatisticsMock), - ], - ); - }); - test('Comment successfully added', () async { final comment = Activity( id: '5', @@ -178,6 +220,10 @@ void main() { assetId: 'test-asset', ); + final albumProvider = albumActivityProvider('test-album'); + container.read(albumProvider.notifier); + await container.read(albumProvider.future); + when( () => activityMock.addActivity( 'test-album', @@ -206,6 +252,10 @@ void main() { verify(() => activityStatisticsMock.addActivity()); verify(() => albumActivityStatisticsMock.addActivity()); + + final albumActivities = container.read(albumProvider).requireValue; + expect(albumActivities, hasLength(5)); + expect(albumActivities, contains(comment)); }); test('Comment successfully added without assetId', () async { @@ -225,6 +275,8 @@ void main() { when(() => activityMock.getAllActivities('test-album')).thenAnswer((_) async => [..._activities]); final albumProvider = albumActivityProvider('test-album'); + container.read(albumProvider.notifier); + await container.read(albumProvider.future); await container.read(albumProvider.notifier).addComment('Test-Comment'); verify( @@ -258,6 +310,10 @@ void main() { ), ).thenAnswer((_) async => AsyncError(Exception('Error'), StackTrace.current)); + final albumProvider = albumActivityProvider('test-album'); + container.read(albumProvider.notifier); + await container.read(albumProvider.future); + await container.read(provider.notifier).addComment('Test-Comment'); final activities = await container.read(provider.future); @@ -266,6 +322,10 @@ void main() { verifyNever(() => activityStatisticsMock.addActivity()); verifyNever(() => albumActivityStatisticsMock.addActivity()); + + final albumActivities = container.read(albumProvider).requireValue; + expect(albumActivities, hasLength(4)); + expect(albumActivities, isNot(contains(comment))); }); }); }