Skip to content

Commit 84ad638

Browse files
committed
core: Add translations for exceptions related to api requests
Also hook up some other test suites that require localization contexts for tests to pass given this change.
1 parent 62e6132 commit 84ad638

File tree

8 files changed

+88
-14
lines changed

8 files changed

+88
-14
lines changed

assets/l10n/app_en.arb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
"@aboutPageTapToView": {
1616
"description": "Item subtitle in About Zulip page to navigate to Licenses page"
1717
},
18+
"apiConnectionNetworkRequestFailed": "Network request failed",
19+
"@apiConnectionNetworkRequestFailed": {
20+
"description": "Message for error dialog when a network request fails."
21+
},
1822
"chooseAccountPageTitle": "Choose account",
1923
"@chooseAccountPageTitle": {
2024
"description": "Title for ChooseAccountPage"
@@ -49,6 +53,26 @@
4953
}
5054
}
5155
},
56+
"serverExceptionMalformedResponse": "Server gave malformed response; HTTP status {httpStatus}",
57+
"@serverExceptionMalformedResponse": {
58+
"description": "Message for error dialog when an API call fails because we could not parse it.",
59+
"placeholders": {
60+
"httpStatus": {
61+
"type": "int",
62+
"example": "500"
63+
}
64+
}
65+
},
66+
"serverExceptionRequestFailed": "Network request failed: HTTP status {httpStatus}",
67+
"@serverExceptionRequestFailed": {
68+
"description": "Message for error dialog when an API call fails.",
69+
"placeholders": {
70+
"httpStatus": {
71+
"type": "int",
72+
"example": "500"
73+
}
74+
}
75+
},
5276
"userRoleOwner": "Owner",
5377
"@userRoleOwner": {
5478
"description": "Label for UserRole.owner"

lib/api/core.dart

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'dart:convert';
22
import 'dart:io';
33

4+
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
45
import 'package:http/http.dart' as http;
56

67
import '../log.dart';
@@ -84,15 +85,25 @@ class ApiConnection {
8485
try {
8586
response = await _client.send(request);
8687
} catch (e) {
87-
final String message;
88+
final String? message;
89+
final String Function(ZulipLocalizations)? translatableMessage;
8890
if (e is http.ClientException) {
8991
message = e.message;
92+
translatableMessage = null;
9093
} else if (e is TlsException) {
9194
message = e.message;
95+
translatableMessage = null;
9296
} else {
93-
message = 'Network request failed';
97+
message = null;
98+
translatableMessage = (ZulipLocalizations zulipLocalizations) {
99+
return zulipLocalizations.apiConnectionNetworkRequestFailed;
100+
};
94101
}
95-
throw NetworkException(routeName: routeName, cause: e, message: message);
102+
throw NetworkException(
103+
routeName: routeName,
104+
cause: e,
105+
message: message,
106+
translatableMessage: translatableMessage);
96107
}
97108

98109
final int httpStatus = response.statusCode;
@@ -173,6 +184,7 @@ ApiRequestException _makeApiException(String routeName, int httpStatus, Map<Stri
173184
// TODO(server): `code` should always be present. Get the "Invalid API key" case fixed.
174185
code: json.remove('code') ?? 'BAD_REQUEST',
175186
message: json.remove('msg'),
187+
translatableMessage: null,
176188
data: json,
177189
);
178190
}

lib/api/exception.dart

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11

2+
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
3+
24
/// Some kind of error from a Zulip API network request.
35
sealed class ApiRequestException implements Exception {
46
/// The name of the Zulip API route for the request.
@@ -11,13 +13,26 @@ sealed class ApiRequestException implements Exception {
1113
/// and its route name would be "getMessages".
1214
final String routeName;
1315

14-
/// A user-facing description of the error.
16+
/// Server supplied user-facing description of the error.
17+
///
18+
/// For [ZulipApiException] this may be supplied by the server as the `message`
19+
/// property in the JSON response.
20+
// TODO(i18n): no method yet to translate a server provided string.
21+
final String? message;
22+
23+
/// App supplied user-facing description of the error.
1524
///
16-
/// For [ZulipApiException] this is supplied by the server as the `message`
17-
/// property in the JSON response, and is translated into the user's language.
18-
final String message;
25+
/// This description will be translated according to the
26+
/// current set locale.
27+
final String Function(ZulipLocalizations)? translatableMessage;
28+
29+
ApiRequestException({required this.routeName, required this.message, required this.translatableMessage});
1930

20-
ApiRequestException({required this.routeName, required this.message});
31+
String toTranslatedString(ZulipLocalizations zulipLocalizations) {
32+
return (translatableMessage != null)
33+
? translatableMessage!(zulipLocalizations)
34+
: super.toString();
35+
}
2136
}
2237

2338
/// An error returned through the Zulip server API.
@@ -46,6 +61,7 @@ class ZulipApiException extends ApiRequestException {
4661
required this.httpStatus,
4762
required this.data,
4863
required super.message,
64+
required super.translatableMessage,
4965
}) : assert(400 <= httpStatus && httpStatus <= 499);
5066
}
5167

