Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/stream_chat_flutter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
🐛 Fixed

- Fixed a use-after-dispose race condition in `StreamAttachmentPickerController`, `StreamAudioRecorderController`, and `StreamAudioPlaylistController`: async methods could write `value` after `dispose()`, causing a `notifyListeners()` assertion throw in debug mode. All three now use the `DisposeAwareValueNotifier` mixin from `stream_chat_flutter_core`.
- Fixed last-message preview flicker during channel-state reloads.

✅ Added

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,18 @@ class _ChannelLastMessageWithStatusState extends State<_ChannelLastMessageWithSt

// Find the last valid message.
final message = messages.lastWhereOrNull(_defaultLastMessagePredicate);
final latestLastMessage = [message, _currentLastMessage].latest;
// `_currentLastMessage` holds the most recent message seen while the
// channel has the latest messages (isUpToDate).
// While isUpToDate is false (e.g. Channel.query(idAround:) truncates
// state mid-load), fall back to it so the preview shows the actual
// latest message.
final Message? latestLastMessage;
if (channelState.isUpToDate) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move this after we calculate latestLastMessage? and set the _currentLastMessage to latestLastMessage?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an interesting edge-case which kinda prevents us doing this: While the _currentLastMessage will be properly populated in both cases, there is a subtle difference in the calculation of latestLastMessage, visible in the case where a channel is actually truncated (all messages removed).
If we move the statement after calculating the new latestLastMessage, the latestLastMessage will take into consideration the old _currentLastMessage. And for the truncation case, it means that it will wrongfully be calculated as final latestLastMessage = [message (null), _currentLastMessage (oldCachedMessage)].latest; - which would be wrong.
By keeping the if (channelState.isUpToDate) _currentLastMessage = message; before calculating latestLastMessage, it means that the new latest message will also be taken into consideration for the calculation: final latestLastMessage = [message (null), _currentLastMessage (null)].latest;

The concrete example where this logic fails:

  • If we assume a real channel truncation (channel.truncated event) - (messages = [], isUpToDate=true, cache=msg1 from the previous build):

    Order A — assign first, then snapshot:

    if (channelState.isUpToDate) {                                                                                                                                                   
      _currentLastMessage = message;   // cache = null                                                                                                                                
    }                                                  
    final latestLastMessage = [message, _currentLastMessage].latest;                                                                                                                   
    // = [null, null].latest = null  → empty text shown ✓         

    Order B — snapshot first, then assign:

    final latestLastMessage = [message, _currentLastMessage].latest;                                                                                                                   
    // = [null, msg1].latest = msg1  ← captured BEFORE the assignment         
    if (channelState.isUpToDate) {                                                                                                                                                     
      _currentLastMessage = message;   // cache = null, but latestLastMessage is already msg1                                                                                         
    }                                                                                                                                                                                
    // display msg1 → stale "hello" still shown ❌          

    But this whole discussion made me realise that we can make this whole thing a bit more readable:

    final Message? latestLastMessage;
    if (channelState.isUpToDate) {
        latestLastMessage = message;
        _currentLastMessage = latestLastMessage;
    } else {
        latestLastMessage = [message, _currentLastMessage].latest;
    }

    I will make this update!

latestLastMessage = message;
_currentLastMessage = latestLastMessage;
} else {
latestLastMessage = [message, _currentLastMessage].latest;
}

