From d09d3817b591f5ceb20de21218791d04c0bea2b3 Mon Sep 17 00:00:00 2001 From: Salvatore Giordano Date: Tue, 25 Oct 2022 17:07:02 +0200 Subject: [PATCH] =?UTF-8?q?fix(persistence,llc,core,ui):=20deprecated=20so?= =?UTF-8?q?rt,=20add=20channelStateSort=20s=E2=80=A6=20(#1366)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(persistence,llc,core,ui): deprecated sort, add channelStateSort sorting the channels offline using the state * update changelogs * update tests * remove deprecated test * change threashold * minor fixes * remove tests * fix(persistence): minor changes. Signed-off-by: xsahil03x Signed-off-by: xsahil03x Co-authored-by: Sahil Kumar --- .github/workflows/stream_flutter_workflow.yml | 2 +- packages/stream_chat/CHANGELOG.md | 2 + .../stream_chat/lib/src/client/client.dart | 18 ++++-- .../lib/src/core/api/channel_api.dart | 3 +- .../lib/src/db/chat_persistence_client.dart | 5 +- .../test/src/client/client_test.dart | 8 +-- .../src/db/chat_persistence_client_test.dart | 5 +- .../stream_chat_flutter/example/lib/main.dart | 2 +- .../example/lib/split_view.dart | 2 +- .../example/lib/tutorial_part_2.dart | 2 +- .../example/lib/tutorial_part_3.dart | 2 +- .../example/lib/tutorial_part_4.dart | 2 +- .../example/lib/tutorial_part_5.dart | 2 +- .../example/lib/tutorial_part_6.dart | 2 +- .../stream_chat_flutter_core/CHANGELOG.md | 4 ++ .../src/stream_channel_list_controller.dart | 28 ++++++++- packages/stream_chat_persistence/CHANGELOG.md | 2 + .../src/stream_chat_persistence_client.dart | 63 ++++++++++++++++++- .../test/src/dao/channel_query_dao_test.dart | 32 +--------- 19 files changed, 131 insertions(+), 55 deletions(-) diff --git a/.github/workflows/stream_flutter_workflow.yml b/.github/workflows/stream_flutter_workflow.yml index e24582e7..9de7ca4d 100644 --- a/.github/workflows/stream_flutter_workflow.yml +++ b/.github/workflows/stream_flutter_workflow.yml @@ -121,7 +121,7 @@ jobs: uses: VeryGoodOpenSource/very_good_coverage@v1.1.1 with: path: packages/stream_chat_persistence/coverage/lcov.info - min_coverage: 97 + min_coverage: 95 - name: "Stream Chat Flutter Core Coverage Check" uses: VeryGoodOpenSource/very_good_coverage@v1.1.1 with: diff --git a/packages/stream_chat/CHANGELOG.md b/packages/stream_chat/CHANGELOG.md index fe3b08f1..0f00b5ba 100644 --- a/packages/stream_chat/CHANGELOG.md +++ b/packages/stream_chat/CHANGELOG.md @@ -8,6 +8,8 @@ - Remove disposed channel clients from the client state. +- Deprecated the `sort` parameter in `queryChannels` in favor of `channelStateSort`. + ## 5.0.0 - Included the changes from version [4.5.0](#450). diff --git a/packages/stream_chat/lib/src/client/client.dart b/packages/stream_chat/lib/src/client/client.dart index 1ab85e94..a9569c20 100644 --- a/packages/stream_chat/lib/src/client/client.dart +++ b/packages/stream_chat/lib/src/client/client.dart @@ -517,7 +517,10 @@ class StreamChatClient { /// Requests channels with a given query. Stream> queryChannels({ Filter? filter, - List>? sort, + @Deprecated(''' + sort has been deprecated. + Please use channelStateSort instead.''') List>? sort, + List>? channelStateSort, bool state = true, bool watch = true, bool presence = false, @@ -547,7 +550,9 @@ class StreamChatClient { } else { final channels = await queryChannelsOffline( filter: filter, + // ignore: deprecated_member_use_from_same_package sort: sort, + channelStateSort: channelStateSort, paginationParams: paginationParams, ); if (channels.isNotEmpty) yield channels; @@ -555,7 +560,7 @@ class StreamChatClient { try { final newQueryChannelsFuture = queryChannelsOnline( filter: filter, - sort: sort, + sort: channelStateSort ?? sort, state: state, watch: watch, presence: presence, @@ -598,7 +603,7 @@ class StreamChatClient { /// Requests channels with a given query from the API. Future> queryChannelsOnline({ Filter? filter, - List>? sort, + List? sort, bool state = true, bool watch = true, bool presence = false, @@ -672,12 +677,17 @@ class StreamChatClient { /// Requests channels with a given query from the Persistence client. Future> queryChannelsOffline({ Filter? filter, - List>? sort, + @Deprecated(''' + sort has been deprecated. + Please use channelStateSort instead.''') List>? sort, + List>? channelStateSort, PaginationParams paginationParams = const PaginationParams(), }) async { final offlineChannels = (await _chatPersistenceClient?.getChannelStates( filter: filter, + // ignore: deprecated_member_use_from_same_package sort: sort, + channelStateSort: channelStateSort, paginationParams: paginationParams, )) ?? []; diff --git a/packages/stream_chat/lib/src/core/api/channel_api.dart b/packages/stream_chat/lib/src/core/api/channel_api.dart index 08f6b6e4..84780418 100644 --- a/packages/stream_chat/lib/src/core/api/channel_api.dart +++ b/packages/stream_chat/lib/src/core/api/channel_api.dart @@ -3,7 +3,6 @@ import 'dart:convert'; import 'package:stream_chat/src/core/api/requests.dart'; import 'package:stream_chat/src/core/api/responses.dart'; import 'package:stream_chat/src/core/http/stream_http_client.dart'; -import 'package:stream_chat/src/core/models/channel_model.dart'; import 'package:stream_chat/src/core/models/channel_state.dart'; import 'package:stream_chat/src/core/models/event.dart'; import 'package:stream_chat/src/core/models/filter.dart'; @@ -51,7 +50,7 @@ class ChannelApi { /// Requests channels with a given query from the API. Future queryChannels({ Filter? filter, - List>? sort, + List? sort, int? memberLimit, int? messageLimit, bool state = true, diff --git a/packages/stream_chat/lib/src/db/chat_persistence_client.dart b/packages/stream_chat/lib/src/db/chat_persistence_client.dart index 3dfbbc81..1826c4b5 100644 --- a/packages/stream_chat/lib/src/db/chat_persistence_client.dart +++ b/packages/stream_chat/lib/src/db/chat_persistence_client.dart @@ -94,7 +94,10 @@ abstract class ChatPersistenceClient { /// for filtering out states. Future> getChannelStates({ Filter? filter, - List>? sort, + @Deprecated(''' + sort has been deprecated. + Please use channelStateSort instead.''') List>? sort, + List>? channelStateSort, PaginationParams? paginationParams, }); diff --git a/packages/stream_chat/test/src/client/client_test.dart b/packages/stream_chat/test/src/client/client_test.dart index 5317e6c3..bc6ff843 100644 --- a/packages/stream_chat/test/src/client/client_test.dart +++ b/packages/stream_chat/test/src/client/client_test.dart @@ -618,7 +618,7 @@ void main() { when(() => persistence.getChannelStates( filter: any(named: 'filter'), - sort: any(named: 'sort'), + channelStateSort: any(named: 'channelStateSort'), paginationParams: any(named: 'paginationParams'), )).thenAnswer((_) async => persistentChannelStates); @@ -674,7 +674,7 @@ void main() { verify(() => persistence.getChannelStates( filter: any(named: 'filter'), - sort: any(named: 'sort'), + channelStateSort: any(named: 'channelStateSort'), paginationParams: any(named: 'paginationParams'), )).called(1); @@ -715,7 +715,7 @@ void main() { when(() => persistence.getChannelStates( filter: any(named: 'filter'), - sort: any(named: 'sort'), + channelStateSort: any(named: 'channelStateSort'), paginationParams: any(named: 'paginationParams'), )).thenAnswer((_) async => persistentChannelStates); @@ -757,7 +757,7 @@ void main() { verify(() => persistence.getChannelStates( filter: any(named: 'filter'), - sort: any(named: 'sort'), + channelStateSort: any(named: 'channelStateSort'), paginationParams: any(named: 'paginationParams'), )).called(1); diff --git a/packages/stream_chat/test/src/db/chat_persistence_client_test.dart b/packages/stream_chat/test/src/db/chat_persistence_client_test.dart index 121d279d..33e9b52b 100644 --- a/packages/stream_chat/test/src/db/chat_persistence_client_test.dart +++ b/packages/stream_chat/test/src/db/chat_persistence_client_test.dart @@ -56,7 +56,10 @@ class TestPersistenceClient extends ChatPersistenceClient { @override Future> getChannelStates( {Filter? filter, - List>? sort, + @Deprecated(''' + sort has been deprecated. + Please use channelStateSort instead.''') List>? sort, + List>? channelStateSort, PaginationParams? paginationParams}) => throw UnimplementedError(); diff --git a/packages/stream_chat_flutter/example/lib/main.dart b/packages/stream_chat_flutter/example/lib/main.dart index 0beffcc0..67420d06 100644 --- a/packages/stream_chat_flutter/example/lib/main.dart +++ b/packages/stream_chat_flutter/example/lib/main.dart @@ -209,7 +209,7 @@ class _ChannelListPageState extends State { 'members', [StreamChat.of(context).currentUser!.id], ), - sort: const [SortOption('last_message_at')], + channelStateSort: const [SortOption('last_message_at')], limit: 20, ); diff --git a/packages/stream_chat_flutter/example/lib/split_view.dart b/packages/stream_chat_flutter/example/lib/split_view.dart index 4ecc6079..2ae8e8aa 100644 --- a/packages/stream_chat_flutter/example/lib/split_view.dart +++ b/packages/stream_chat_flutter/example/lib/split_view.dart @@ -107,7 +107,7 @@ class _ChannelListPageState extends State { 'members', [StreamChat.of(context).currentUser!.id], ), - sort: const [SortOption('last_message_at')], + channelStateSort: const [SortOption('last_message_at')], limit: 20, ); diff --git a/packages/stream_chat_flutter/example/lib/tutorial_part_2.dart b/packages/stream_chat_flutter/example/lib/tutorial_part_2.dart index 7801e9f1..0c509b51 100644 --- a/packages/stream_chat_flutter/example/lib/tutorial_part_2.dart +++ b/packages/stream_chat_flutter/example/lib/tutorial_part_2.dart @@ -88,7 +88,7 @@ class _ChannelListPageState extends State { 'members', [StreamChat.of(context).currentUser!.id], ), - sort: const [SortOption('last_message_at')], + channelStateSort: const [SortOption('last_message_at')], ); @override diff --git a/packages/stream_chat_flutter/example/lib/tutorial_part_3.dart b/packages/stream_chat_flutter/example/lib/tutorial_part_3.dart index 110f8d01..39dca5fa 100644 --- a/packages/stream_chat_flutter/example/lib/tutorial_part_3.dart +++ b/packages/stream_chat_flutter/example/lib/tutorial_part_3.dart @@ -84,7 +84,7 @@ class _ChannelListPageState extends State { 'members', [StreamChat.of(context).currentUser!.id], ), - sort: const [SortOption('last_message_at')], + channelStateSort: const [SortOption('last_message_at')], limit: 20, ); diff --git a/packages/stream_chat_flutter/example/lib/tutorial_part_4.dart b/packages/stream_chat_flutter/example/lib/tutorial_part_4.dart index 439c4718..290f621c 100644 --- a/packages/stream_chat_flutter/example/lib/tutorial_part_4.dart +++ b/packages/stream_chat_flutter/example/lib/tutorial_part_4.dart @@ -70,7 +70,7 @@ class _ChannelListPageState extends State { 'members', [StreamChat.of(context).currentUser!.id], ), - sort: const [SortOption('last_message_at')], + channelStateSort: const [SortOption('last_message_at')], limit: 20, ); diff --git a/packages/stream_chat_flutter/example/lib/tutorial_part_5.dart b/packages/stream_chat_flutter/example/lib/tutorial_part_5.dart index 3a8ff420..adaf7228 100644 --- a/packages/stream_chat_flutter/example/lib/tutorial_part_5.dart +++ b/packages/stream_chat_flutter/example/lib/tutorial_part_5.dart @@ -74,7 +74,7 @@ class _ChannelListPageState extends State { 'members', [StreamChat.of(context).currentUser!.id], ), - sort: const [SortOption('last_message_at')], + channelStateSort: const [SortOption('last_message_at')], limit: 20, ); diff --git a/packages/stream_chat_flutter/example/lib/tutorial_part_6.dart b/packages/stream_chat_flutter/example/lib/tutorial_part_6.dart index 7dbe41bf..cd80af44 100644 --- a/packages/stream_chat_flutter/example/lib/tutorial_part_6.dart +++ b/packages/stream_chat_flutter/example/lib/tutorial_part_6.dart @@ -113,7 +113,7 @@ class _ChannelListPageState extends State { 'members', [StreamChat.of(context).currentUser!.id], ), - sort: const [SortOption('last_message_at')], + channelStateSort: const [SortOption('last_message_at')], limit: 20, ); diff --git a/packages/stream_chat_flutter_core/CHANGELOG.md b/packages/stream_chat_flutter_core/CHANGELOG.md index 96c2a8ed..3d7f8703 100644 --- a/packages/stream_chat_flutter_core/CHANGELOG.md +++ b/packages/stream_chat_flutter_core/CHANGELOG.md @@ -1,3 +1,7 @@ +## Upcoming + +- Deprecated the `sort` parameter in the `StreamChannelListController` in favor of `channelStateSort`. + ## 5.0.0 - Included the changes from version [4.5.0](#450). diff --git a/packages/stream_chat_flutter_core/lib/src/stream_channel_list_controller.dart b/packages/stream_chat_flutter_core/lib/src/stream_channel_list_controller.dart index 7cd425a6..bc98780d 100644 --- a/packages/stream_chat_flutter_core/lib/src/stream_channel_list_controller.dart +++ b/packages/stream_chat_flutter_core/lib/src/stream_channel_list_controller.dart @@ -45,7 +45,10 @@ class StreamChannelListController extends PagedValueNotifier { required this.client, StreamChannelListEventHandler? eventHandler, this.filter, - this.sort, + @Deprecated(''' + sort has been deprecated. + Please use channelStateSort instead.''') this.sort, + this.channelStateSort, this.presence = true, this.limit = defaultChannelPagedLimit, this.messageLimit, @@ -59,7 +62,10 @@ class StreamChannelListController extends PagedValueNotifier { required this.client, StreamChannelListEventHandler? eventHandler, this.filter, - this.sort, + this.channelStateSort, + @Deprecated(''' + sort has been deprecated. + Please use channelStateSort instead.''') this.sort, this.presence = true, this.limit = defaultChannelPagedLimit, this.messageLimit, @@ -88,8 +94,22 @@ class StreamChannelListController extends PagedValueNotifier { /// created_at or member_count. /// /// Direction can be ascending or descending. + @Deprecated(''' + sort has been deprecated. + Please use channelStateSort instead.''') final List>? sort; + /// The sorting used for the channels matching the filters. + /// + /// Sorting is based on field and direction, multiple sorting options + /// can be provided. + /// + /// You can sort based on last_updated, last_message_at, updated_at, + /// created_at or member_count. + /// + /// Direction can be ascending or descending. + final List>? channelStateSort; + /// If true you’ll receive user presence updates via the websocket events final bool presence; @@ -112,6 +132,8 @@ class StreamChannelListController extends PagedValueNotifier { try { await for (final channels in client.queryChannels( filter: filter, + channelStateSort: channelStateSort, + // ignore: deprecated_member_use, deprecated_member_use_from_same_package sort: sort, memberLimit: memberLimit, messageLimit: messageLimit, @@ -141,7 +163,9 @@ class StreamChannelListController extends PagedValueNotifier { try { await for (final channels in client.queryChannels( filter: filter, + // ignore: deprecated_member_use, deprecated_member_use_from_same_package sort: sort, + channelStateSort: channelStateSort, memberLimit: memberLimit, messageLimit: messageLimit, presence: presence, diff --git a/packages/stream_chat_persistence/CHANGELOG.md b/packages/stream_chat_persistence/CHANGELOG.md index 063c6e8f..e5272d34 100644 --- a/packages/stream_chat_persistence/CHANGELOG.md +++ b/packages/stream_chat_persistence/CHANGELOG.md @@ -1,6 +1,8 @@ ## Upcoming - Reintroduce support for experimental indexedDB on Web. +- Deprecated the `sort` parameter in the getChannelStates method in favor of `channelStateSort`. +- Use the comparator function to sort the channel states and not the channel models. 🐞 Fixed diff --git a/packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart b/packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart index 3cb4eef1..48e563dc 100644 --- a/packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart +++ b/packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart @@ -263,19 +263,76 @@ class StreamChatPersistenceClient extends ChatPersistenceClient { @override Future> getChannelStates({ Filter? filter, - List>? sort, + @Deprecated(''' + sort has been deprecated. + Please use channelStateSort instead.''') List>? sort, + List>? channelStateSort, PaginationParams? paginationParams, }) { assert(_debugIsConnected, ''); + assert( + sort == null || channelStateSort == null, + 'sort and channelStateSort cannot be used together', + ); _logger.info('getChannelStates'); return _readProtected( () async { final channels = await db!.channelQueryDao.getChannels( filter: filter, sort: sort, - paginationParams: paginationParams, ); - return Future.wait(channels.map((e) => getChannelStateByCid(e.cid))); + + final channelStates = await Future.wait( + channels.map((e) => getChannelStateByCid(e.cid)), + ); + + // Only sort the channel states if the channels are not already sorted. + if (sort == null) { + var chainedComparator = (ChannelState a, ChannelState b) { + final dateA = a.channel?.lastMessageAt ?? a.channel?.createdAt; + final dateB = b.channel?.lastMessageAt ?? b.channel?.createdAt; + + if (dateA == null && dateB == null) { + return 0; + } else if (dateA == null) { + return 1; + } else if (dateB == null) { + return -1; + } else { + return dateB.compareTo(dateA); + } + }; + + if (channelStateSort != null && channelStateSort.isNotEmpty) { + chainedComparator = (a, b) { + int result; + for (final comparator in channelStateSort + .map((it) => it.comparator) + .withNullifyer) { + try { + result = comparator(a, b); + } catch (e) { + result = 0; + } + if (result != 0) return result; + } + return 0; + }; + } + + channelStates.sort(chainedComparator); + } + + final offset = paginationParams?.offset; + if (offset != null && offset > 0 && channelStates.isNotEmpty) { + channelStates.removeRange(0, offset); + } + + if (paginationParams?.limit != null) { + return channelStates.take(paginationParams!.limit).toList(); + } + + return channelStates; }, ); } diff --git a/packages/stream_chat_persistence/test/src/dao/channel_query_dao_test.dart b/packages/stream_chat_persistence/test/src/dao/channel_query_dao_test.dart index 8319d297..08905719 100644 --- a/packages/stream_chat_persistence/test/src/dao/channel_query_dao_test.dart +++ b/packages/stream_chat_persistence/test/src/dao/channel_query_dao_test.dart @@ -137,27 +137,6 @@ void main() { } }); - test( - 'should return all the inserted channels along with pagination applied', - () async { - const offset = 5; - const limit = 15; - const pagination = PaginationParams(offset: offset, limit: limit); - - // Inserting test data for get channels - await _insertTestDataForGetChannel(filter, count: 30); - - // Should match with the inserted channels - final updatedChannels = await channelQueryDao.getChannels( - filter: filter, - paginationParams: pagination, - ); - expect(updatedChannels.length, limit); - expect(updatedChannels.first.id, 'testId24'); - expect(updatedChannels.first.cid, 'testCid24'); - }, - ); - test('should return sorted channels using member count', () async { int sortComparator(ChannelModel a, ChannelModel b) => b.memberCount.compareTo(a.memberCount); @@ -169,6 +148,7 @@ void main() { // Should match with the inserted channels final updatedChannels = await channelQueryDao.getChannels( filter: filter, + // ignore: deprecated_member_use_from_same_package sort: [ SortOption( 'member_count', @@ -202,15 +182,6 @@ void main() { } }); - test('should throw if comparator is not provided in sort list', () { - expect( - () => channelQueryDao.getChannels( - sort: [const SortOption('test_custom_field')], - ), - throwsArgumentError, - ); - }); - test('should return sorted channels using custom field', () async { int sortComparator(ChannelModel a, ChannelModel b) { final aData = int.parse(a.extraData['test_custom_field'].toString()); @@ -225,6 +196,7 @@ void main() { // Should match with the inserted channels final updatedChannels = await channelQueryDao.getChannels( filter: filter, + // ignore: deprecated_member_use_from_same_package sort: [SortOption('test_custom_field', comparator: sortComparator)], );