@@ -58,7 +74,7 @@ class NetworkException extends ApiRequestException {
5874
/// but empirically it can be [TlsException] and possibly others.
5975
final Object cause;
6076

61-
NetworkException({required super.routeName, required super.message, required this.cause});
77+
NetworkException({required super.routeName, required super.message, required super.translatableMessage, required this.cause});
6278
}
6379

6480
/// Some kind of server-side error in handling the request.
@@ -79,6 +95,7 @@ abstract class ServerException extends ApiRequestException {
7995
required this.httpStatus,
8096
required this.data,
8197
required super.message,
98+
required super.translatableMessage,
8299
});
83100
}
84101

@@ -89,7 +106,11 @@ class Server5xxException extends ServerException {
89106
required super.httpStatus,
90107
required super.data,
91108
}) : assert(500 <= httpStatus && httpStatus <= 599),
92-
super(message: 'Network request failed: HTTP status $httpStatus'); // TODO(i18n)
109+
super(
110+
message: null,
111+
translatableMessage: (ZulipLocalizations zulipLocalizations) {
112+
return zulipLocalizations.serverExceptionRequestFailed(httpStatus);
113+
});
93114
}
94115

95116
/// An error where the server's response doesn't match the Zulip API.
@@ -110,5 +131,9 @@ class MalformedServerResponseException extends ServerException {
110131
required super.routeName,
111132
required super.httpStatus,
112133
required super.data,
113-
}) : super(message: 'Server gave malformed response; HTTP status $httpStatus'); // TODO(i18n)
134+
}) : super(
135+
message: null,
136+
translatableMessage: (ZulipLocalizations zulipLocalizations) {
137+
return zulipLocalizations.serverExceptionMalformedResponse(httpStatus);
138+
});
114139
}

test/api/core_test.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import 'dart:io';
33

44
import 'package:checks/checks.dart';
55
import 'package:checks/context.dart';
6+
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
67
import 'package:http/http.dart' as http;
78
import 'package:test/scaffolding.dart';
89
import 'package:zulip/api/core.dart';
@@ -155,7 +156,11 @@ void main() {
155156

156157
checkRequest(http.ClientException('Oops'), it()..message.equals('Oops'));
157158
checkRequest(const TlsException('Oops'), it()..message.equals('Oops'));
158-
checkRequest((foo: 'bar'), it()..message.equals('Network request failed'));
159+
final zulipLocalizations = lookupZulipLocalizations(ZulipLocalizations.supportedLocales.first);
160+
final expectedMessage = zulipLocalizations.apiConnectionNetworkRequestFailed;
161+
checkRequest((foo: 'bar'), it()
162+
..toTranslatedString(zulipLocalizations)
163+
.equals(expectedMessage));
159164
});
160165

161166
test('API 4xx errors, well formed', () async {

test/api/exception_checks.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import 'package:checks/checks.dart';
2+
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
23
import 'package:zulip/api/exception.dart';
34

45
extension ApiRequestExceptionChecks on Subject<ApiRequestException> {
56
Subject<String> get routeName => has((e) => e.routeName, 'routeName');
6-
Subject<String> get message => has((e) => e.message, 'message');
7+
Subject<String?> get message => has((e) => e.message, 'message');
8+
Subject<String> toTranslatedString(ZulipLocalizations zulipLocalizations) =>
9+
has((e) => e.toTranslatedString(zulipLocalizations), 'getTranslatedMessage');
710
}
811

912
extension ZulipApiExceptionChecks on Subject<ZulipApiException> {

test/widgets/autocomplete_test.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import 'package:checks/checks.dart';
22
import 'package:flutter/material.dart';
3+
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
34
import 'package:flutter_test/flutter_test.dart';
45
import 'package:zulip/api/model/model.dart';
56
import 'package:zulip/api/route/messages.dart';
@@ -40,6 +41,8 @@ Future<Finder> setupToComposeInput(WidgetTester tester, {
4041

4142
await tester.pumpWidget(
4243
MaterialApp(
44+
localizationsDelegates: ZulipLocalizations.localizationsDelegates,
45+
supportedLocales: ZulipLocalizations.supportedLocales,
4346
home: GlobalStoreWidget(
4447
child: PerAccountStoreWidget(
4548
accountId: eg.selfAccount.id,

test/widgets/clipboard_test.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ void main() {
3232
body: Builder(builder: (context) => Center(
3333
child: ElevatedButton(
3434
onPressed: () async {
35-
// TODO(i18n)
3635
copyWithPopup(context: context, successContent: const Text('Text copied'),
3736
data: ClipboardData(text: text));
3837
},

test/widgets/content_test.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import 'dart:io';
33
import 'package:checks/checks.dart';
44
import 'package:flutter/foundation.dart';
55
import 'package:flutter/material.dart';
6+
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
67
import 'package:flutter_test/flutter_test.dart';
78
import 'package:url_launcher/url_launcher.dart';
89
import 'package:zulip/api/core.dart';
@@ -68,6 +69,8 @@ void main() {
6869
addTearDown(testBinding.reset);
6970

7071
await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
72+
localizationsDelegates: ZulipLocalizations.localizationsDelegates,
73+
supportedLocales: ZulipLocalizations.supportedLocales,
7174
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
7275
child: BlockContentList(
7376
nodes: parseContent(html).nodes)))));

0 commit comments

Comments
 (0)