if (latestLastMessage == null) {
return Text(
Expand Down Expand Up @@ -736,7 +747,18 @@ class _ChannelLastMessageTextState extends State<ChannelLastMessageText> {

// Otherwise, show the channel last message if it exists.
final message = messages.lastWhereOrNull(widget.lastMessagePredicate);
final latestLastMessage = [message, _currentLastMessage].latest;
// `_currentLastMessage` holds the most recent message seen while the
// channel has the latest messages (isUpToDate).
// While isUpToDate is false (e.g. Channel.query(idAround:) truncates
// state mid-load), fall back to it so the preview shows the actual
// latest message.
final Message? latestLastMessage;
if (channelState.isUpToDate) {
latestLastMessage = message;
_currentLastMessage = latestLastMessage;
} else {
latestLastMessage = [message, _currentLastMessage].latest;
}

if (latestLastMessage == null) {
return Text(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
// Tests for the channel-list preview widget that derives the last-message
// text from the channel state.

import 'dart:async';

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mocktail/mocktail.dart';
import 'package:stream_chat_flutter/stream_chat_flutter.dart';

import '../../mocks.dart';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use a package import instead of a relative import.

import '../../mocks.dart'; violates the Dart import rule configured for this repo; switch it to a package: import so linting stays consistent across the package.

As per coding guidelines, **/*.dart: “Always use package imports instead of relative imports (always_use_package_imports)”.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/stream_chat_flutter/test/src/scroll_view/channel_scroll_view/stream_channel_list_item_test.dart`
at line 11, The import statement for mocks.dart at the top of
stream_channel_list_item_test.dart uses a relative import path
(../../mocks.dart) which violates the repo's linting rule requiring package
imports. Replace the relative import with a package import by converting it to
use the package:stream_chat_flutter syntax, pointing to the mocks.dart file
location relative to the package root rather than using relative path traversal.

Source: Coding guidelines


void main() {
// The default English translation; matches `context.translations.emptyMessagesText`.
const emptyText = 'No messages yet';

late MockClient client;
late MockClientState clientState;
late OwnUser currentUser;

late MockChannel channelA;
late MockChannelState stateA;
late StreamController<List<Message>> messagesA;
late bool isUpToDateA;

late MockChannel channelB;
late MockChannelState stateB;
late StreamController<List<Message>> messagesB;
late bool isUpToDateB;

void wireChannel({
required MockChannel channel,
required MockChannelState state,
required StreamController<List<Message>> controller,
required bool Function() isUpToDate,
required String cid,
}) {
when(() => channel.client).thenReturn(client);
when(() => channel.state).thenReturn(state);

when(() => state.messagesStream).thenAnswer((_) => controller.stream);
when(() => state.messages).thenReturn(<Message>[]);
when(() => state.draft).thenReturn(null);
when(() => state.draftStream).thenAnswer((_) => Stream.value(null));
when(() => state.read).thenReturn(const []);
when(() => state.readStream).thenAnswer((_) => const Stream.empty());
when(() => state.typingEvents).thenReturn(const {});
when(() => state.typingEventsStream).thenAnswer((_) => Stream.value(const {}));
when(() => state.unreadCount).thenReturn(0);
when(() => state.unreadCountStream).thenAnswer((_) => Stream.value(0));
when(() => state.isUpToDate).thenAnswer((_) => isUpToDate());
when(() => state.channelState).thenReturn(
ChannelState(channel: ChannelModel(cid: cid)),
);
}

setUp(() {
client = MockClient();
clientState = MockClientState();
currentUser = OwnUser(id: 'me');

when(() => client.state).thenReturn(clientState);
when(() => clientState.currentUser).thenReturn(currentUser);
when(() => clientState.currentUserStream).thenAnswer((_) => Stream.value(currentUser));

isUpToDateA = true;
channelA = MockChannel(id: 'a', type: 'messaging');
stateA = MockChannelState();
messagesA = StreamController<List<Message>>.broadcast();
addTearDown(messagesA.close);
wireChannel(
channel: channelA,
state: stateA,
controller: messagesA,
isUpToDate: () => isUpToDateA,
cid: 'messaging:a',
);

isUpToDateB = true;
channelB = MockChannel(id: 'b', type: 'messaging');
stateB = MockChannelState();
messagesB = StreamController<List<Message>>.broadcast();
addTearDown(messagesB.close);
wireChannel(
channel: channelB,
state: stateB,
controller: messagesB,
isUpToDate: () => isUpToDateB,
cid: 'messaging:b',
);
});

Future<void> pumpSubtitle(WidgetTester tester, Channel channel) {
return tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: StreamChat(
client: client,
themeData: StreamChatThemeData(),
child: ChannelListTileSubtitle(channel: channel),
),
),
),
);
}

testWidgets(
'preserves the last-known message when state is not up-to-date and emits empty',
(tester) async {
final other = User(id: 'other');
final msg1 = Message(id: 'm1', text: 'hello', user: other);

when(() => stateA.messages).thenReturn([msg1]);
await pumpSubtitle(tester, channelA);
messagesA.add([msg1]);
await tester.pumpAndSettle();

expect(find.text('hello'), findsOneWidget);
expect(find.text(emptyText), findsNothing);

// While loading a historical window, isUpToDate flips false and the
// intermediate state emits an empty messages list.
isUpToDateA = false;
when(() => stateA.messages).thenReturn([]);
messagesA.add([]);
await tester.pumpAndSettle();

expect(find.text(emptyText), findsNothing);
expect(find.text('hello'), findsOneWidget);

// The loaded window arrives; isUpToDate restored.
final msg2 = Message(id: 'm2', text: 'world', user: other);
when(() => stateA.messages).thenReturn([msg1, msg2]);
messagesA.add([msg1, msg2]);
isUpToDateA = true;
await tester.pumpAndSettle();

expect(find.text('world'), findsOneWidget);
expect(find.text(emptyText), findsNothing);
},
);

testWidgets(
"rebinding the widget to a different channel shows that channel's state, not the previous one's",
(tester) async {
// List reordering reuses the underlying State across channels (no key
// on the list item). Channel B is empty; channel A had a message —
// the cell must render B's empty state, not A's last message.
final other = User(id: 'other');
final msgA = Message(id: 'ma', text: 'channel-a-message', user: other);

when(() => stateA.messages).thenReturn([msgA]);
await pumpSubtitle(tester, channelA);
messagesA.add([msgA]);
await tester.pumpAndSettle();

expect(find.text('channel-a-message'), findsOneWidget);

when(() => stateB.messages).thenReturn([]);
await pumpSubtitle(tester, channelB);
messagesB.add([]);
await tester.pumpAndSettle();

expect(find.text('channel-a-message'), findsNothing);
expect(find.text(emptyText), findsOneWidget);
},
);

testWidgets(
'shows the empty-state when the channel is truncated while up-to-date',
(tester) async {
// A channel.truncated event clears messages but leaves isUpToDate true.
// The preview must reflect the now-empty channel.
final other = User(id: 'other');
final msg1 = Message(id: 'm1', text: 'hello', user: other);

when(() => stateA.messages).thenReturn([msg1]);
await pumpSubtitle(tester, channelA);
messagesA.add([msg1]);
await tester.pumpAndSettle();

expect(find.text('hello'), findsOneWidget);

when(() => stateA.messages).thenReturn([]);
messagesA.add([]);
await tester.pumpAndSettle();

expect(find.text('hello'), findsNothing);
expect(find.text(emptyText), findsOneWidget);
},
);
}
Loading