Skip to content

Commit

Permalink
send [nfc]: Always pass readBySender true
Browse files Browse the repository at this point in the history
I think we're always going to want this parameter to be true
for all the messages we send from the app.

In the API bindings themselves, i.e. in lib/api/ , I generally
prefer not to hard-code this sort of thing.  That lets the API
bindings be a faithful reflection of the Zulip Server API, with
a minimum of ad-hoc specializations for the needs of our app.
So that's why the `sendMessage` binding just takes the parameter
from its caller.

But this method PerAccountStore.sendMessage is already something
specific to our app rather than an API binding; it doesn't do much
now beyond wrap the API, but the reason it exists is to be the
future home of our "outbox" logic.  That means (a) recording the
message locally so that we can display it immediately, (b) possibly
retrying the send, and (c) if the send fails or times out, recording
that fact so that our UI can present the user with that information
and the option to retry.

So that makes it a natural home for encapsulating any other logic
that this app will always want when sending a message.
In particular, passing this parameter.
  • Loading branch information
gnprice committed Dec 21, 2023
1 parent d30383c commit 48dc619
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 3 deletions.
4 changes: 2 additions & 2 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,13 @@ class PerAccountStore extends ChangeNotifier with StreamStore {
}
}

Future<void> sendMessage({required MessageDestination destination, required String content, bool? readBySender}) {
Future<void> sendMessage({required MessageDestination destination, required String content}) {
// TODO implement outbox; see design at
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M3881.20Sending.20outbox.20messages.20is.20fraught.20with.20issues/near/1405739
return _apiSendMessage(connection,
destination: destination,
content: content,
readBySender: readBySender,
readBySender: true,
);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ class _SendButtonState extends State<_SendButton> {

final store = PerAccountStoreWidget.of(context);
final content = widget.contentController.textNormalized;
store.sendMessage(destination: widget.getDestination(), content: content, readBySender: true);
store.sendMessage(destination: widget.getDestination(), content: content);

widget.contentController.clear();
}
Expand Down
23 changes: 23 additions & 0 deletions test/model/store_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'package:checks/checks.dart';
import 'package:flutter/foundation.dart';
import 'package:http/http.dart' as http;
import 'package:test/scaffolding.dart';
import 'package:zulip/api/route/messages.dart';
import 'package:zulip/model/store.dart';
import 'package:zulip/notifications.dart';

Expand Down Expand Up @@ -108,6 +109,28 @@ void main() {
check(completers(1)).length.equals(1);
});

group('PerAccountStore.sendMessage', () {
test('smoke', () async {
final store = eg.store();
final connection = store.connection as FakeApiConnection;
final stream = eg.stream();
connection.prepare(json: SendMessageResult(id: 12345).toJson());
await store.sendMessage(
destination: StreamDestination(stream.streamId, 'world'),
content: 'hello');
check(connection.lastRequest).isA<http.Request>()
..method.equals('POST')
..url.path.equals('/api/v1/messages')
..bodyFields.deepEquals({
'type': 'stream',
'to': stream.streamId.toString(),
'topic': 'world',
'content': 'hello',
'read_by_sender': 'true',
});
});
});

group('UpdateMachine.registerNotificationToken', () {
late UpdateMachine updateMachine;
late FakeApiConnection connection;
Expand Down

0 comments on commit 48dc619

Please sign in to comment.