diff --git a/mobile/lib/domain/services/person_merge_tracker.service.dart b/mobile/lib/domain/services/person_merge_tracker.service.dart index 04d90aedf6..a53d39cbe9 100644 --- a/mobile/lib/domain/services/person_merge_tracker.service.dart +++ b/mobile/lib/domain/services/person_merge_tracker.service.dart @@ -9,11 +9,11 @@ /// So when popping back to the profile page (and the user is missing) we check /// which other person B we have to display instead. class PersonMergeTrackerService { - // Map of merged person ID -> target person ID + /// Map of merged person ID -> target person ID final Map _mergeForwardingMap = {}; - // Set of person IDs for which the merge record has been handled (redirected) - // To prevent multiple redirects for the same merge + /// Set of person IDs for which the merge record has been handled (redirected) + /// To prevent multiple redirects for the same merge final Set _handledMergeRecords = {}; /// Record a person merge operation diff --git a/mobile/lib/presentation/pages/drift_person.page.dart b/mobile/lib/presentation/pages/drift_person.page.dart index bbce31d8ba..ea8d12d3b3 100644 --- a/mobile/lib/presentation/pages/drift_person.page.dart +++ b/mobile/lib/presentation/pages/drift_person.page.dart @@ -71,8 +71,8 @@ class _DriftPersonPageState extends ConsumerState { ref.watch(currentRouteNameProvider.select((name) => name ?? DriftPersonRoute.name)); return personAsync.when( - data: (personByProvider) { - if (personByProvider == null) { + data: (personByIdProvider) { + if (personByIdProvider == null) { // Check if the person was merged and redirect if necessary final shouldRedirect = mergeTracker.shouldRedirectForPerson(_person.id); final targetPersonId = mergeTracker.getTargetPersonId(_person.id); @@ -81,56 +81,60 @@ class _DriftPersonPageState extends ConsumerState { bool isOnPersonDetailPage = ModalRoute.of(context)?.isCurrent ?? false; // Only redirect if we're currently on the person detail page, not in a nested view, e.g. image viewer - if (isOnPersonDetailPage) { - // Person was merged, redirect to the target person - WidgetsBinding.instance.addPostFrameCallback((_) { - if (mounted) { - // Use the service directly to get the target person - ref - .read(driftPeopleServiceProvider) - .watchPersonById(targetPersonId) - .first - .then((targetPerson) { - if (targetPerson != null && mounted) { - // Mark the merge record as handled - mergeTracker.markMergeRecordHandled(_person.id); - _person = targetPerson; - setState(() {}); - } else { - // Target person not found, go back - context.maybePop(); - } - }) - .catchError((error) { - // If we can't load the target person, go back - mergeLogger.severe("Error during read of targetPerson", error); - if (mounted) { - context.maybePop(); - } - }); - } - }); - return const Center(child: CircularProgressIndicator()); - } else { - // We're in an image viewer or other nested view, don't redirect yet - // Just show loading spinner to indicate something is happening + if (!isOnPersonDetailPage) { return const Center(child: CircularProgressIndicator()); } - } + // Person was merged, redirect to the target person + WidgetsBinding.instance.addPostFrameCallback((_) { + if (mounted) { + ref + .read(driftPeopleServiceProvider) + .watchPersonById(targetPersonId) + .first + .then((targetPerson) { + if (targetPerson != null && mounted) { + // Mark the merge record as handled + mergeTracker.markMergeRecordHandled(_person.id); - // Person not found and no merge record - mergeLogger.info( - 'Person ${_person.name} (${_person.id}) not found and no merge records exist, it was probably deleted', - ); - WidgetsBinding.instance.addPostFrameCallback((_) { - if (mounted) { - context.maybePop(); - } - }); + // Open the target person's page + if (mounted) { + context.replaceRoute( + DriftPersonRoute(key: ValueKey(targetPerson.id), initialPerson: targetPerson), + ); + } + } else { + // Target person not found, go back + context.maybePop(); + } + }) + .catchError((error) { + // If we can't load the target person, go back + mergeLogger.severe("Error during read of targetPerson", error); + if (mounted) { + context.maybePop(); + } + }); + } + }); + return const Center(child: CircularProgressIndicator()); + } else if (shouldRedirect && targetPersonId == null) { + // This should never happen, but just in case + // Person not found and no merge record + mergeLogger.info( + 'Person ${_person.name} (${_person.id}) not found and no merge records exist, it was probably deleted', + ); + WidgetsBinding.instance.addPostFrameCallback((_) { + if (mounted) { + context.maybePop(); + } + }); + return const Center(child: CircularProgressIndicator()); + } + // Waiting for the personByProvider to load return const Center(child: CircularProgressIndicator()); } - _person = personByProvider; + _person = personByIdProvider; return ProviderScope( overrides: [ timelineServiceProvider.overrideWith((ref) { @@ -154,7 +158,6 @@ class _DriftPersonPageState extends ConsumerState { ), ); }, - // TODO(m123): Show initialPerson data while loading new data (optimistic ui update, but we need to handle scroll state etc) loading: () => const Center(child: CircularProgressIndicator()), error: (e, s) => Text('Error: $e'), ); diff --git a/mobile/lib/presentation/widgets/asset_viewer/bottom_sheet/sheet_people_details.widget.dart b/mobile/lib/presentation/widgets/asset_viewer/bottom_sheet/sheet_people_details.widget.dart index 0871db3510..50443848a0 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/bottom_sheet/sheet_people_details.widget.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/bottom_sheet/sheet_people_details.widget.dart @@ -61,7 +61,6 @@ class _SheetPeopleDetailsState extends ConsumerState { final previousRouteData = ref.read(previousRouteDataProvider); final previousRouteArgs = previousRouteData?.arguments; - // TODO: Check what happens if the person id from the previous route is not the correct one anymore e.g. after a merge // Prevent circular navigation if (previousRouteArgs is DriftPersonRouteArgs && previousRouteArgs.initialPerson.id == person.id) { @@ -72,24 +71,25 @@ class _SheetPeopleDetailsState extends ConsumerState { context.pushRoute(DriftPersonRoute(initialPerson: person)); }, onNameTap: () async { - DriftPerson? newPerson = await showNameEditModal(context, person); - - ref.invalidate(driftPeopleAssetProvider(asset.id)); - - // If the name edit resulted in a new person (e.g. from merging) - // And if we are currently nested below the drift person page if said - // old person id, we need to pop, otherwise the timeline provider - // complains because the indexes are off - // TODO: Preferably we would replace the timeline provider, and let it listen to the new person id (Relevant function is the ```TimelineService person(String userId, String personId)``` in timeline.service.dart) + // Needs to be before the modal, as this overwrites the previousRouteDataProvider final previousRouteData = ref.read(previousRouteDataProvider); final previousRouteArgs = previousRouteData?.arguments; final previousPersonId = previousRouteArgs is DriftPersonRouteArgs ? previousRouteArgs.initialPerson.id : null; + DriftPerson? newPerson = await showNameEditModal(context, person); + + // If the name edit resulted in a new person (e.g. from merging) + // And if we are currently nested below the drift person page if said + // old person id, we need to pop, otherwise the timeline provider complains + // and the asset viewer goes black + // TODO: Preferably we would replace the timeline provider, and let it listen to the new person id (Relevant function is the ```TimelineService person(String userId, String personId)``` in timeline.service.dart) if (newPerson != null && newPerson.id != person.id && previousPersonId == person.id) { context.pop(); } + + ref.invalidate(driftPeopleAssetProvider(asset.id)); }, ), ], diff --git a/mobile/lib/presentation/widgets/people/person_edit_name_modal.widget.dart b/mobile/lib/presentation/widgets/people/person_edit_name_modal.widget.dart index 28a0f055d7..9d3b9f736c 100644 --- a/mobile/lib/presentation/widgets/people/person_edit_name_modal.widget.dart +++ b/mobile/lib/presentation/widgets/people/person_edit_name_modal.widget.dart @@ -11,6 +11,7 @@ import 'package:immich_mobile/providers/infrastructure/people.provider.dart'; import 'package:immich_mobile/utils/debug_print.dart'; import 'package:immich_mobile/utils/people.utils.dart'; import 'package:immich_mobile/widgets/common/immich_toast.dart'; +import 'package:logging/logging.dart'; class DriftPersonNameEditForm extends ConsumerStatefulWidget { final DriftPerson person; @@ -72,7 +73,7 @@ class _DriftPersonNameEditFormState extends ConsumerState people, String query) { final queryParts = query.toLowerCase().split(' ').where((e) => e.isNotEmpty).toList(); @@ -80,7 +81,10 @@ class _DriftPersonNameEditFormState extends ConsumerState containsMatches = []; for (final p in people) { + if (p.id == widget.person.id) continue; + final nameParts = p.name.toLowerCase().split(' ').where((e) => e.isNotEmpty).toList(); + final allStart = queryParts.every((q) => nameParts.any((n) => n.startsWith(q))); final allContain = queryParts.every((q) => nameParts.any((n) => n.contains(q))); @@ -94,7 +98,7 @@ class _DriftPersonNameEditFormState extends ConsumerState people = []; return AlertDialog( title: const Text("edit_name", style: TextStyle(fontWeight: FontWeight.bold)).tr(), - content: curatedPeople.when( - data: (people) { - return SingleChildScrollView( - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - TextFormField( - autofocus: true, - controller: _formController, - decoration: InputDecoration( - hintText: 'add_a_name'.tr(), - border: const OutlineInputBorder(borderRadius: BorderRadius.all(Radius.circular(8))), - ), - onChanged: (value) => _filterPeople(people, value), - onTapOutside: (event) => FocusScope.of(context).unfocus(), - ), - AnimatedSize( + content: SingleChildScrollView( + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + TextFormField( + autofocus: true, + controller: _formController, + decoration: InputDecoration( + hintText: 'add_a_name'.tr(), + border: const OutlineInputBorder(borderRadius: BorderRadius.all(Radius.circular(8))), + ), + onChanged: (value) => _filterPeople(people, value), + onTapOutside: (event) => FocusScope.of(context).unfocus(), + ), + curatedPeople.when( + data: (p) { + people = p; + return AnimatedSize( duration: const Duration(milliseconds: 200), child: SizedBox( width: double.infinity, @@ -158,13 +164,16 @@ class _DriftPersonNameEditFormState extends ConsumerState const Center(child: CircularProgressIndicator()), + error: (err, stack) { + Logger('PersonEditNameModal').warning('Error loading people for name edit modal', err, stack); + return Center(child: Text('Error loading people for name edit modal: $err')); + }, ), - ); - }, - loading: () => const Center(child: CircularProgressIndicator()), - error: (err, stack) => Text('Error: $err'), + ], + ), ), actions: [ TextButton(