TF-3298 Fix blink when refresh email list (#3299)

lib/features/thread/data/extensions/list_email_extension.dart

@@ -17,23 +17,24 @@ extension ListEmailExtension on List<Email> {
     };
   }
 
-  List<Email> sortingByOrderOfIdList(List<Id> ids) {
+  List<Email> sortEmailsById(List<Id> referenceIds) {
-    if (ids.length != length) {
+    final indexMap = {
-      return this;
+      for (var i = 0; i < referenceIds.length; i++)
-    }
+        referenceIds[i]: i
+    };
 
     sort((email1, email2) {
-      final id1 = email1.id?.id;
+      final emailId1 = email1.id?.id;
-      final id2 = email2.id?.id;
+      final emailId2 = email2.id?.id;
 
-      if (id1 == null || id2 == null) {
+      if (emailId1 == null || emailId2 == null) {
-        return 0;
+        return emailId1 == null ? 1 : -1;
       }
 
-      final index1 = ids.indexWhere((id) => id == id1);
+      final indexEmail1 = indexMap[emailId1] ?? double.maxFinite;
-      final index2 = ids.indexWhere((id) => id == id2);
+      final indexEmail2 = indexMap[emailId2] ?? double.maxFinite;
 
-      return index1.compareTo(index2);
+      return indexEmail1.compareTo(indexEmail2);
     });
 
     return this;

lib/features/thread/data/network/thread_api.dart

@@ -20,7 +20,6 @@ import 'package:jmap_dart_client/jmap/mail/email/get/get_email_method.dart';
 import 'package:jmap_dart_client/jmap/mail/email/get/get_email_response.dart';
 import 'package:jmap_dart_client/jmap/mail/email/query/query_email_method.dart';
 import 'package:jmap_dart_client/jmap/mail/email/query/query_email_response.dart';
-import 'package:model/extensions/list_email_extension.dart';
 import 'package:tmail_ui_user/features/thread/data/extensions/list_email_extension.dart';
 import 'package:jmap_dart_client/jmap/mail/email/search_snippet/search_snippet.dart';
 import 'package:jmap_dart_client/jmap/mail/email/search_snippet/search_snippet_get_method.dart';
@@ -91,24 +90,33 @@ class ThreadAPI {
       QueryEmailResponse.deserialize,
     );
 
-    List<Email>? emailList;
+    final emailList = sortEmails(
+      getEmailResponse: responseOfGetEmailMethod,
+      queryEmailResponse: responseOfQueryEmailMethod,
+    );
 
-    if (responseOfGetEmailMethod?.list.isNotEmpty == true &&
-        responseOfQueryEmailMethod?.ids.isNotEmpty == true) {
-      log('ThreadAPI::getAllEmail: QUERY_EMAIL_IDS = ${responseOfQueryEmailMethod?.ids}');
-      final listSortedEmail = responseOfGetEmailMethod!.list
-        .sortingByOrderOfIdList(responseOfQueryEmailMethod!.ids.toList());
-      emailList = listSortedEmail;
-    } else {
-      emailList = responseOfGetEmailMethod?.list;
-    }
-    log('ThreadAPI::getAllEmail: EMAIL_DISPLAYED_IDS = ${emailList?.listEmailIds}');
     return EmailsResponse(
       emailList: emailList,
       state: responseOfGetEmailMethod?.state,
     );
   }
 
+  List<Email>? sortEmails({
+    GetEmailResponse? getEmailResponse,
+    QueryEmailResponse? queryEmailResponse,
+  }) {
+    final listEmails = getEmailResponse?.list;
+    final listIds = queryEmailResponse?.ids.toList();
+
+    if (listEmails?.isNotEmpty != true || listIds?.isNotEmpty != true) {
+      return listEmails;
+    }
+
+    final listSortedEmails = listEmails!.sortEmailsById(listIds!);
+
+    return listSortedEmails;
+  }
+
   Future<SearchEmailsResponse> searchEmails(
     Session session,
     AccountId accountId,
@@ -166,21 +174,16 @@ class ThreadAPI {
       .build()
       .execute();
 
-    final emailResultList = result.parse<GetEmailResponse>(
+    final responseOfGetEmailMethod = result.parse<GetEmailResponse>(
         getEmailInvocation.methodCallId, GetEmailResponse.deserialize);
     final responseOfQueryEmailMethod = result.parse<QueryEmailResponse>(
       queryEmailInvocation.methodCallId,
       QueryEmailResponse.deserialize);
 
-    List<Email>? sortedEmailList;
+    final sortedEmailList = sortEmails(
-
+      getEmailResponse: responseOfGetEmailMethod,
-    if (emailResultList?.list.isNotEmpty == true &&
+      queryEmailResponse: responseOfQueryEmailMethod,
-        responseOfQueryEmailMethod?.ids.isNotEmpty == true) {
+    );
-      sortedEmailList = emailResultList!.list
-        .sortingByOrderOfIdList(responseOfQueryEmailMethod!.ids.toList());
-    } else {
-      sortedEmailList = emailResultList?.list;
-    }
 
     final searchSnippets = _getSearchSnippetsFromResponse(
       result,
@@ -188,7 +191,7 @@ class ThreadAPI {
     );
     return SearchEmailsResponse(
         emailList: sortedEmailList,
-        state: emailResultList?.state,
+        state: responseOfGetEmailMethod?.state,
         searchSnippets: searchSnippets);
   }
 

test/features/email/sorting_list_email_by_order_id_list_test.dart

@@ -1,54 +0,0 @@
-import 'package:flutter_test/flutter_test.dart';
-import 'package:jmap_dart_client/jmap/core/id.dart';
-import 'package:jmap_dart_client/jmap/mail/email/email.dart';
-import 'package:tmail_ui_user/features/thread/data/extensions/list_email_extension.dart';
-
-void main() {
-  group('sorting_list_email_by_order_id_list test', () {
-    test('sortingByOrderOfIdList method should return an ordered list of ids when of the same length', () {
-      List<Id> ids = [
-        Id('a'),
-        Id('b'),
-        Id('c'),
-        Id('d'),
-        Id('e')
-      ];
-      List<Email> emails = [
-        Email(id: EmailId(Id('a'))),
-        Email(id: EmailId(Id('c'))),
-        Email(id: EmailId(Id('e'))),
-        Email(id: EmailId(Id('d'))),
-        Email(id: EmailId(Id('b')))
-      ];
-
-      List<Email> sortedEmails = emails.sortingByOrderOfIdList(ids);
-
-      expect(
-          sortedEmails.map((e) => e.id?.id.value),
-          equals(['a', 'b', 'c', 'd', 'e'])
-      );
-    });
-
-    test('sortingByOrderOfIdList method should return the original list when the length of the two lists is different', () {
-      List<Id> ids = [
-        Id('a'),
-        Id('b'),
-        Id('c'),
-      ];
-      List<Email> emails = [
-        Email(id: EmailId(Id('a'))),
-        Email(id: EmailId(Id('c'))),
-        Email(id: EmailId(Id('e'))),
-        Email(id: EmailId(Id('d'))),
-        Email(id: EmailId(Id('b')))
-      ];
-
-      List<Email> sortedEmails = emails.sortingByOrderOfIdList(ids);
-
-      expect(
-          sortedEmails.map((e) => e.id?.id.value),
-          equals(['a', 'c', 'e', 'd', 'b'])
-      );
-    });
-  });
-}

test/features/thread/data/extensions/list_email_extension_test.dart

@@ -0,0 +1,166 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:jmap_dart_client/jmap/core/id.dart';
+import 'package:jmap_dart_client/jmap/mail/email/email.dart';
+import 'package:model/extensions/email_id_extensions.dart';
+import 'package:tmail_ui_user/features/thread/data/extensions/list_email_extension.dart';
+
+void main() {
+  group('ListEmailExtension::sortEmailsById::test', () {
+    test('Sort the full list', () {
+      final referenceIds = <Id>[
+        Id('id1'),
+        Id('id2'),
+        Id('id3'),
+        Id('id4'),
+      ];
+
+      final emails = <Email>[
+        Email(id: EmailId(Id('id3'))),
+        Email(id: EmailId(Id('id1'))),
+        Email(id: EmailId(Id('id4'))),
+        Email(id: EmailId(Id('id2'))),
+      ];
+
+      final result = emails.sortEmailsById(referenceIds);
+
+      expect(
+        result.map((e) => e.id!.asString).toList(),
+        ['id1', 'id2', 'id3', 'id4'],
+      );
+    });
+
+    test('Emails list has more elements than referenceIds', () {
+      final referenceIds = <Id>[
+        Id('id1'),
+        Id('id2'),
+        Id('id3'),
+      ];
+
+      final emails = <Email>[
+        Email(id: EmailId(Id('id3'))),
+        Email(id: EmailId(Id('id4'))),
+        Email(id: EmailId(Id('id1'))),
+        Email(id: EmailId(Id('id5'))),
+        Email(id: EmailId(Id('id2'))),
+      ];
+
+      final result = emails.sortEmailsById(referenceIds);
+
+      expect(
+        result.map((e) => e.id!.asString).toList(),
+        ['id1', 'id2', 'id3', 'id4', 'id5'],
+      );
+    });
+
+    test('Emails list has fewer elements than referenceIds', () {
+      final referenceIds = <Id>[
+        Id('id1'),
+        Id('id2'),
+        Id('id3'),
+        Id('id4'),
+      ];
+
+      final emails = <Email>[
+        Email(id: EmailId(Id('id3'))),
+        Email(id: EmailId(Id('id1'))),
+      ];
+
+      final result = emails.sortEmailsById(referenceIds);
+
+      expect(
+        result.map((e) => e.id!.asString).toList(),
+        ['id1', 'id3'],
+      );
+    });
+
+    test('Emails list is empty', () {
+      final referenceIds = <Id>[
+        Id('id1'),
+        Id('id2'),
+        Id('id3'),
+      ];
+
+      final emails = <Email>[];
+
+      final result = emails.sortEmailsById(referenceIds);
+
+      expect(
+        result.map((e) => e.id!.asString).toList(),
+        [],
+      );
+    });
+
+    test('ReferenceIds list is empty', () {
+      final referenceIds = <Id>[];
+
+      final emails = <Email>[
+        Email(id: EmailId(Id('id3'))),
+        Email(id: EmailId(Id('id4'))),
+        Email(id: EmailId(Id('id1'))),
+        Email(id: EmailId(Id('id5'))),
+      ];
+
+      final result = emails.sortEmailsById(referenceIds);
+
+      expect(
+        result.map((e) => e.id!.asString).toList(),
+        ['id3', 'id4', 'id1', 'id5'],
+      );
+    });
+
+    test('Both lists are empty', () {
+      final referenceIds = <Id>[];
+      final emails = <Email>[];
+
+      final result = emails.sortEmailsById(referenceIds);
+
+      expect(
+        result.map((e) => e.id!.asString).toList(),
+        [],
+      );
+    });
+
+    test('Emails have ids that do not match referenceIds', () {
+      final referenceIds = <Id>[
+        Id('id1'),
+        Id('id2'),
+      ];
+
+      final emails = <Email>[
+        Email(id: EmailId(Id('id3'))),
+        Email(id: EmailId(Id('id4'))),
+        Email(id: EmailId(Id('id5'))),
+      ];
+
+      final result = emails.sortEmailsById(referenceIds);
+
+      expect(
+        result.map((e) => e.id!.asString).toList(),
+        ['id3', 'id4', 'id5'],
+      );
+    });
+
+    test('should keep emails with null IDs in their original order at the end', () {
+      // Arrange
+      final referenceIds = [
+        Id('id2'),
+        Id('id1'),
+      ];
+
+      final emails = [
+        Email(id: EmailId(Id('id1'))),
+        Email(id: null),
+        Email(id: EmailId(Id('id2'))),
+      ];
+
+      // Act
+      final sortedEmails = emails.sortEmailsById(referenceIds);
+
+      // Assert
+      expect(
+        sortedEmails.map((e) => e.id?.asString).toList(),
+        ['id2', 'id1', null],
+      );
+    });
+  });
+}

test/features/thread/data/network/thread_api_test.dart

@@ -2,13 +2,18 @@ import 'package:dio/dio.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:http_mock_adapter/http_mock_adapter.dart';
 import 'package:jmap_dart_client/http/http_client.dart';
+import 'package:jmap_dart_client/jmap/account_id.dart';
 import 'package:jmap_dart_client/jmap/core/error/method/error_method_response.dart';
 import 'package:jmap_dart_client/jmap/core/filter/filter.dart';
 import 'package:jmap_dart_client/jmap/core/id.dart';
 import 'package:jmap_dart_client/jmap/core/state.dart';
+import 'package:jmap_dart_client/jmap/core/unsigned_int.dart';
 import 'package:jmap_dart_client/jmap/mail/email/email.dart';
 import 'package:jmap_dart_client/jmap/mail/email/email_filter_condition.dart';
+import 'package:jmap_dart_client/jmap/mail/email/get/get_email_response.dart';
+import 'package:jmap_dart_client/jmap/mail/email/query/query_email_response.dart';
 import 'package:jmap_dart_client/jmap/mail/email/search_snippet/search_snippet.dart';
+import 'package:model/extensions/email_id_extensions.dart';
 import 'package:tmail_ui_user/features/thread/data/network/thread_api.dart';
 import 'package:tmail_ui_user/features/thread/domain/model/search_email.dart';
 import 'package:tmail_ui_user/features/thread/domain/model/search_emails_response.dart';
@@ -16,6 +21,31 @@ import 'package:tmail_ui_user/features/thread/domain/model/search_emails_respons
 import '../../../../fixtures/account_fixtures.dart';
 import '../../../../fixtures/session_fixtures.dart';
 
+class MockGetEmailResponse extends GetEmailResponse {
+  final List<Email> emailList;
+
+  MockGetEmailResponse(this.emailList) : super(
+    AccountId(Id('abc')),
+    State('123'),
+    emailList,
+    [],
+  );
+}
+
+class MockQueryEmailResponse extends QueryEmailResponse {
+  final Set<Id> idList;
+
+  MockQueryEmailResponse(this.idList) : super(
+    AccountId(Id('abc')),
+    State('123'),
+    false,
+    UnsignedInt(0),
+    idList,
+    UnsignedInt(0),
+    UnsignedInt(0),
+  );
+}
+
 void main() {
   final baseOption  = BaseOptions(method: 'POST');
   final dio = Dio(baseOption)..options.baseUrl = 'http://domain.com/jmap';
@@ -228,5 +258,170 @@ void main() {
         );
       });
     });
+
+    group('sortEmails::test', () {
+      test('Should returns emails as is when emailList is empty', () {
+        final getEmailResponse = MockGetEmailResponse([]);
+        final queryEmailResponse = MockQueryEmailResponse({
+          Id('id1'),
+          Id('id2'),
+        });
+
+        final result = threadApi.sortEmails(
+          getEmailResponse: getEmailResponse,
+          queryEmailResponse: queryEmailResponse,
+        );
+
+        expect(result, []);
+      });
+
+      test('Should returns emails as is when idList is empty', () {
+        final getEmailResponse = MockGetEmailResponse([
+          Email(id: EmailId(Id('id1'))),
+          Email(id: EmailId(Id('id2'))),
+        ]);
+        final queryEmailResponse = MockQueryEmailResponse({});
+
+        final result = threadApi.sortEmails(
+          getEmailResponse: getEmailResponse,
+          queryEmailResponse: queryEmailResponse,
+        );
+
+        expect(result, [
+          Email(id: EmailId(Id('id1'))),
+          Email(id: EmailId(Id('id2'))),
+        ]);
+      });
+
+      test('Sorts emails according to idList', () {
+        final getEmailResponse = MockGetEmailResponse([
+          Email(id: EmailId(Id('id1'))),
+          Email(id: EmailId(Id('id2'))),
+          Email(id: EmailId(Id('id3'))),
+        ]);
+        final queryEmailResponse = MockQueryEmailResponse({
+          Id('id2'),
+          Id('id3'),
+          Id('id1'),
+        });
+
+        final result = threadApi.sortEmails(
+          getEmailResponse: getEmailResponse,
+          queryEmailResponse: queryEmailResponse,
+        );
+
+        expect(
+          result?.map((e) => e.id!.asString).toList(),
+          ['id2', 'id3', 'id1'],
+        );
+      });
+
+      test('Should returns null if getEmailResponse is null', () {
+        const getEmailResponse = null;
+        final queryEmailResponse = MockQueryEmailResponse({Id('id1')});
+
+        final result = threadApi.sortEmails(
+          getEmailResponse: getEmailResponse,
+          queryEmailResponse: queryEmailResponse,
+        );
+
+        expect(result, isNull);
+      });
+
+      test('Should returns emails as is when queryEmailResponse is null', () {
+        final getEmailResponse = MockGetEmailResponse([
+          Email(id: EmailId(Id('id1'))),
+          Email(id: EmailId(Id('id2'))),
+        ]);
+        const queryEmailResponse = null;
+
+        final result = threadApi.sortEmails(
+          getEmailResponse: getEmailResponse,
+          queryEmailResponse: queryEmailResponse,
+        );
+
+        expect(
+          result,
+          [
+            Email(id: EmailId(Id('id1'))),
+            Email(id: EmailId(Id('id2'))),
+          ],
+        );
+      });
+
+      test('Should remain in original order when the emailList contains emails whose id does not appear in idList', () {
+        final getEmailResponse = MockGetEmailResponse([
+          Email(id: EmailId(Id('id1'))),
+          Email(id: EmailId(Id('id2'))),
+          Email(id: EmailId(Id('id3'))),
+        ]);
+        final queryEmailResponse = MockQueryEmailResponse({
+          Id('id4'),
+          Id('id5'),
+        });
+
+        final result = threadApi.sortEmails(
+          getEmailResponse: getEmailResponse,
+          queryEmailResponse: queryEmailResponse,
+        );
+
+        expect(
+          result?.map((e) => e.id!.asString).toList(),
+          ['id1', 'id2', 'id3'],
+        );
+      });
+
+      test(
+        'Should still be sorted according to the ids that are present in emailList\n'
+        'when idList contains ids that do not match any in emailList',
+      () {
+        final getEmailResponse = MockGetEmailResponse([
+          Email(id: EmailId(Id('id1'))),
+          Email(id: EmailId(Id('id2'))),
+        ]);
+        final queryEmailResponse = MockQueryEmailResponse({
+          Id('id1'),
+          Id('id2'),
+          Id('id3'),
+        });
+
+        final result = threadApi.sortEmails(
+          getEmailResponse: getEmailResponse,
+          queryEmailResponse: queryEmailResponse,
+        );
+
+        expect(
+          result?.map((e) => e.id!.asString).toList(),
+          ['id1', 'id2'],
+        );
+      });
+
+      test(
+        'When both emailList and idList have ids that do not match\n'
+        'only the emails whose ids appear in both lists are sorted according to the order in idList\n'
+        'and emails that are not in idList are preserved in their original positions in the final list',
+      () {
+        final getEmailResponse = MockGetEmailResponse([
+          Email(id: EmailId(Id('id1'))),
+          Email(id: EmailId(Id('id2'))),
+          Email(id: EmailId(Id('id3'))),
+          Email(id: EmailId(Id('id4'))),
+        ]);
+        final queryEmailResponse = MockQueryEmailResponse({
+          Id('id3'),
+          Id('id1'),
+        });
+
+        final result = threadApi.sortEmails(
+          getEmailResponse: getEmailResponse,
+          queryEmailResponse: queryEmailResponse,
+        );
+
+        expect(
+          result?.map((e) => e.id!.asString).toList(),
+          ['id3', 'id1', 'id2', 'id4'],
+        );
+      });
+    });
   });
 }