diff --git a/lib/api/route/channels.dart b/lib/api/route/channels.dart index c21a6c8752..67647a4a9b 100644 --- a/lib/api/route/channels.dart +++ b/lib/api/route/channels.dart @@ -41,43 +41,43 @@ Future unsubscribeFromChannel(ApiConnection connection, { } /// https://zulip.com/api/get-stream-topics -Future getStreamTopics(ApiConnection connection, { - required int streamId, +Future getChannelTopics(ApiConnection connection, { + required int channelId, required bool allowEmptyTopicName, }) { assert(allowEmptyTopicName, '`allowEmptyTopicName` should only be true'); - return connection.get('getStreamTopics', GetStreamTopicsResult.fromJson, 'users/me/$streamId/topics', { + return connection.get('getChannelTopics', GetChannelTopicsResult.fromJson, 'users/me/$channelId/topics', { 'allow_empty_topic_name': allowEmptyTopicName, }); } @JsonSerializable(fieldRename: FieldRename.snake) -class GetStreamTopicsResult { - final List topics; +class GetChannelTopicsResult { + final List topics; - GetStreamTopicsResult({ + GetChannelTopicsResult({ required this.topics, }); - factory GetStreamTopicsResult.fromJson(Map json) => - _$GetStreamTopicsResultFromJson(json); + factory GetChannelTopicsResult.fromJson(Map json) => + _$GetChannelTopicsResultFromJson(json); - Map toJson() => _$GetStreamTopicsResultToJson(this); + Map toJson() => _$GetChannelTopicsResultToJson(this); } @JsonSerializable(fieldRename: FieldRename.snake) -class GetStreamTopicsEntry { +class GetChannelTopicsEntry { final int maxId; final TopicName name; - GetStreamTopicsEntry({ + GetChannelTopicsEntry({ required this.maxId, required this.name, }); - factory GetStreamTopicsEntry.fromJson(Map json) => _$GetStreamTopicsEntryFromJson(json); + factory GetChannelTopicsEntry.fromJson(Map json) => _$GetChannelTopicsEntryFromJson(json); - Map toJson() => _$GetStreamTopicsEntryToJson(this); + Map toJson() => _$GetChannelTopicsEntryToJson(this); } /// Update a topic's visibility policy. diff --git a/lib/api/route/channels.g.dart b/lib/api/route/channels.g.dart index c43f0f50f0..38c18a3b7b 100644 --- a/lib/api/route/channels.g.dart +++ b/lib/api/route/channels.g.dart @@ -8,25 +8,25 @@ part of 'channels.dart'; // JsonSerializableGenerator // ************************************************************************** -GetStreamTopicsResult _$GetStreamTopicsResultFromJson( +GetChannelTopicsResult _$GetChannelTopicsResultFromJson( Map json, -) => GetStreamTopicsResult( +) => GetChannelTopicsResult( topics: (json['topics'] as List) - .map((e) => GetStreamTopicsEntry.fromJson(e as Map)) + .map((e) => GetChannelTopicsEntry.fromJson(e as Map)) .toList(), ); -Map _$GetStreamTopicsResultToJson( - GetStreamTopicsResult instance, +Map _$GetChannelTopicsResultToJson( + GetChannelTopicsResult instance, ) => {'topics': instance.topics}; -GetStreamTopicsEntry _$GetStreamTopicsEntryFromJson( +GetChannelTopicsEntry _$GetChannelTopicsEntryFromJson( Map json, -) => GetStreamTopicsEntry( +) => GetChannelTopicsEntry( maxId: (json['max_id'] as num).toInt(), name: TopicName.fromJson(json['name'] as String), ); -Map _$GetStreamTopicsEntryToJson( - GetStreamTopicsEntry instance, +Map _$GetChannelTopicsEntryToJson( + GetChannelTopicsEntry instance, ) => {'max_id': instance.maxId, 'name': instance.name}; diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index d87fb466c8..a84a6834d2 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -7,7 +7,6 @@ import 'package:unorm_dart/unorm_dart.dart' as unorm; import '../api/model/events.dart'; import '../api/model/model.dart'; -import '../api/route/channels.dart'; import '../generated/l10n/zulip_localizations.dart'; import '../widgets/compose_box.dart'; import 'algorithms.dart'; @@ -1239,22 +1238,15 @@ class TopicAutocompleteView extends AutocompleteView _topics = []; - bool _isFetching = false; - /// Fetches topics of the current stream narrow, expected to fetch - /// only once per lifecycle. + /// Fetches topics of the current stream narrow. /// /// Starts fetching once the stream narrow is active, then when results /// are fetched it restarts search to refresh UI showing the newly /// fetched topics. Future _fetch() async { - assert(!_isFetching); - _isFetching = true; - final result = await getStreamTopics(store.connection, streamId: streamId, - allowEmptyTopicName: true, - ); - _topics = result.topics.map((e) => e.name); - _isFetching = false; + await store.topics.fetchChannelTopics(streamId); // TODO: handle fetch failure + _topics = store.topics.getChannelTopics(streamId)!.map((e) => e.name); return _startSearch(); } diff --git a/lib/model/narrow.dart b/lib/model/narrow.dart index 863867a195..71a4f26718 100644 --- a/lib/model/narrow.dart +++ b/lib/model/narrow.dart @@ -112,7 +112,7 @@ class TopicNarrow extends Narrow implements SendableNarrow { bool containsMessage(MessageBase message) { final conversation = message.conversation; return conversation is StreamConversation - && conversation.streamId == streamId && conversation.topic == topic; + && conversation.streamId == streamId && conversation.topic.isSameAs(topic); } @override diff --git a/lib/model/store.dart b/lib/model/store.dart index b051e75b72..11615be4ff 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -32,6 +32,7 @@ import 'server_support.dart'; import 'channel.dart'; import 'saved_snippet.dart'; import 'settings.dart'; +import 'topics.dart'; import 'typing_status.dart'; import 'unreads.dart'; import 'user.dart'; @@ -571,6 +572,7 @@ class PerAccountStore extends PerAccountStoreBase with presence: Presence(realm: realm, initial: initialSnapshot.presences), channels: channels, + topics: Topics(core: core), messages: MessageStoreImpl(channels: channels), unreads: Unreads(core: core, channelStore: channels, initial: initialSnapshot.unreadMsgs), @@ -593,6 +595,7 @@ class PerAccountStore extends PerAccountStoreBase with required this.typingStatus, required this.presence, required ChannelStoreImpl channels, + required this.topics, required MessageStoreImpl messages, required this.unreads, required this.recentDmConversationsView, @@ -686,6 +689,8 @@ class PerAccountStore extends PerAccountStoreBase with ChannelStore get channelStore => _channels; final ChannelStoreImpl _channels; + final Topics topics; + //|////////////////////////////// // Messages, and summaries of messages. @@ -879,6 +884,7 @@ class PerAccountStore extends PerAccountStoreBase with unreads.handleMessageEvent(event); recentDmConversationsView.handleMessageEvent(event); recentSenders.handleMessage(event.message); // TODO(#824) + topics.handleMessageEvent(event); // When adding anything here (to handle [MessageEvent]), // it probably belongs in [reconcileMessages] too. @@ -886,6 +892,7 @@ class PerAccountStore extends PerAccountStoreBase with assert(debugLog("server event: update_message ${event.messageId}")); _messages.handleUpdateMessageEvent(event); unreads.handleUpdateMessageEvent(event); + topics.handleUpdateMessageEvent(event); case DeleteMessageEvent(): assert(debugLog("server event: delete_message ${event.messageIds}")); diff --git a/lib/model/topics.dart b/lib/model/topics.dart new file mode 100644 index 0000000000..09018a66a2 --- /dev/null +++ b/lib/model/topics.dart @@ -0,0 +1,158 @@ +import 'dart:async'; + +import 'package:collection/collection.dart'; +import 'package:flutter/foundation.dart'; + +import '../api/model/events.dart'; +import '../api/model/model.dart'; +import '../api/route/channels.dart'; +import 'channel.dart'; +import 'store.dart'; + +// similar to _apiSendMessage in lib/model/message.dart +final _apiGetChannelTopics = getChannelTopics; + +/// The view-model for tracking channel topics. +/// +/// Use [fetchChannelTopics] to first fetch the topics for a channel from the +/// server and then use [getChannelTopics] to get those topics, affected +/// by the relevant events. +class Topics extends PerAccountStoreBase with ChangeNotifier { + Topics({required super.core}); + + /// Maps indexed by channel IDs, of the known latest message IDs in each topic. + /// + /// For example: `_latestMessageIdsByChannelTopic[channel.streamId][topic] = maxId` + /// + /// Occasionally, the latest message ID of a topic will refer to a message + /// that doesn't exist or is no longer in the topic. + /// This happens when the topic's latest message is deleted or moved + /// and we don't have enough information to replace it accurately. + /// (We don't keep a snapshot of all messages.) + // TODO(#2004): handle more cases where this can change + final Map> _latestMessageIdsByChannelTopic = {}; + + final Map> _channelTopicsFetching = {}; + + /// Fetch topics in a channel from the server, only if they're not fetched yet. + /// + /// Once fetched, the data will be updated by events; + /// use [getChannelTopics] to consume the data. + Future fetchChannelTopics(int channelId) async { + if (_latestMessageIdsByChannelTopic[channelId] != null) return; + + Future? future = _channelTopicsFetching[channelId]; + // If another call has already started fetching topics for this channel, + // ignore this call. + if (future != null) return; + + future = _apiGetChannelTopics(connection, channelId: channelId, + allowEmptyTopicName: true); + _channelTopicsFetching[channelId] = future; + + try { + final result = await future; + assert(_latestMessageIdsByChannelTopic[channelId] == null); + (_latestMessageIdsByChannelTopic[channelId] = makeTopicKeyedMap()) + .addEntries(result.topics.map((entry) => MapEntry(entry.name, entry.maxId))); + } finally { + unawaited(_channelTopicsFetching.remove(channelId)); + } + } + + /// Map of topics per channel, sorted by latest message IDs descending. + /// + /// Derived from [_latestMessageIdsByChannelTopic]; + /// used for optimized topics sorting. + /// + /// A channel entry should be discarded when there is a change to + /// the ordering of its topics or the channel is no longer present + /// in [_latestMessageIdsByChannelTopic], allowing [getChannelTopics] + /// to recalculate the sorted topics on demand. + // TODO(#2004): handle more cases where this can change + final Map> _sortedTopicsByChannel = {}; + + /// The topics the user can access, along with their latest message ID, + /// reflecting updates from events that arrived since the data was fetched. + /// + /// Returns null if the data has not been fetched yet. + /// To fetch it from the server, use [fetchChannelTopics]. + /// + /// The result is sorted by [GetChannelTopicsEntry.maxId] descending, + /// and the topics are distinct. + /// + /// Occasionally, [GetChannelTopicsEntry.maxId] will refer to a message + /// that doesn't exist or is no longer in the topic. + /// This happens when a topic's latest message is deleted or moved + /// and we don't have enough information + /// to replace [GetChannelTopicsEntry.maxId] accurately. + /// (We don't keep a snapshot of all messages.) + /// Use [PerAccountStore.messages] to check a message's topic accurately. + List? getChannelTopics(int channelId) { + final latestMessageIdsByTopic = _latestMessageIdsByChannelTopic[channelId]; + if (latestMessageIdsByTopic == null) return null; + return _sortedTopicsByChannel[channelId] ??= latestMessageIdsByTopic.entries + .map((e) => GetChannelTopicsEntry(maxId: e.value, name: e.key)) + .sortedBy((value) => -value.maxId); + } + + void handleMessageEvent(MessageEvent event) { + if (event.message is! StreamMessage) return; + final StreamMessage(:id, streamId: channelId, :topic) = event.message as StreamMessage; + + final latestMessageIdsByTopic = _latestMessageIdsByChannelTopic[channelId]; + if (latestMessageIdsByTopic == null) { + // We're not tracking this channel's topics yet. + // We'll start doing that when we get the full topic list; + // see [fetchChannelTopics]. + return; + } + + final currentLatestMessageId = latestMessageIdsByTopic[topic]; + if (currentLatestMessageId != null && currentLatestMessageId >= id) { + // The event raced with a topic-list fetch. + return; + } + latestMessageIdsByTopic[topic] = id; + _sortedTopicsByChannel.remove(channelId); + notifyListeners(); + } + + void handleUpdateMessageEvent(UpdateMessageEvent event) { + if (event.moveData == null) return; + final UpdateMessageMoveData( + :origStreamId, :origTopic, :newStreamId, :newTopic, :propagateMode, + ) = event.moveData!; + bool shouldNotify = false; + + final origLatestMessageIdsByTopic = _latestMessageIdsByChannelTopic[origStreamId]; + if (origLatestMessageIdsByTopic != null) { + switch (propagateMode) { + case .changeOne: + case .changeLater: + // We can't know the new `maxId` for the original topic. + // Shrug; leave it unchanged. (See dartdoc of [getChannelTopics], + // where we call out this possibility that `maxId` is incorrect.) + break; + case .changeAll: + origLatestMessageIdsByTopic.remove(origTopic); + _sortedTopicsByChannel.remove(origStreamId); + shouldNotify = true; + } + } + + final newLatestMessageIdsByTopic = _latestMessageIdsByChannelTopic[newStreamId]; + if (newLatestMessageIdsByTopic != null) { + // TODO(server-11): rely on `event.messageIds` being sorted, to avoid this linear scan + final movedMaxId = event.messageIds.max; + final currentMaxId = newLatestMessageIdsByTopic[newTopic]; + if (currentMaxId == null || currentMaxId < movedMaxId) { + newLatestMessageIdsByTopic[newTopic] = movedMaxId; + _sortedTopicsByChannel.remove(newStreamId); + shouldNotify = true; + } + } + + if (shouldNotify) notifyListeners(); + } +} diff --git a/lib/widgets/topic_list.dart b/lib/widgets/topic_list.dart index aad7866f8c..634547c140 100644 --- a/lib/widgets/topic_list.dart +++ b/lib/widgets/topic_list.dart @@ -4,6 +4,7 @@ import '../api/model/model.dart'; import '../api/route/channels.dart'; import '../generated/l10n/zulip_localizations.dart'; import '../model/narrow.dart'; +import '../model/topics.dart'; import '../model/unreads.dart'; import 'action_sheet.dart'; import 'app_bar.dart'; @@ -125,27 +126,28 @@ class _TopicList extends StatefulWidget { class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMixin { Unreads? unreadsModel; - // TODO(#1499): store the results on [ChannelStore], and keep them - // up-to-date by handling events - List? lastFetchedTopics; + Topics? topicsModel; @override void onNewStore() { + final newStore = PerAccountStoreWidget.of(context); unreadsModel?.removeListener(_modelChanged); - final store = PerAccountStoreWidget.of(context); - unreadsModel = store.unreads..addListener(_modelChanged); + unreadsModel = newStore.unreads..addListener(_modelChanged); + topicsModel?.removeListener(_modelChanged); + topicsModel = newStore.topics..addListener(_modelChanged); _fetchTopics(); } @override void dispose() { unreadsModel?.removeListener(_modelChanged); + topicsModel?.removeListener(_modelChanged); super.dispose(); } void _modelChanged() { setState(() { - // The actual state lives in `unreadsModel`. + // The actual states live in `unreadsModel` and `topicsModel`. }); } @@ -153,23 +155,21 @@ class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMi // Do nothing when the fetch fails; the topic-list will stay on // the loading screen, until the user navigates away and back. // TODO(design) show a nice error message on screen when this fails - final store = PerAccountStoreWidget.of(context); - final result = await getStreamTopics(store.connection, - streamId: widget.streamId, - allowEmptyTopicName: true); + await topicsModel!.fetchChannelTopics(widget.streamId); if (!mounted) return; setState(() { - lastFetchedTopics = result.topics; + // The actual state lives in the `topicsModel`. }); } @override Widget build(BuildContext context) { - if (lastFetchedTopics == null) { + final channelTopics = topicsModel!.getChannelTopics(widget.streamId); + if (channelTopics == null) { return const Center(child: CircularProgressIndicator()); } - if (lastFetchedTopics!.isEmpty) { + if (channelTopics.isEmpty) { final zulipLocalizations = ZulipLocalizations.of(context); return PageBodyEmptyContentPlaceholder( header: zulipLocalizations.topicListEmptyPlaceholderHeader); @@ -177,7 +177,7 @@ class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMi // This is adapted from parts of the build method on [_InboxPageState]. final topicItems = <_TopicItemData>[]; - for (final GetStreamTopicsEntry(:maxId, name: topic) in lastFetchedTopics!) { + for (final GetChannelTopicsEntry(:maxId, name: topic) in channelTopics) { final unreadMessageIds = unreadsModel!.streams[widget.streamId]?[topic] ?? []; final countInTopic = unreadMessageIds.length; @@ -187,17 +187,9 @@ class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMi topic: topic, unreadCount: countInTopic, hasMention: hasMention, - // `lastFetchedTopics.maxId` can become outdated when a new message - // arrives or when there are message moves, until we re-fetch. - // TODO(#1499): track changes to this maxId: maxId, )); } - topicItems.sort((a, b) { - final aMaxId = a.maxId; - final bMaxId = b.maxId; - return bMaxId.compareTo(aMaxId); - }); return SafeArea( // Don't pad the bottom here; we want the list content to do that. @@ -239,6 +231,14 @@ class _TopicItem extends StatelessWidget { final store = PerAccountStoreWidget.of(context); final designVariables = DesignVariables.of(context); + // `maxId` might be incorrect (see [ChannelStore.getChannelTopics]). + // Check if it refers to a message that's currently in the topic; + // if not, we just won't have `someMessageIdInTopic` for the action sheet. + final maxIdMessage = store.messages[maxId]; + final someMessageIdInTopic = + (maxIdMessage != null && TopicNarrow(streamId, topic).containsMessage(maxIdMessage)) + ? maxIdMessage.id + : null; final visibilityPolicy = store.topicVisibilityPolicy(streamId, topic); final double opacity; @@ -267,7 +267,7 @@ class _TopicItem extends StatelessWidget { onLongPress: () => showTopicActionSheet(context, channelId: streamId, topic: topic, - someMessageIdInTopic: maxId), + someMessageIdInTopic: someMessageIdInTopic), splashFactory: NoSplash.splashFactory, child: ConstrainedBox( constraints: BoxConstraints(minHeight: 40), diff --git a/test/api/route/route_checks.dart b/test/api/route/route_checks.dart index daa4628efd..93f5f6a7fe 100644 --- a/test/api/route/route_checks.dart +++ b/test/api/route/route_checks.dart @@ -1,4 +1,6 @@ import 'package:checks/checks.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/route/channels.dart'; import 'package:zulip/api/route/messages.dart'; import 'package:zulip/api/route/realm.dart'; import 'package:zulip/api/route/saved_snippets.dart'; @@ -15,4 +17,9 @@ extension GetServerSettingsResultChecks on Subject { Subject get realmUrl => has((e) => e.realmUrl, 'realmUrl'); } +extension GetChannelTopicsEntryChecks on Subject { + Subject get maxId => has((e) => e.maxId, 'maxId'); + Subject get name => has((e) => e.name, 'name'); +} + // TODO add similar extensions for other classes in api/route/*.dart diff --git a/test/example_data.dart b/test/example_data.dart index c9beaef72f..7cf179ecca 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -510,9 +510,9 @@ ZulipStream stream({ } const _stream = stream; -GetStreamTopicsEntry getStreamTopicsEntry({int? maxId, String? name}) { +GetChannelTopicsEntry getChannelTopicsEntry({int? maxId, String? name}) { maxId ??= 123; - return GetStreamTopicsEntry(maxId: maxId, + return GetChannelTopicsEntry(maxId: maxId, name: TopicName(name ?? 'Test Topic #$maxId')); } diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index 9f0381b40c..4764c2c4a2 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -1327,13 +1327,17 @@ void main() { doTest('a^bc^', TopicAutocompleteQuery('abc')); }); + Condition isTopic(TopicName topic) { + return (it) => it.isA().topic.equals(topic); + } + test('TopicAutocompleteView misc', () async { final store = eg.store(); final connection = store.connection as FakeApiConnection; - final first = eg.getStreamTopicsEntry(maxId: 1, name: 'First Topic'); - final second = eg.getStreamTopicsEntry(maxId: 2, name: 'Second Topic'); - final third = eg.getStreamTopicsEntry(maxId: 3, name: 'Third Topic'); - connection.prepare(json: GetStreamTopicsResult( + final first = eg.getChannelTopicsEntry(maxId: 1, name: 'First Topic'); + final second = eg.getChannelTopicsEntry(maxId: 2, name: 'Second Topic'); + final third = eg.getChannelTopicsEntry(maxId: 3, name: 'Third Topic'); + connection.prepare(json: GetChannelTopicsResult( topics: [first, second, third]).toJson()); final view = TopicAutocompleteView.init( @@ -1347,16 +1351,14 @@ void main() { await Future(() {}); await Future(() {}); check(done).isTrue(); - check(view.results).single - .isA() - .topic.equals(third.name); + check(view.results).single.which(isTopic(third.name)); }); - test('TopicAutocompleteView updates results when streams are loaded', () async { + test('TopicAutocompleteView updates results when topics are loaded', () async { final store = eg.store(); final connection = store.connection as FakeApiConnection; - connection.prepare(json: GetStreamTopicsResult( - topics: [eg.getStreamTopicsEntry(name: 'test')] + connection.prepare(json: GetChannelTopicsResult( + topics: [eg.getChannelTopicsEntry(name: 'test')] ).toJson()); final view = TopicAutocompleteView.init( @@ -1371,19 +1373,47 @@ void main() { check(done).isTrue(); }); - test('TopicAutocompleteView getStreamTopics request', () async { + test('TopicAutocompleteView fetches topics once for a channel', () async { final store = eg.store(); final connection = store.connection as FakeApiConnection; - connection.prepare(json: GetStreamTopicsResult( - topics: [eg.getStreamTopicsEntry(name: '')], - ).toJson()); - TopicAutocompleteView.init(store: store, streamId: 1000, - query: TopicAutocompleteQuery('foo')); - check(connection.lastRequest).isA() + final topic1 = eg.getChannelTopicsEntry(maxId: 20, name: 'server releases'); + final topic2 = eg.getChannelTopicsEntry(maxId: 10, name: 'mobile releases'); + + connection.prepare(json: GetChannelTopicsResult(topics: [topic1, topic2]).toJson()); + final view1 = TopicAutocompleteView.init(store: store, streamId: 1000, + query: TopicAutocompleteQuery('')); + bool done = false; + view1.addListener(() { done = true; }); + + check(connection.takeRequests()).last.isA() ..method.equals('GET') ..url.path.equals('/api/v1/users/me/1000/topics') ..url.queryParameters['allow_empty_topic_name'].equals('true'); + + await Future(() {}); + await Future(() {}); + check(done).isTrue(); + check(view1.results).deepEquals([isTopic(topic1.name), isTopic(topic2.name)]); + + view1.query = TopicAutocompleteQuery('server'); + check(connection.takeRequests()).isEmpty(); + await Future(() {}); + check(view1.results).single.which(isTopic(topic1.name)); + view1.dispose(); + + // No need to prepare a response as there will be no request made. + final view2 = TopicAutocompleteView.init(store: store, streamId: 1000, + query: TopicAutocompleteQuery('mobile')); + done = false; + view2.addListener(() { done = true; }); + + check(connection.takeRequests()).isEmpty(); + + await Future(() {}); + await Future(() {}); + check(done).isTrue(); + check(view2.results).single.which(isTopic(topic2.name)); }); group('TopicAutocompleteQuery.testTopic', () { diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 7a75647d9d..205ac58516 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -754,6 +754,18 @@ void main() { check(model).messages.length.equals(30); }); + test('in topic narrow: topics compared case-insensitively', () async { + final channel = eg.stream(); + await prepare(narrow: eg.topicNarrow(channel.streamId, 't')); + await prepareMessages(foundOldest: true, messages: + .generate(30, (i) => eg.streamMessage(stream: channel, topic: 't'))); + + check(model).messages.length.equals(30); + await store.addMessage(eg.streamMessage(stream: channel, topic: 'T')); + checkNotifiedOnce(); + check(model).messages.length.equals(31); + }); + test('while in mid-history', () async { final stream = eg.stream(); await prepare(narrow: ChannelNarrow(stream.streamId), stream: stream, diff --git a/test/model/narrow_test.dart b/test/model/narrow_test.dart index c62c56438c..47d8841ec6 100644 --- a/test/model/narrow_test.dart +++ b/test/model/narrow_test.dart @@ -63,6 +63,8 @@ void main() { eg.streamMessage(stream: stream, topic: 'topic2'))).isFalse(); check(narrow.containsMessage( eg.streamMessage(stream: stream, topic: 'topic'))).isTrue(); + check(narrow.containsMessage( + eg.streamMessage(stream: stream, topic: 'ToPiC'))).isTrue(); check(narrow.containsMessage( eg.dmOutboxMessage(from: eg.selfUser, to: [eg.otherUser]))).isFalse(); @@ -72,6 +74,8 @@ void main() { eg.streamOutboxMessage(stream: stream, topic: 'topic2'))).isFalse(); check(narrow.containsMessage( eg.streamOutboxMessage(stream: stream, topic: 'topic'))).isTrue(); + check(narrow.containsMessage( + eg.streamOutboxMessage(stream: stream, topic: 'ToPiC'))).isTrue(); }); }); diff --git a/test/model/topics_test.dart b/test/model/topics_test.dart new file mode 100644 index 0000000000..d3da89d3cb --- /dev/null +++ b/test/model/topics_test.dart @@ -0,0 +1,465 @@ +import 'package:checks/checks.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/route/channels.dart'; +import 'package:zulip/model/store.dart'; +import 'package:zulip/model/topics.dart'; + +import '../api/fake_api.dart'; +import '../api/route/route_checks.dart'; +import '../example_data.dart' as eg; +import '../fake_async.dart'; +import '../stdlib_checks.dart'; +import 'test_store.dart'; + +void main() { + late PerAccountStore store; + late Topics model; + late FakeApiConnection connection; + late int notifiedCount; + + void checkNotified({required int count}) { + check(notifiedCount).equals(count); + notifiedCount = 0; + } + void checkNotNotified() => checkNotified(count: 0); + void checkNotifiedOnce() => checkNotified(count: 1); + + Condition isChannelTopicsRequest(int channelId) { + return (it) => it.isA() + ..method.equals('GET') + ..url.path.equals('/api/v1/users/me/$channelId/topics') + ..url.queryParameters['allow_empty_topic_name'].equals('true'); + } + + Condition isChannelTopicsEntry(String topic, int maxId) { + return (it) => it.isA() + ..maxId.equals(maxId) + ..name.equals(eg.t(topic)); + } + + final channel = eg.stream(); + final otherChannel = eg.stream(); + + Future prepare({ + List? topics, + }) async { + store = eg.store(); + + notifiedCount = 0; + model = store.topics..addListener(() => notifiedCount++); + + connection = store.connection as FakeApiConnection; + if (topics != null) { + connection.prepare(json: GetChannelTopicsResult(topics: topics).toJson()); + await model.fetchChannelTopics(channel.streamId); + check(connection.takeRequests()).single.which(isChannelTopicsRequest(channel.streamId)); + } + } + + test('fetch topics -> update data, try fetching again -> no request sent', () async { + await prepare(topics: [ + eg.getChannelTopicsEntry(name: 'foo', maxId: 100), + ]); + check(model).getChannelTopics(channel.streamId).isNotNull() + .single.which(isChannelTopicsEntry('foo', 100)); + + // No need to prepare a response as there will be no request made. + await model.fetchChannelTopics(channel.streamId); + check(connection.takeRequests()).isEmpty(); + check(model).getChannelTopics(channel.streamId).isNotNull() + .single.which(isChannelTopicsEntry('foo', 100)); + }); + + test('fetchChannelTopics prevents concurrent requests for the same channel', () => awaitFakeAsync((async) async { + await prepare(); + + connection.prepare(delay: Duration(seconds: 3), + json: GetChannelTopicsResult(topics: [ + eg.getChannelTopicsEntry(name: 'foo', maxId: 100), + ]).toJson()); + final future1 = model.fetchChannelTopics(channel.streamId); + check(connection.takeRequests()).single.which(isChannelTopicsRequest(channel.streamId)); + check(model).getChannelTopics(channel.streamId).isNull(); + + // No need to prepare a response as there will be no request made. + final future2 = model.fetchChannelTopics(channel.streamId); + check(connection.takeRequests()).isEmpty(); + check(model).getChannelTopics(channel.streamId).isNull(); + + await future2; + check(model).getChannelTopics(channel.streamId).isNull(); + + async.elapse(Duration(seconds: 3)); + await future1; + check(model).getChannelTopics(channel.streamId).isNotNull() + .single.which(isChannelTopicsEntry('foo', 100)); + })); + + test('getChannelTopics sorts descending by maxId', () async { + await prepare(topics: [ + eg.getChannelTopicsEntry(name: 'bar', maxId: 200), + eg.getChannelTopicsEntry(name: 'foo', maxId: 100), + eg.getChannelTopicsEntry(name: 'baz', maxId: 300), + ]); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('baz', 300), + isChannelTopicsEntry('bar', 200), + isChannelTopicsEntry('foo', 100), + ]); + + await store.addMessage(eg.streamMessage( + id: 301, stream: channel, topic: 'foo')); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('foo', 301), + isChannelTopicsEntry('baz', 300), + isChannelTopicsEntry('bar', 200), + ]); + }); + + group('handleMessageEvent', () { + test('new message in fetched channel', () async { + await prepare(topics: [ + eg.getChannelTopicsEntry(name: 'old topic', maxId: 1), + ]); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('old topic', 1), + ]); + + await store.addMessage(eg.streamMessage(id: 2, stream: channel, topic: 'new topic')); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('new topic', 2), + isChannelTopicsEntry('old topic', 1), + ]); + checkNotifiedOnce(); + + await store.addMessage(eg.streamMessage(id: 3, stream: channel, topic: 'old topic')); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('old topic', 3), + isChannelTopicsEntry('new topic', 2), + ]); + checkNotifiedOnce(); + }); + + test('new message in channel not fetched before', () async { + await prepare(topics: []); + check(model).getChannelTopics(otherChannel.streamId).isNull(); + + await store.addMessage( + eg.streamMessage(id: 2, stream: otherChannel, topic: 'new topic')); + check(model).getChannelTopics(otherChannel.streamId).isNull(); + checkNotNotified(); + }); + + test('new message with equal or lower message ID', () async { + await prepare(topics: [ + eg.getChannelTopicsEntry(name: 'topic', maxId: 2), + ]); + + await store.addMessage( + eg.streamMessage(id: 2, stream: channel, topic: 'topic')); + check(model).getChannelTopics(channel.streamId).isNotNull() + .single.which(isChannelTopicsEntry('topic', 2)); + checkNotNotified(); + + await store.addMessage( + eg.streamMessage(id: 1, stream: channel, topic: 'topic')); + check(model).getChannelTopics(channel.streamId).isNotNull() + .single.which(isChannelTopicsEntry('topic', 2)); + checkNotNotified(); + }); + + test('ignore DM messages', () async { + await prepare(topics: []); + + await store.addMessage(eg.dmMessage(from: eg.selfUser, to: [eg.otherUser])); + checkNotNotified(); + }); + }); + + group('handleUpdateMessageEvent', () { + Future prepareWithMessages(List messages, { + required List> expectedTopics, + }) async { + await prepare(topics: []); + assert(messages.isNotEmpty); + assert(messages.every((m) => m.streamId == channel.streamId)); + await store.addMessages(messages); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals(expectedTopics); + checkNotified(count: messages.length); + } + + test('ignore content-only update', () async { + final message = eg.streamMessage(id: 123, stream: channel, topic: 'foo'); + await prepareWithMessages([message], expectedTopics: [ + isChannelTopicsEntry('foo', 123), + ]); + + await store.handleEvent(eg.updateMessageEditEvent(message)); + checkNotNotified(); + }); + + group('PropagateMode.changeAll', () { + test('topic moved to another channel with no previously fetched topics', () async { + final messagesToMove = List.generate(10, (i) => + eg.streamMessage(id: 100 + i, stream: channel, topic: 'foo')); + await prepareWithMessages(messagesToMove, expectedTopics: [ + isChannelTopicsEntry('foo', 109), + ]); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newStreamId: otherChannel.streamId, + propagateMode: .changeAll)); + check(model).getChannelTopics(channel.streamId).isNotNull().isEmpty(); + check(model).getChannelTopics(otherChannel.streamId).isNull(); + checkNotifiedOnce(); + }); + + test('topic moved to new topic in another channel', () async { + final messagesToMove = List.generate(10, (i) => + eg.streamMessage(id: 100 + i, stream: channel, topic: 'foo')); + await prepareWithMessages(messagesToMove, expectedTopics: [ + isChannelTopicsEntry('foo', 109), + ]); + + // Make sure that topics in otherChannel have been fetched. + connection.prepare(json: GetChannelTopicsResult(topics: [ + eg.getChannelTopicsEntry(name: 'foo', maxId: 1), + ]).toJson()); + await model.fetchChannelTopics(otherChannel.streamId); + check(model).getChannelTopics(otherChannel.streamId).isNotNull() + .single.which(isChannelTopicsEntry('foo', 1)); + checkNotNotified(); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newStreamId: otherChannel.streamId, + newTopicStr: 'bar', + propagateMode: .changeAll)); + check(model).getChannelTopics(channel.streamId).isNotNull().isEmpty(); + check(model).getChannelTopics(otherChannel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('bar', 109), + isChannelTopicsEntry('foo', 1), + ]); + checkNotifiedOnce(); + }); + + test('topic moved to existing topic in another channel', () async { + final messagesToMove = List.generate(10, (i) => + eg.streamMessage(id: 100 + i, stream: channel, topic: 'foo')); + await prepareWithMessages(messagesToMove, expectedTopics: [ + isChannelTopicsEntry('foo', 109), + ]); + + // Make sure that topics in otherChannel have been fetched. + connection.prepare(json: GetChannelTopicsResult(topics: [ + eg.getChannelTopicsEntry(name: 'foo', maxId: 1), + ]).toJson()); + await model.fetchChannelTopics(otherChannel.streamId); + check(model).getChannelTopics(otherChannel.streamId).isNotNull() + .single.which(isChannelTopicsEntry('foo', 1)); + checkNotNotified(); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newStreamId: otherChannel.streamId, + propagateMode: .changeAll)); + check(model).getChannelTopics(channel.streamId).isNotNull().isEmpty(); + check(model).getChannelTopics(otherChannel.streamId).isNotNull() + .single.which(isChannelTopicsEntry('foo', 109)); + checkNotifiedOnce(); + }); + + test('topic moved to new topic in the same channel', () async { + final messagesToMove = List.generate(10, (i) => + eg.streamMessage(id: 100 + i, stream: channel, topic: 'foo')); + await prepareWithMessages(messagesToMove, expectedTopics: [ + isChannelTopicsEntry('foo', 109), + ]); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newTopicStr: 'bar', + propagateMode: .changeAll)); + check(model).getChannelTopics(channel.streamId).isNotNull() + .single.which(isChannelTopicsEntry('bar', 109)); + checkNotifiedOnce(); + }); + + test('topic moved to existing topic in the same channel', () async { + final messagesToMove = List.generate(10, (i) => + eg.streamMessage(id: 100 + i, stream: channel, topic: 'foo')); + await prepareWithMessages([ + ...messagesToMove, + eg.streamMessage(id: 1, stream: channel, topic: 'bar'), + ], expectedTopics: [ + isChannelTopicsEntry('foo', 109), + isChannelTopicsEntry('bar', 1), + ]); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newTopicStr: 'bar', + propagateMode: .changeAll)); + check(model).getChannelTopics(channel.streamId).isNotNull() + .single.which(isChannelTopicsEntry('bar', 109)); + checkNotifiedOnce(); + }); + }); + + group('PropagateMode.changeOne', () { + test('message moved to new topic', () async { + final messageToMove = + eg.streamMessage(id: 101, stream: channel, topic: 'foo'); + await prepareWithMessages([ + messageToMove, + ], expectedTopics: [ + isChannelTopicsEntry('foo', 101), + ]); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: [messageToMove], + newTopicStr: 'bar', + propagateMode: .changeOne)); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('foo', 101), + isChannelTopicsEntry('bar', 101), + ]); + checkNotifiedOnce(); + }); + + test('message moved to existing topic; moved message ID < maxId', () async { + final messageToMove = + eg.streamMessage(id: 100, stream: channel, topic: 'foo'); + await prepareWithMessages([ + messageToMove, + eg.streamMessage(id: 999, stream: channel, topic: 'bar'), + ], expectedTopics: [ + isChannelTopicsEntry('bar', 999), + isChannelTopicsEntry('foo', 100), + ]); + + // Message with ID 100 moved from "foo" to "bar", whose maxId was 999. + // We expect no updates to "bar"'s maxId, since the moved message + // has a lower ID. + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: [messageToMove], + newTopicStr: 'bar', + propagateMode: .changeOne)); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('bar', 999), + isChannelTopicsEntry('foo', 100), + ]); + checkNotNotified(); + }); + + test('message moved to existing topic; moved message ID > maxId', () async { + final messageToMove = + eg.streamMessage(id: 999, stream: channel, topic: 'foo'); + await prepareWithMessages([ + messageToMove, + eg.streamMessage(id: 100, stream: channel, topic: 'bar'), + ], expectedTopics: [ + isChannelTopicsEntry('foo', 999), + isChannelTopicsEntry('bar', 100), + ]); + + // Message with ID 999 moved from "foo" to "bar", whose maxId was 100. + // We expect an update to "bar"'s maxId, since the moved message has + // a higher ID. + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: [messageToMove], + newTopicStr: 'bar', + propagateMode: .changeOne)); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('foo', 999), + isChannelTopicsEntry('bar', 999), + ]); + checkNotifiedOnce(); + }); + }); + + group('PropagateMode.changeLater', () { + test('messages moved to new topic', () async { + final messagesToMove = [ + eg.streamMessage(id: 99, stream: channel, topic: 'foo'), + eg.streamMessage(id: 100, stream: channel, topic: 'foo'), + ]; + await prepareWithMessages(messagesToMove, + expectedTopics: [ + isChannelTopicsEntry('foo', 100), + ]); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newTopicStr: 'bar', + propagateMode: .changeLater)); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('foo', 100), + isChannelTopicsEntry('bar', 100), + ]); + checkNotifiedOnce(); + }); + + test('messages moved to existing topic; moved messages max ID < maxId', () async { + final messagesToMove = [ + eg.streamMessage(id: 99, stream: channel, topic: 'foo'), + eg.streamMessage(id: 100, stream: channel, topic: 'foo'), + ]; + await prepareWithMessages([ + ...messagesToMove, + eg.streamMessage(id: 999, stream: channel, topic: 'bar'), + ], expectedTopics: [ + isChannelTopicsEntry('bar', 999), + isChannelTopicsEntry('foo', 100), + ]); + + // Messages with max ID 100 moved from "foo" to "bar", whose maxId + // was 999. We expect no updates to "bar"'s maxId, since the moved + // messages have a lower max ID. + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newTopicStr: 'bar', + propagateMode: .changeLater)); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('bar', 999), + isChannelTopicsEntry('foo', 100), + ]); + checkNotNotified(); + }); + + test('message moved to existing topic; moved messages max ID > maxId', () async { + final messagesToMove = [ + eg.streamMessage(id: 999, stream: channel, topic: 'foo'), + eg.streamMessage(id: 1000, stream: channel, topic: 'foo'), + ]; + await prepareWithMessages([ + ...messagesToMove, + eg.streamMessage(id: 100, stream: channel, topic: 'bar'), + ], expectedTopics: [ + isChannelTopicsEntry('foo', 1000), + isChannelTopicsEntry('bar', 100), + ]); + + // Messages with max ID 1000 moved from "foo" to "bar", whose maxId + // was 100. We expect an update to "bar"'s maxId, since the moved + // messages have a higher max ID. + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newTopicStr: 'bar', + propagateMode: .changeLater)); + check(model).getChannelTopics(channel.streamId).isNotNull().deepEquals([ + isChannelTopicsEntry('foo', 1000), + isChannelTopicsEntry('bar', 1000), + ]); + checkNotifiedOnce(); + }); + }); + }); +} + +extension TopicsChecks on Subject { + Subject?> getChannelTopics(int channelId) => has((x) => x.getChannelTopics(channelId), 'getChannelTopics'); +} diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index c6910473dd..6a6cbb0c8b 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -222,12 +222,14 @@ void main() { channel ??= someChannel; connection.prepare(json: eg.newestGetMessagesResult( - foundOldest: true, messages: []).toJson()); - if (narrow case ChannelNarrow()) { - // We auto-focus the topic input when there are no messages; - // this is for topic autocomplete. - connection.prepare(json: GetStreamTopicsResult(topics: []).toJson()); - } + foundOldest: true, + // Include one message so that we don't auto-focus the topic input, + // which would trigger a topic-list fetch for topic autocomplete. + // That's helpful for the test that opens the topic-list page. + // With the topic-list fetch deferred until that page is opened, + // the test can prepare the fetch response as it chooses. + messages: [eg.streamMessage(stream: channel)], + ).toJson()); await tester.pumpWidget(TestZulipApp( accountId: eg.selfAccount.id, child: MessageListPage( @@ -261,7 +263,7 @@ void main() { streamId ??= someChannel.streamId; final transitionDurationObserver = TransitionDurationObserver(); - connection.prepare(json: GetStreamTopicsResult(topics: []).toJson()); + connection.prepare(json: GetChannelTopicsResult(topics: []).toJson()); await tester.pumpWidget(TestZulipApp( navigatorObservers: [transitionDurationObserver], accountId: eg.selfAccount.id, @@ -510,8 +512,8 @@ void main() { await showFromMsglistAppBar(tester, narrow: ChannelNarrow(someChannel.streamId)); - connection.prepare(json: GetStreamTopicsResult(topics: [ - eg.getStreamTopicsEntry(name: 'some topic foo'), + connection.prepare(json: GetChannelTopicsResult(topics: [ + eg.getChannelTopicsEntry(name: 'some topic foo'), ]).toJson()); await tester.tap(findButtonForLabel('List of topics')); await tester.pumpAndSettle(); @@ -565,7 +567,7 @@ void main() { connection.prepare(json: eg.newestGetMessagesResult( foundOldest: true, messages: []).toJson()); // for topic autocomplete - connection.prepare(json: GetStreamTopicsResult(topics: []).toJson()); + connection.prepare(json: GetChannelTopicsResult(topics: []).toJson()); await tapButtonAndPump(tester); await transitionDurationObserver.pumpPastTransition(tester); @@ -1685,7 +1687,7 @@ void main() { // Ensure channel-topics are loaded before testing quote & reply behavior connection.prepare(body: - jsonEncode(GetStreamTopicsResult(topics: [eg.getStreamTopicsEntry()]).toJson())); + jsonEncode(GetChannelTopicsResult(topics: [eg.getChannelTopicsEntry()]).toJson())); final topicController = composeBoxController.topic; topicController.value = TextEditingValue(text: 'other topic'); @@ -1710,7 +1712,7 @@ void main() { // Ensure channel-topics are loaded before testing quote & reply behavior connection.prepare(body: - jsonEncode(GetStreamTopicsResult(topics: [eg.getStreamTopicsEntry()]).toJson())); + jsonEncode(GetChannelTopicsResult(topics: [eg.getChannelTopicsEntry()]).toJson())); final topicController = composeBoxController.topic; topicController.value = const TextEditingValue(text: ''); diff --git a/test/widgets/autocomplete_test.dart b/test/widgets/autocomplete_test.dart index a6d076a497..d696c79220 100644 --- a/test/widgets/autocomplete_test.dart +++ b/test/widgets/autocomplete_test.dart @@ -106,7 +106,7 @@ Future setupToComposeInput(WidgetTester tester, { /// /// Returns a [Finder] for the topic input's [TextField]. Future setupToTopicInput(WidgetTester tester, { - required List topics, + required List topics, String? realmEmptyTopicDisplayName, }) async { addTearDown(testBinding.reset); @@ -133,7 +133,7 @@ Future setupToTopicInput(WidgetTester tester, { child: MessageListPage(initNarrow: ChannelNarrow(stream.streamId)))); await tester.pumpAndSettle(); - connection.prepare(json: GetStreamTopicsResult(topics: topics).toJson()); + connection.prepare(json: GetChannelTopicsResult(topics: topics).toJson()); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; final finder = find.byWidgetPredicate((widget) => widget is TextField && widget.decoration?.hintText == zulipLocalizations.composeBoxTopicHintText); @@ -507,9 +507,9 @@ void main() { group('TopicAutocomplete', () { testWidgets('options appear, disappear, and change correctly', (WidgetTester tester) async { - final topic1 = eg.getStreamTopicsEntry(maxId: 1, name: 'Topic one'); - final topic2 = eg.getStreamTopicsEntry(maxId: 2, name: 'Topic two'); - final topic3 = eg.getStreamTopicsEntry(maxId: 3, name: 'Topic three'); + final topic1 = eg.getChannelTopicsEntry(maxId: 1, name: 'Topic one'); + final topic2 = eg.getChannelTopicsEntry(maxId: 2, name: 'Topic two'); + final topic3 = eg.getChannelTopicsEntry(maxId: 3, name: 'Topic three'); final topicInputFinder = await setupToTopicInput(tester, topics: [topic1, topic2, topic3]); // Options are filtered correctly for query @@ -545,7 +545,7 @@ void main() { // to suffice to set it up; the controller value after the pump still // has empty composing region, so there's nothing to check after tap.) - final topic = eg.getStreamTopicsEntry(name: 'some topic'); + final topic = eg.getChannelTopicsEntry(name: 'some topic'); final topicInputFinder = await setupToTopicInput(tester, topics: [topic]); final controller = tester.widget(topicInputFinder).controller!; @@ -575,7 +575,7 @@ void main() { }); testWidgets('display realmEmptyTopicDisplayName for empty topic', (tester) async { - final topic = eg.getStreamTopicsEntry(name: ''); + final topic = eg.getChannelTopicsEntry(name: ''); final topicInputFinder = await setupToTopicInput(tester, topics: [topic], realmEmptyTopicDisplayName: 'some display name'); @@ -588,7 +588,7 @@ void main() { }); testWidgets('match realmEmptyTopicDisplayName in autocomplete', (tester) async { - final topic = eg.getStreamTopicsEntry(name: ''); + final topic = eg.getChannelTopicsEntry(name: ''); final topicInputFinder = await setupToTopicInput(tester, topics: [topic], realmEmptyTopicDisplayName: 'general chat'); @@ -601,7 +601,7 @@ void main() { }); testWidgets('autocomplete to realmEmptyTopicDisplayName sets topic to empty string', (tester) async { - final topic = eg.getStreamTopicsEntry(name: ''); + final topic = eg.getChannelTopicsEntry(name: ''); final topicInputFinder = await setupToTopicInput(tester, topics: [topic], realmEmptyTopicDisplayName: 'general chat'); final controller = tester.widget(topicInputFinder).controller!; diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index a847120691..9858896ae6 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -75,7 +75,7 @@ void main() { if (narrow is ChannelNarrow) { // By default, bypass the complexity where the topic input is autofocused // on an empty fetch, by making the fetch not empty. (In particular that - // complexity includes a getStreamTopics fetch for topic autocomplete.) + // complexity includes a getChannelTopics fetch for topic autocomplete.) messages ??= [eg.streamMessage(stream: channel)]; } } @@ -102,8 +102,8 @@ void main() { connection.prepare(json: eg.newestGetMessagesResult(foundOldest: true, messages: messages).toJson()); if (narrow is ChannelNarrow && messages.isEmpty) { - // The topic input will autofocus, triggering a getStreamTopics request. - connection.prepare(json: GetStreamTopicsResult(topics: []).toJson()); + // The topic input will autofocus, triggering a getChannelTopics request. + connection.prepare(json: GetChannelTopicsResult(topics: []).toJson()); } await tester.pumpWidget(TestZulipApp(accountId: selfAccount.id, child: MessageListPage(initNarrow: narrow))); @@ -126,7 +126,7 @@ void main() { required String topic, }) async { connection.prepare(body: - jsonEncode(GetStreamTopicsResult(topics: [eg.getStreamTopicsEntry()]).toJson())); + jsonEncode(GetChannelTopicsResult(topics: [eg.getChannelTopicsEntry()]).toJson())); await tester.enterText(topicInputFinder, topic); check(connection.takeRequests()).single ..method.equals('GET') @@ -1797,7 +1797,7 @@ void main() { otherUsers: otherUsers); if (narrow is ChannelNarrow) { - connection.prepare(json: GetStreamTopicsResult(topics: []).toJson()); + connection.prepare(json: GetChannelTopicsResult(topics: []).toJson()); await enterTopic(tester, narrow: narrow, topic: topic); } await enterContent(tester, failedMessageContent); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 47d46a6bbe..3688e4c5bc 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -302,8 +302,8 @@ void main() { subscriptions: [eg.subscription(channel)], messages: [eg.streamMessage(stream: channel, topic: 'topic foo')]); - connection.prepare(json: GetStreamTopicsResult(topics: [ - eg.getStreamTopicsEntry(name: 'topic foo'), + connection.prepare(json: GetChannelTopicsResult(topics: [ + eg.getChannelTopicsEntry(name: 'topic foo'), ]).toJson()); await tester.tap(find.byIcon(ZulipIcons.topics)); await tester.pump(); // tap the button @@ -337,8 +337,8 @@ void main() { subscriptions: [eg.subscription(channel)], messages: [eg.streamMessage(stream: channel, topic: 'topic foo')]); - connection.prepare(json: GetStreamTopicsResult(topics: [ - eg.getStreamTopicsEntry(name: 'topic foo'), + connection.prepare(json: GetChannelTopicsResult(topics: [ + eg.getChannelTopicsEntry(name: 'topic foo'), ]).toJson()); await tester.tap(find.byIcon(ZulipIcons.topics)); await tester.pump(); // tap the button @@ -392,7 +392,7 @@ void main() { skipPumpAndSettle: true); // The topic input is autofocused, triggering topic autocomplete. - connection.prepare(json: GetStreamTopicsResult(topics: []).toJson()); + connection.prepare(json: GetChannelTopicsResult(topics: []).toJson()); await tester.pumpAndSettle(); check(findPlaceholder).findsOne(); @@ -406,7 +406,7 @@ void main() { skipPumpAndSettle: true); // The topic input is autofocused, triggering topic autocomplete. - connection.prepare(json: GetStreamTopicsResult(topics: []).toJson()); + connection.prepare(json: GetChannelTopicsResult(topics: []).toJson()); await tester.pumpAndSettle(); check(store.selfHasContentAccess(channel)).isFalse(); @@ -448,7 +448,7 @@ void main() { skipPumpAndSettle: true); // The topic input is autofocused, triggering topic autocomplete. - connection.prepare(json: GetStreamTopicsResult(topics: []).toJson()); + connection.prepare(json: GetChannelTopicsResult(topics: []).toJson()); await tester.pumpAndSettle(); check(store.selfHasContentAccess(channel)).isFalse(); diff --git a/test/widgets/topic_list_test.dart b/test/widgets/topic_list_test.dart index 9f3b8d0dfa..488116be90 100644 --- a/test/widgets/topic_list_test.dart +++ b/test/widgets/topic_list_test.dart @@ -28,7 +28,7 @@ void main() { Future prepare(WidgetTester tester, { ZulipStream? channel, - List? topics, + List? topics, List userTopics = const [], List? messages, }) async { @@ -45,11 +45,11 @@ void main() { await store.setUserTopic( channel, userTopic.topicName.apiName, userTopic.visibilityPolicy); } - topics ??= [eg.getStreamTopicsEntry()]; + topics ??= [eg.getChannelTopicsEntry()]; messages ??= [eg.streamMessage(stream: channel, topic: topics.first.name.apiName)]; await store.addMessages(messages); - connection.prepare(json: GetStreamTopicsResult(topics: topics).toJson()); + connection.prepare(json: GetChannelTopicsResult(topics: topics).toJson()); await tester.pumpWidget(TestZulipApp( accountId: eg.selfAccount.id, child: TopicListPage(streamId: channel.streamId))); @@ -69,7 +69,7 @@ void main() { final channel = eg.stream(); (store.connection as FakeApiConnection).prepare( - json: GetStreamTopicsResult(topics: []).toJson()); + json: GetChannelTopicsResult(topics: []).toJson()); await tester.pumpWidget(TestZulipApp( accountId: eg.selfAccount.id, child: TopicListPage(streamId: channel.streamId))); @@ -111,7 +111,7 @@ void main() { final channel = eg.stream(); (store.connection as FakeApiConnection).prepare( - json: GetStreamTopicsResult(topics: []).toJson(), + json: GetChannelTopicsResult(topics: []).toJson(), delay: Duration(seconds: 1), ); await tester.pumpWidget(TestZulipApp( @@ -130,7 +130,7 @@ void main() { check(find.text('There are no topics here yet.')).findsOne(); }); - testWidgets('fetch again when navigating away and back', (tester) async { + testWidgets("don't fetch again when navigating away and back", (tester) async { addTearDown(testBinding.reset); await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); @@ -146,25 +146,30 @@ void main() { await tester.pump(); // Tap "TOPICS" button navigating to the topic-list page… - connection.prepare(json: GetStreamTopicsResult( - topics: [eg.getStreamTopicsEntry(name: 'topic A')]).toJson()); + connection.prepare(json: GetChannelTopicsResult( + topics: [eg.getChannelTopicsEntry(name: 'topic A')]).toJson()); await tester.tap(find.byIcon(ZulipIcons.topics)); await tester.pump(); await tester.pump(Duration.zero); + check(connection.takeRequests()) + ..length.equals(2) // one for the messages request, another for the topics + ..last.which((last) => last + .isA() + ..method.equals('GET') + ..url.path.equals('/api/v1/users/me/${channel.streamId}/topics')); check(find.text('topic A')).findsOne(); // … go back to the message list page… await tester.pageBack(); await tester.pump(); - // … then back to the topic-list page, expecting to fetch again. - connection.prepare(json: GetStreamTopicsResult( - topics: [eg.getStreamTopicsEntry(name: 'topic B')]).toJson()); + // … then back to the topic-list page, expecting not to fetch again but + // use existing data which is kept up-to-date anyway. await tester.tap(find.byIcon(ZulipIcons.topics)); await tester.pump(); await tester.pump(Duration.zero); - check(find.text('topic A')).findsNothing(); - check(find.text('topic B')).findsOne(); + check(connection.takeRequests()).isEmpty(); + check(find.text('topic A')).findsOne(); }); Finder topicItemFinder = find.descendant( @@ -175,10 +180,22 @@ void main() { of: topicItemFinder.at(index), matching: finder); + testWidgets('sort topics by maxId', (tester) async { + await prepare(tester, topics: [ + eg.getChannelTopicsEntry(name: 'A', maxId: 3), + eg.getChannelTopicsEntry(name: 'B', maxId: 2), + eg.getChannelTopicsEntry(name: 'C', maxId: 4), + ]); + + check(findInTopicItemAt(0, find.text('C'))).findsOne(); + check(findInTopicItemAt(1, find.text('A'))).findsOne(); + check(findInTopicItemAt(2, find.text('B'))).findsOne(); + }); + testWidgets('show topic action sheet', (tester) async { final channel = eg.stream(); await prepare(tester, channel: channel, - topics: [eg.getStreamTopicsEntry(name: 'topic foo')]); + topics: [eg.getChannelTopicsEntry(name: 'topic foo')]); await tester.longPress(topicItemFinder); await tester.pump(Duration(milliseconds: 150)); // bottom-sheet animation @@ -196,24 +213,76 @@ void main() { }); }); - testWidgets('sort topics by maxId', (tester) async { - await prepare(tester, topics: [ - eg.getStreamTopicsEntry(name: 'A', maxId: 3), - eg.getStreamTopicsEntry(name: 'B', maxId: 2), - eg.getStreamTopicsEntry(name: 'C', maxId: 4), - ]); + testWidgets('topic action sheet before and after moves', (tester) async { + final channel = eg.stream(); + final message = eg.streamMessage(id: 100, stream: channel, topic: 'foo'); + await prepare(tester, channel: channel, + topics: [eg.getChannelTopicsEntry(name: 'foo', maxId: 100)], + messages: [message]); + check(topicItemFinder).findsOne(); - check(findInTopicItemAt(0, find.text('C'))).findsOne(); - check(findInTopicItemAt(1, find.text('A'))).findsOne(); - check(findInTopicItemAt(2, find.text('B'))).findsOne(); + // Before the move, "foo"'s maxId is known to be accurate. This makes + // topic actions that require `someMessageIdInTopic` available. + await tester.longPress(find.text('foo')); + await tester.pump(Duration(milliseconds: 150)); // bottom-sheet animation + check(find.text('Mark as resolved')).findsOne(); + await tester.tap(find.text('Cancel')); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: [message], + newTopicStr: 'bar')); + await tester.pump(); + check(topicItemFinder).findsExactly(2); + + // After the move, the message with maxId moved away from "foo". The topic + // actions that require `someMessageIdInTopic` are no longer available. + await tester.longPress(find.text('foo')); + await tester.pump(Duration(milliseconds: 150)); // bottom-sheet animation + check(find.text('Mark as resolved')).findsNothing(); + await tester.tap(find.text('Cancel')); + await tester.pump(); + + // Topic actions that require `someMessageIdInTopic` are available + // for "bar", the new topic that the message moved to. + await tester.longPress(find.text('bar')); + await tester.pump(Duration(milliseconds: 150)); // bottom-sheet animation + check(find.text('Mark as resolved')).findsOne(); + }); + + // event handling is more thoroughly tested in test/model/channel_test.dart + testWidgets('smoke resolve topic from topic action sheet', (tester) async { + final channel = eg.stream(); + final messages = List.generate(10, (i) => + eg.streamMessage(id: 100 + i, stream: channel, topic: 'foo')); + + await prepare(tester, channel: channel, + topics: [eg.getChannelTopicsEntry(maxId: 109, name: 'foo')], + messages: messages); + await tester.longPress(topicItemFinder); + await tester.pump(Duration(milliseconds: 150)); // bottom-sheet animation + check(findInTopicItemAt(0, find.byIcon(ZulipIcons.check))).findsNothing(); + check(findInTopicItemAt(0, find.text('foo'))).findsOne(); + + connection.prepare(json: {}); + await tester.tap(find.text('Mark as resolved')); + await tester.pump(); + await tester.pump(Duration.zero); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: messages, + newTopic: eg.t('foo').resolve(), + propagateMode: .changeAll)); + await tester.pump(); + check(findInTopicItemAt(0, find.byIcon(ZulipIcons.check))).findsOne(); + check(findInTopicItemAt(0, find.text('foo'))).findsOne(); }); testWidgets('resolved and unresolved topics', (tester) async { final resolvedTopic = TopicName('resolved').resolve(); final unresolvedTopic = TopicName('unresolved'); await prepare(tester, topics: [ - eg.getStreamTopicsEntry(maxId: 2, name: resolvedTopic.apiName), - eg.getStreamTopicsEntry(maxId: 1, name: unresolvedTopic.apiName), + eg.getChannelTopicsEntry(maxId: 2, name: resolvedTopic.apiName), + eg.getChannelTopicsEntry(maxId: 1, name: unresolvedTopic.apiName), ]); assert(resolvedTopic.displayName == '✔ resolved', resolvedTopic.displayName); @@ -230,7 +299,7 @@ void main() { testWidgets('handle empty topics', (tester) async { await prepare(tester, topics: [ - eg.getStreamTopicsEntry(name: ''), + eg.getChannelTopicsEntry(name: ''), ]); check(findInTopicItemAt(0, find.text(eg.defaultRealmEmptyTopicDisplayName))).findsOne(); @@ -241,8 +310,8 @@ void main() { final channel = eg.stream(); await prepare(tester, channel: channel, topics: [ - eg.getStreamTopicsEntry(maxId: 2, name: 'muted'), - eg.getStreamTopicsEntry(maxId: 1, name: 'non-muted'), + eg.getChannelTopicsEntry(maxId: 2, name: 'muted'), + eg.getChannelTopicsEntry(maxId: 1, name: 'non-muted'), ], userTopics: [ eg.userTopicItem(channel, 'muted', UserTopicVisibilityPolicy.muted), @@ -268,8 +337,8 @@ void main() { final channel = eg.stream(); await prepare(tester, channel: channel, topics: [ - eg.getStreamTopicsEntry(maxId: 2, name: 'not mentioned'), - eg.getStreamTopicsEntry(maxId: 1, name: 'mentioned'), + eg.getChannelTopicsEntry(maxId: 2, name: 'not mentioned'), + eg.getChannelTopicsEntry(maxId: 1, name: 'mentioned'), ], messages: [ eg.streamMessage(stream: channel, topic: 'not mentioned'), @@ -294,7 +363,7 @@ void main() { testWidgets('default', (tester) async { final channel = eg.stream(); await prepare(tester, channel: channel, - topics: [eg.getStreamTopicsEntry(name: 'topic')]); + topics: [eg.getChannelTopicsEntry(name: 'topic')]); check(find.descendant(of: topicItemFinder, matching: find.byType(Icons))).findsNothing(); @@ -303,7 +372,7 @@ void main() { testWidgets('muted', (tester) async { final channel = eg.stream(); await prepare(tester, channel: channel, - topics: [eg.getStreamTopicsEntry(name: 'topic')], + topics: [eg.getChannelTopicsEntry(name: 'topic')], userTopics: [ eg.userTopicItem(channel, 'topic', UserTopicVisibilityPolicy.muted), ]); @@ -314,7 +383,7 @@ void main() { testWidgets('unmuted', (tester) async { final channel = eg.stream(); await prepare(tester, channel: channel, - topics: [eg.getStreamTopicsEntry(name: 'topic')], + topics: [eg.getChannelTopicsEntry(name: 'topic')], userTopics: [ eg.userTopicItem(channel, 'topic', UserTopicVisibilityPolicy.unmuted), ]); @@ -325,7 +394,7 @@ void main() { testWidgets('followed', (tester) async { final channel = eg.stream(); await prepare(tester, channel: channel, - topics: [eg.getStreamTopicsEntry(name: 'topic')], + topics: [eg.getChannelTopicsEntry(name: 'topic')], userTopics: [ eg.userTopicItem(channel, 'topic', UserTopicVisibilityPolicy.followed), ]);