From 8f6bfd7b472acfbe007c4501b87c3b3584680fb7 Mon Sep 17 00:00:00 2001 From: Sahil Kumar Date: Thu, 17 Feb 2022 17:17:52 +0530 Subject: [PATCH] fix(llc): improve removeMessage logic. --- .../stream_chat/lib/src/client/channel.dart | 116 ++++++++---------- .../lib/src/client/retry_queue.dart | 2 +- 2 files changed, 51 insertions(+), 67 deletions(-) diff --git a/packages/stream_chat/lib/src/client/channel.dart b/packages/stream_chat/lib/src/client/channel.dart index 84f0321e..b5bad6cb 100644 --- a/packages/stream_chat/lib/src/client/channel.dart +++ b/packages/stream_chat/lib/src/client/channel.dart @@ -407,7 +407,7 @@ class Channel { if (index != -1) { final newAttachments = [...message!.attachments]..[index] = attachment; final updatedMessage = message!.copyWith(attachments: newAttachments); - state?.addMessage(updatedMessage); + state?.updateMessage(updatedMessage); // updating original message for next iteration message = message!.merge(updatedMessage); } @@ -512,7 +512,7 @@ class Channel { ).toList(), ); - state!.addMessage(message); + state!.updateMessage(message); try { if (message.attachments.any((it) => !it.uploadState.isSuccess)) { @@ -535,7 +535,7 @@ class Channel { type, skipPush: skipPush, ); - state!.addMessage(response.message); + state!.updateMessage(response.message); if (cooldown > 0) cooldownStartedAt = DateTime.now(); return response; } catch (e) { @@ -571,7 +571,7 @@ class Channel { ).toList(), ); - state?.addMessage(message); + state?.updateMessage(message); try { if (message.attachments.any((it) => !it.uploadState.isSuccess)) { @@ -594,7 +594,7 @@ class Channel { ownReactions: message.ownReactions, ); - state?.addMessage(m); + state?.updateMessage(m); return response; } catch (e) { @@ -602,7 +602,7 @@ class Channel { if (e.isRetriable) { state!._retryQueue.add([message]); } else { - state?.addMessage(originalMessage); + state?.updateMessage(originalMessage); } } rethrow; @@ -630,7 +630,7 @@ class Channel { ownReactions: message.ownReactions, ); - state?.addMessage(updatedMessage); + state?.updateMessage(updatedMessage); return response; } catch (e) { @@ -646,7 +646,7 @@ class Channel { // Directly deleting the local messages which are not yet sent to server if (message.status == MessageSendingStatus.sending || message.status == MessageSendingStatus.failed) { - state!.addMessage(message.copyWith( + state!.updateMessage(message.copyWith( type: 'deleted', status: MessageSendingStatus.sent, )); @@ -667,11 +667,11 @@ class Channel { deletedAt: message.deletedAt ?? DateTime.now(), ); - state?.addMessage(message); + state?.updateMessage(message); final response = await _client.deleteMessage(message.id, hard: hard); - state?.addMessage(message.copyWith(status: MessageSendingStatus.sent)); + state?.updateMessage(message.copyWith(status: MessageSendingStatus.sent)); return response; } catch (e) { @@ -860,7 +860,7 @@ class Channel { ownReactions: ownReactions, ); - state?.addMessage(newMessage); + state?.updateMessage(newMessage); try { final reactionResp = await _client.sendReaction( @@ -872,7 +872,7 @@ class Channel { return reactionResp; } catch (_) { // Reset the message if the update fails - state?.addMessage(message); + state?.updateMessage(message); rethrow; } } @@ -912,7 +912,7 @@ class Channel { ownReactions: ownReactions, ); - state?.addMessage(newMessage); + state?.updateMessage(newMessage); try { final deleteResponse = await _client.deleteReaction( @@ -922,7 +922,7 @@ class Channel { return deleteResponse; } catch (_) { // Reset the message if the update fails - state?.addMessage(message); + state?.updateMessage(message); rethrow; } } @@ -1079,11 +1079,11 @@ class Channel { // update the passed message with response message if (res.message != null) { - state!.addMessage(res.message!); + state!.updateMessage(res.message!); } else { // remove the passed message if response does // not contain message - state!.removeMessage(message, decreaseReplyCount: true); + state!.removeMessage(message); await _client.chatPersistenceClient?.deleteMessageById(messageId); } return res; @@ -1322,7 +1322,8 @@ class Channel { /// Remove the ban for the user with given [userID] in the channel. @Deprecated( - "Use 'unbanMember' instead. This method will be removed in v4.0.0") + "Use 'unbanMember' instead. This method will be removed in v4.0.0", + ) Future unbanUser(String userID) => unbanMember(userID); /// Remove the ban for the member with given [userID] in the channel. @@ -1694,7 +1695,9 @@ class ChannelClientState { void _listenReactionDeleted() { _subscriptions.add(_channel.on(EventType.reactionDeleted).listen((event) { final oldMessage = - messages.firstWhereOrNull((it) => it.id == event.message?.id); + messages.firstWhereOrNull((it) => it.id == event.message?.id) ?? + threads[event.message?.parentId] + ?.firstWhereOrNull((e) => e.id == event.message?.id); final reaction = event.reaction; final ownReactions = oldMessage?.ownReactions ?.whereNot((it) => @@ -1707,18 +1710,20 @@ class ChannelClientState { final message = event.message!.copyWith( ownReactions: ownReactions, ); - addMessage(message); + updateMessage(message); })); } void _listenReactions() { _subscriptions.add(_channel.on(EventType.reactionNew).listen((event) { final oldMessage = - messages.firstWhereOrNull((it) => it.id == event.message?.id); + messages.firstWhereOrNull((it) => it.id == event.message?.id) ?? + threads[event.message?.parentId] + ?.firstWhereOrNull((e) => e.id == event.message?.id); final message = event.message!.copyWith( ownReactions: oldMessage?.ownReactions, ); - addMessage(message); + updateMessage(message); })); } @@ -1730,12 +1735,13 @@ class ChannelClientState { ) .listen((event) { final oldMessage = - messages.firstWhereOrNull((it) => it.id == event.message?.id); - + messages.firstWhereOrNull((it) => it.id == event.message?.id) ?? + threads[event.message?.parentId] + ?.firstWhereOrNull((e) => e.id == event.message?.id); final message = event.message!.copyWith( ownReactions: oldMessage?.ownReactions, ); - addMessage(message); + updateMessage(message); if (message.pinned) { _channelState = _channelState.copyWith( @@ -1752,10 +1758,9 @@ class ChannelClientState { _subscriptions.add(_channel.on(EventType.messageDeleted).listen((event) { final message = event.message!; if (event.hardDelete == true) { - //do not decrease reply count here - it is done in _listenMessageUpdated - removeMessage(message, hardDelete: true); + removeMessage(message); } else { - addMessage(message); + updateMessage(message); } })); } @@ -1770,7 +1775,7 @@ class ChannelClientState { final message = event.message!; if (isUpToDate || (message.parentId != null && message.showInChannel != true)) { - addMessage(message); + updateMessage(message); } if (_countMessageAsUnread(message)) { @@ -1780,9 +1785,13 @@ class ChannelClientState { } /// Add a [message] to this [channelState]. - void addMessage(Message message) { + @Deprecated('Use updateMessage instead') + void addMessage(Message message) => updateMessage(message); + + /// Updates the [message] in the state if it exists. Adds it otherwise. + void updateMessage(Message message) { if (message.parentId == null || message.showInChannel == true) { - final newMessages = List.from(_channelState.messages); + final newMessages = [...messages]; final oldIndex = newMessages.indexWhere((m) => m.id == message.id); if (oldIndex != -1) { Message? m; @@ -1811,47 +1820,22 @@ class ChannelClientState { } /// Remove a [message] from this [channelState]. - void removeMessage( - Message message, { - bool hardDelete = false, - bool decreaseReplyCount = false, - }) { + void removeMessage(Message message) { final parentId = message.parentId; - // i.e. it's a thread message - // 1. Remove the thread message - // 2. Reduce total reply count of parent message + // i.e. it's a thread message, Remove it if (parentId != null) { - if (decreaseReplyCount) { - final allMessages = [...messages]; - final parentMessage = allMessages.firstWhereOrNull( - (it) => it.id == parentId, - ); - - // return if message not available in the memory - if (parentMessage == null) return; - final replyCount = parentMessage.replyCount; - // return if reply count is null or zero - if (replyCount == null || replyCount == 0) return; - - addMessage(parentMessage.copyWith(replyCount: replyCount - 1)); - } - updateThreadInfo( + final threadMessages = [...threads[parentId]!]; + return updateThreadInfo( parentId, - threads[parentId]! - ..removeWhere( - (e) => e.id == message.id, - ), + threadMessages..removeWhere((e) => e.id == message.id), ); - } else { - // Remove regular message - final allMessages = [...messages]; - if (hardDelete) { - allMessages.removeWhere((e) => e.id == message.id); - _channelState = _channelState.copyWith(messages: allMessages); - } else if (allMessages.remove(message)) { - _channelState = _channelState.copyWith(messages: allMessages); - } } + + // Remove regular message + final allMessages = [...messages]; + _channelState = _channelState.copyWith( + messages: allMessages..removeWhere((e) => e.id == message.id), + ); } void _listenReadEvents() { diff --git a/packages/stream_chat/lib/src/client/retry_queue.dart b/packages/stream_chat/lib/src/client/retry_queue.dart index 17b019a5..5dfa9aa3 100644 --- a/packages/stream_chat/lib/src/client/retry_queue.dart +++ b/packages/stream_chat/lib/src/client/retry_queue.dart @@ -158,7 +158,7 @@ class RetryQueue { : message.status == MessageSendingStatus.updating ? MessageSendingStatus.failed_update : MessageSendingStatus.failed_delete; - channel.state?.addMessage(message.copyWith(status: newStatus)); + channel.state?.updateMessage(message.copyWith(status: newStatus)); } Future _retryMessage(Message message) async {