Skip to content

Commit 977fe82

Browse files
sm-sayedichrisbobbe
sm-sayedi
and
chrisbobbe
committed
app: On launch, go to the last visited account
Fixes: #524 [chris: rebased atop some notification-test refactors in the last few commits] Co-authored-by: Chris Bobbe <[email protected]>
1 parent 37b39e4 commit 977fe82

File tree

6 files changed

+134
-29
lines changed

6 files changed

+134
-29
lines changed

‎lib/widgets/app.dart‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,14 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
211211
}
212212

213213
final globalStore = GlobalStoreWidget.of(context);
214-
// TODO(#524) choose initial account as last one used
215-
final initialAccountId = globalStore.accounts.firstOrNull?.id;
214+
final lastVisitedAccount = globalStore.lastVisitedAccount;
215+
216216
return [
217-
if (initialAccountId == null)
217+
if (lastVisitedAccount == null)
218+
// There are no accounts, or the last-visited account was logged out.
218219
MaterialWidgetRoute(page: const ChooseAccountPage())
219220
else
220-
HomePage.buildRoute(accountId: initialAccountId),
221+
HomePage.buildRoute(accountId: lastVisitedAccount.id),
221222
];
222223
}
223224

‎lib/widgets/share.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ class ShareService {
6060

6161
final globalStore = GlobalStoreWidget.of(context);
6262

63-
// TODO(#524) use last account used, not the first in the list
6463
// TODO(#1779) allow selecting account, if there are multiple
65-
final accountId = globalStore.accounts.firstOrNull?.id;
64+
final lastVisitedAccount = globalStore.lastVisitedAccount;
65+
final accountId = lastVisitedAccount?.id ?? globalStore.accountIds.firstOrNull;
6666

6767
if (accountId == null) {
6868
final zulipLocalizations = ZulipLocalizations.of(context);

‎test/model/test_store.dart‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,13 +169,24 @@ class TestGlobalStore extends GlobalStore with _ApiConnectionsMixin, _DatabaseMi
169169
/// The given initial snapshot will be used to initialize a corresponding
170170
/// [PerAccountStore] when [perAccount] is subsequently called for this
171171
/// account, in particular when a [PerAccountStoreWidget] is mounted.
172-
Future<void> add(Account account, InitialSnapshot initialSnapshot) async {
172+
///
173+
/// By default, the account is marked as [IntGlobalSetting.lastVisitedAccountId].
174+
/// Pass false for [markLastVisited] to unmark it.
175+
Future<void> add(
176+
Account account,
177+
InitialSnapshot initialSnapshot, {
178+
bool markLastVisited = true,
179+
}) async {
173180
assert(initialSnapshot.zulipVersion == account.zulipVersion);
174181
assert(initialSnapshot.zulipMergeBase == account.zulipMergeBase);
175182
assert(initialSnapshot.zulipFeatureLevel == account.zulipFeatureLevel);
176183
await insertAccount(account.toCompanion(false));
177184
assert(!_initialSnapshots.containsKey(account.id));
178185
_initialSnapshots[account.id] = initialSnapshot;
186+
187+
if (markLastVisited) {
188+
await setLastVisitedAccount(account.id);
189+
}
179190
}
180191

181192
Duration? loadPerAccountDuration;

‎test/notifications/open_test.dart‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import 'package:zulip/widgets/page.dart';
2020
import '../example_data.dart' as eg;
2121
import '../model/binding.dart';
2222
import '../model/narrow_checks.dart';
23+
import '../model/store_checks.dart';
2324
import '../stdlib_checks.dart';
2425
import '../test_navigation.dart';
2526
import '../widgets/checks.dart';
@@ -112,12 +113,11 @@ void main() {
112113
return;
113114
}
114115
await tester.pump();
115-
final accountIds = testBinding.globalStore.accountIds;
116-
final initialAccountId = accountIds.firstOrNull;
117-
if (initialAccountId == null) {
118-
takeChooseAccountPageRoute();
116+
final lastVisitedAccount = testBinding.globalStore.lastVisitedAccount;
117+
if (lastVisitedAccount != null) {
118+
takeHomePageRouteForAccount(lastVisitedAccount.id);
119119
} else {
120-
takeHomePageRouteForAccount(initialAccountId);
120+
takeChooseAccountPageRoute();
121121
}
122122
check(pushedRoutes).isEmpty();
123123
}
@@ -273,7 +273,8 @@ void main() {
273273

274274
testWidgets('wait for app to become ready', (tester) async {
275275
addTearDown(testBinding.reset);
276-
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
276+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot(),
277+
markLastVisited: false);
277278
await prepare(tester, early: true);
278279
final message = eg.streamMessage();
279280
await openNotification(tester, eg.selfAccount, message);
@@ -286,7 +287,7 @@ void main() {
286287
// Now let the GlobalStore get loaded and the app's main UI get mounted.
287288
await tester.pump();
288289
// The navigator first pushes the starting routes…
289-
takeHomePageRouteForAccount(eg.selfAccount.id); // because first in list
290+
takeChooseAccountPageRoute(); // (no lastVisitedAccountId in this test)
290291
// … and then the one the notification leads to.
291292
matchesNavigation(check(pushedRoutes).single, eg.selfAccount, message);
292293
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
@@ -326,6 +327,44 @@ void main() {
326327
takeHomePageRouteForAccount(accountB.id); // because associated account
327328
matchesNavigation(check(pushedRoutes).single, accountB, message);
328329
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
330+
331+
group('changes last visited account', () {
332+
testWidgets('app already opened, then notification is opened', (tester) async {
333+
addTearDown(testBinding.reset);
334+
await testBinding.globalStore.add(
335+
eg.selfAccount, eg.initialSnapshot(realmUsers: [eg.selfUser]));
336+
await testBinding.globalStore.add(
337+
eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]),
338+
markLastVisited: false);
339+
await prepare(tester);
340+
check(testBinding.globalStore).lastVisitedAccount.equals(eg.selfAccount);
341+
342+
await checkOpenNotification(tester, eg.otherAccount, eg.streamMessage());
343+
check(testBinding.globalStore).lastVisitedAccount.equals(eg.otherAccount);
344+
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
345+
346+
testWidgets('app is opened through notification', (tester) async {
347+
addTearDown(testBinding.reset);
348+
349+
final accountA = eg.selfAccount;
350+
final accountB = eg.otherAccount;
351+
final message = eg.streamMessage();
352+
await testBinding.globalStore.add(accountA, eg.initialSnapshot());
353+
await testBinding.globalStore.add(
354+
accountB, eg.initialSnapshot(realmUsers: [eg.otherUser]),
355+
markLastVisited: false);
356+
check(testBinding.globalStore).lastVisitedAccount.equals(eg.selfAccount);
357+
setupNotificationDataForLaunch(tester, accountB, message);
358+
359+
await prepare(tester, early: true);
360+
check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet
361+
362+
await tester.pump();
363+
takeHomePageRouteForAccount(accountB.id); // because associated account
364+
matchesNavigation(check(pushedRoutes).single, accountB, message);
365+
check(testBinding.globalStore).lastVisitedAccount.equals(eg.otherAccount);
366+
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
367+
});
329368
});
330369

331370
group('NotificationOpenPayload', () {

‎test/widgets/app_test.dart‎

Lines changed: 66 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,47 @@ void main() {
4343
]);
4444
});
4545

46-
testWidgets('when have accounts, go to home page for first account', (tester) async {
47-
// We'll need per-account data for the account that a page will be opened
48-
// for, but not for the other account.
49-
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
50-
await testBinding.globalStore.insertAccount(eg.otherAccount.toCompanion(false));
51-
await prepare(tester);
46+
group('when have accounts', () {
47+
testWidgets('with account(s) visited, go to home page for the last visited account', (tester) async {
48+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
49+
await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot(
50+
realmUsers: [eg.otherUser]));
51+
await prepare(tester);
52+
53+
check(pushedRoutes).deepEquals(<Condition<Object?>>[
54+
(it) => it.isA<MaterialAccountWidgetRoute>()
55+
..accountId.equals(eg.otherAccount.id)
56+
..page.isA<HomePage>(),
57+
]);
58+
});
5259

53-
check(pushedRoutes).deepEquals(<Condition<Object?>>[
54-
(it) => it.isA<MaterialAccountWidgetRoute>()
55-
..accountId.equals(eg.selfAccount.id)
56-
..page.isA<HomePage>(),
57-
]);
60+
testWidgets('with no account visited, go to home page for the first account', (tester) async {
61+
await testBinding.globalStore.add(
62+
eg.selfAccount, eg.initialSnapshot(),
63+
markLastVisited: false);
64+
await testBinding.globalStore.add(
65+
eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]),
66+
markLastVisited: false);
67+
check(testBinding.globalStore).lastVisitedAccount.isNull();
68+
await prepare(tester);
69+
70+
check(pushedRoutes).deepEquals(<Condition<Object?>>[
71+
(it) => it.isA<MaterialAccountWidgetRoute>()
72+
..accountId.equals(eg.selfAccount.id)
73+
..page.isA<HomePage>(),
74+
]);
75+
});
76+
77+
testWidgets('with last visited account id matching none of the accounts, go to choose account', (tester) async {
78+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
79+
await testBinding.globalStore.setLastVisitedAccount(eg.otherAccount.id);
80+
check(testBinding.globalStore).getAccount(eg.otherAccount.id).isNull();
81+
await prepare(tester);
82+
83+
check(pushedRoutes).deepEquals(<Condition<Object?>>[
84+
(it) => it.isA<WidgetRoute>().page.isA<ChooseAccountPage>(),
85+
]);
86+
});
5887
});
5988
});
6089

@@ -99,6 +128,7 @@ void main() {
99128
testWidgets('push route when popping last route on stack', (tester) async {
100129
// Set up the loading of per-account data to fail.
101130
await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
131+
await testBinding.globalStore.setLastVisitedAccount(eg.selfAccount.id);
102132
testBinding.globalStore.loadPerAccountDuration = Duration.zero;
103133
testBinding.globalStore.loadPerAccountException = eg.apiExceptionUnauthorized();
104134
await prepare(tester);
@@ -133,6 +163,7 @@ void main() {
133163
const loadPerAccountDuration = Duration(seconds: 30);
134164
assert(loadPerAccountDuration > kTryAnotherAccountWaitPeriod);
135165
await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
166+
await testBinding.globalStore.setLastVisitedAccount(eg.selfAccount.id);
136167
testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
137168
testBinding.globalStore.loadPerAccountException = eg.apiExceptionUnauthorized();
138169
await prepare(tester);
@@ -281,8 +312,9 @@ void main() {
281312
testWidgets('choosing an account clears the navigator stack', (tester) async {
282313
addTearDown(testBinding.reset);
283314
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
284-
await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot(
285-
realmUsers: [eg.otherUser]));
315+
await testBinding.globalStore.add(
316+
eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]),
317+
markLastVisited: false);
286318

287319
final pushedRoutes = <Route<void>>[];
288320
final poppedRoutes = <Route<void>>[];
@@ -319,6 +351,27 @@ void main() {
319351
..page.isA<HomePage>();
320352
});
321353

354+
testWidgets('choosing an account changes the last visited account', (tester) async {
355+
addTearDown(testBinding.reset);
356+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
357+
await testBinding.globalStore.add(
358+
eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]),
359+
markLastVisited: false);
360+
361+
await tester.pumpWidget(ZulipApp());
362+
await tester.pump();
363+
364+
final navigator = await ZulipApp.navigator;
365+
unawaited(navigator.push(MaterialWidgetRoute(page: const ChooseAccountPage())));
366+
await tester.pump();
367+
await tester.pump();
368+
369+
check(testBinding.globalStore).lastVisitedAccount.equals(eg.selfAccount);
370+
await tester.tap(find.text(eg.otherAccount.email));
371+
await tester.pump();
372+
check(testBinding.globalStore).lastVisitedAccount.equals(eg.otherAccount);
373+
});
374+
322375
group('log out', () {
323376
Future<(Widget, Widget)> prepare(WidgetTester tester, {required Account account}) async {
324377
await setupChooseAccountPage(tester, accounts: [account]);

‎test/widgets/home_test.dart‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,9 @@ void main () {
333333
pushedRoutes = [];
334334
lastPoppedRoute = null;
335335
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
336-
await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot(
337-
realmUsers: [eg.otherUser]));
336+
await testBinding.globalStore.add(
337+
eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]),
338+
markLastVisited: false);
338339
await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
339340
await tester.pump(Duration.zero); // wait for the loading page
340341
checkOnLoadingPage();

0 commit comments

Comments
 (0)