chore: Simplification

This commit is contained in:
Marvin M 2025-10-11 12:29:29 +02:00
parent db727eb2e8
commit 0300f23834
4 changed files with 6 additions and 69 deletions

View file

@ -12,10 +12,6 @@ class PersonMergeTrackerService {
/// Map of merged person ID -> target person ID /// Map of merged person ID -> target person ID
final Map<String, String> _mergeForwardingMap = {}; final Map<String, String> _mergeForwardingMap = {};
/// Set of person IDs for which the merge record has been handled (redirected)
/// To prevent multiple redirects for the same merge
final Set<String> _handledMergeRecords = {};
/// Record a person merge operation /// Record a person merge operation
void recordMerge({required String mergedPersonId, required String targetPersonId}) { void recordMerge({required String mergedPersonId, required String targetPersonId}) {
_mergeForwardingMap[mergedPersonId] = targetPersonId; _mergeForwardingMap[mergedPersonId] = targetPersonId;
@ -25,24 +21,4 @@ class PersonMergeTrackerService {
String? getTargetPersonId(String personId) { String? getTargetPersonId(String personId) {
return _mergeForwardingMap[personId]; return _mergeForwardingMap[personId];
} }
/// Check if a person ID has been merged
bool isPersonMerged(String personId) {
return _mergeForwardingMap.containsKey(personId);
}
/// Check if a merge record has been handled (redirected)
bool isMergeRecordHandled(String personId) {
return _handledMergeRecords.contains(personId);
}
/// Check if we should redirect for this person (merged but not yet handled)
bool shouldRedirectForPerson(String personId) {
return isPersonMerged(personId) && !isMergeRecordHandled(personId);
}
/// Mark a merge record as handled (for tracking purposes)
void markMergeRecordHandled(String personId) {
_handledMergeRecords.add(personId);
}
} }

View file

@ -74,10 +74,8 @@ class _DriftPersonPageState extends ConsumerState<DriftPersonPage> {
data: (personByIdProvider) { data: (personByIdProvider) {
if (personByIdProvider == null) { if (personByIdProvider == null) {
// Check if the person was merged and redirect if necessary // Check if the person was merged and redirect if necessary
final shouldRedirect = mergeTracker.shouldRedirectForPerson(_person.id);
final targetPersonId = mergeTracker.getTargetPersonId(_person.id); final targetPersonId = mergeTracker.getTargetPersonId(_person.id);
if (targetPersonId != null) {
if (shouldRedirect && targetPersonId != null) {
bool isOnPersonDetailPage = ModalRoute.of(context)?.isCurrent ?? false; 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 // Only redirect if we're currently on the person detail page, not in a nested view, e.g. image viewer
@ -93,13 +91,10 @@ class _DriftPersonPageState extends ConsumerState<DriftPersonPage> {
.first .first
.then((targetPerson) { .then((targetPerson) {
if (targetPerson != null && mounted) { if (targetPerson != null && mounted) {
mergeTracker.markMergeRecordHandled(_person.id);
// Open the target person's page // Open the target person's page
if (mounted) { if (mounted) {
context.replaceRoute( context.pop();
DriftPersonRoute(key: ValueKey(targetPerson.id), initialPerson: targetPerson), context.pushRoute(DriftPersonRoute(initialPerson: targetPerson));
);
} }
} else { } else {
// Target person not found, go back // Target person not found, go back
@ -116,12 +111,11 @@ class _DriftPersonPageState extends ConsumerState<DriftPersonPage> {
} }
}); });
return const Center(child: CircularProgressIndicator()); return const Center(child: CircularProgressIndicator());
} else if (shouldRedirect && targetPersonId == null) { } else {
// This should never happen, but just in case
// Person not found and no merge record
mergeLogger.info( mergeLogger.info(
'Person ${_person.name} (${_person.id}) not found and no merge records exist, it was probably deleted', 'Person ${_person.name} (${_person.id}) not found and no merge records exist, it was probably deleted',
); );
WidgetsBinding.instance.addPostFrameCallback((_) { WidgetsBinding.instance.addPostFrameCallback((_) {
if (mounted) { if (mounted) {
context.maybePop(); context.maybePop();
@ -129,10 +123,7 @@ class _DriftPersonPageState extends ConsumerState<DriftPersonPage> {
}); });
return const Center(child: CircularProgressIndicator()); return const Center(child: CircularProgressIndicator());
} }
// Waiting for the personByProvider to load
return const Center(child: CircularProgressIndicator());
} }
_person = personByIdProvider; _person = personByIdProvider;
return ProviderScope( return ProviderScope(
overrides: [ overrides: [

View file

@ -86,7 +86,7 @@ class _SheetPeopleDetailsState extends ConsumerState<SheetPeopleDetails> {
// and the asset viewer goes black // 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) // 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) { if (newPerson != null && newPerson.id != person.id && previousPersonId == person.id) {
context.pop(); await context.maybePop();
} }
ref.invalidate(driftPeopleAssetProvider(asset.id)); ref.invalidate(driftPeopleAssetProvider(asset.id));

View file

@ -1,30 +0,0 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:immich_mobile/domain/services/person_merge_tracker.service.dart';
void main() {
group('PersonMergeTrackerService', () {
late PersonMergeTrackerService mergeTracker;
setUp(() {
mergeTracker = PersonMergeTrackerService();
});
test('records and retrieves merge correctly', () {
mergeTracker.recordMerge(mergedPersonId: 'A', targetPersonId: 'B');
expect(mergeTracker.isPersonMerged('A'), isTrue);
expect(mergeTracker.getTargetPersonId('A'), equals('B'));
expect(mergeTracker.shouldRedirectForPerson('A'), isTrue);
mergeTracker.markMergeRecordHandled('A');
expect(mergeTracker.isMergeRecordHandled('A'), isTrue);
expect(mergeTracker.shouldRedirectForPerson('A'), isFalse);
});
test('returns false for non-merged person', () {
expect(mergeTracker.isPersonMerged('X'), isFalse);
expect(mergeTracker.getTargetPersonId('X'), isNull);
expect(mergeTracker.shouldRedirectForPerson('X'), isFalse);
});
});
}