Skip to content

Commit d7e6c76

Browse files
notif: Change account when opening a notification, if different account
Previously, opening a notification from a different account would leave the home page on a different account than the newly opened message list. Now, if the notification's account is different from currently viewing account, we change the whole navigation stack: pop all routes, push HomePage, push MessageList Fixes: #1210
1 parent 9296fb3 commit d7e6c76

File tree

2 files changed

+61
-8
lines changed

2 files changed

+61
-8
lines changed

lib/notifications/open.dart

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import '../model/binding.dart';
1313
import '../model/narrow.dart';
1414
import '../widgets/app.dart';
1515
import '../widgets/dialog.dart';
16+
import '../widgets/home.dart';
1617
import '../widgets/message_list.dart';
1718
import '../widgets/page.dart';
1819
import '../widgets/store.dart';
@@ -135,7 +136,9 @@ class NotificationOpenService {
135136
final route = routeForNotification(context: context, data: notifNavData);
136137
if (route == null) return; // TODO(log)
137138

138-
// TODO(nav): Better interact with existing nav stack on notif open
139+
if (GlobalStoreWidget.of(context).lastVisitedAccount?.id != route.accountId) {
140+
HomePage.navigate(context, accountId: route.accountId);
141+
}
139142
unawaited(navigator.push(route));
140143
}
141144

@@ -158,7 +161,9 @@ class NotificationOpenService {
158161
final route = routeForNotification(context: context, data: data);
159162
if (route == null) return; // TODO(log)
160163

161-
// TODO(nav): Better interact with existing nav stack on notif open
164+
if (GlobalStoreWidget.of(context).lastVisitedAccount?.id != route.accountId) {
165+
HomePage.navigate(context, accountId: route.accountId);
166+
}
162167
unawaited(navigator.push(route));
163168
}
164169

test/notifications/open_test.dart

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ void main() {
8888

8989
group('NotificationOpenService', () {
9090
late List<Route<void>> pushedRoutes;
91+
Route<void>? lastPoppedRoute;
9192

9293
void takeHomePageRouteForAccount(int accountId) {
9394
check(pushedRoutes).first.which(
@@ -106,8 +107,18 @@ void main() {
106107
Future<void> prepare(WidgetTester tester, {bool early = false}) async {
107108
await init();
108109
pushedRoutes = [];
110+
lastPoppedRoute = null;
109111
final testNavObserver = TestNavigatorObserver()
110-
..onPushed = (route, prevRoute) => pushedRoutes.add(route);
112+
..onPushed = (route, prevRoute) {
113+
pushedRoutes.add(route);
114+
}
115+
..onPopped = (route, prevRoute) {
116+
lastPoppedRoute = route;
117+
}
118+
..onReplaced = (route, prevRoute) {
119+
lastPoppedRoute = prevRoute;
120+
pushedRoutes.add(route!);
121+
};
111122
// This uses [ZulipApp] instead of [TestZulipApp] because notification
112123
// logic uses `await ZulipApp.navigator`.
113124
await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
@@ -178,6 +189,14 @@ void main() {
178189
}
179190
}
180191

192+
void takeHomePageReplacement(int accountId) {
193+
check(lastPoppedRoute).which(
194+
(it) => it.isA<MaterialAccountWidgetRoute>()
195+
..page.isA<HomePage>());
196+
lastPoppedRoute = null;
197+
takeHomePageRouteForAccount(accountId);
198+
}
199+
181200
void matchesNavigation(Subject<Route<void>> route, Account account, Message message) {
182201
route.isA<MaterialAccountWidgetRoute>()
183202
..accountId.equals(account.id)
@@ -186,8 +205,18 @@ void main() {
186205
selfUserId: account.userId));
187206
}
188207

189-
Future<void> checkOpenNotification(WidgetTester tester, Account account, Message message) async {
208+
Future<void> checkOpenNotification(
209+
WidgetTester tester,
210+
Account account,
211+
Message message, {
212+
bool expectHomePageReplaced = false,
213+
}) async {
190214
await openNotification(tester, account, message);
215+
if (expectHomePageReplaced) {
216+
takeHomePageReplacement(account.id);
217+
} else {
218+
check(lastPoppedRoute).isNull();
219+
}
191220
matchesNavigation(check(pushedRoutes).single, account, message);
192221
pushedRoutes.clear();
193222
}
@@ -268,10 +297,13 @@ void main() {
268297
accounts[3], eg.initialSnapshot(realmUsers: [user2]));
269298
await prepare(tester);
270299

271-
await checkOpenNotification(tester, accounts[0], eg.streamMessage());
272-
await checkOpenNotification(tester, accounts[1], eg.streamMessage());
273-
await checkOpenNotification(tester, accounts[2], eg.streamMessage());
274300
await checkOpenNotification(tester, accounts[3], eg.streamMessage());
301+
await checkOpenNotification(tester, accounts[2], eg.streamMessage(),
302+
expectHomePageReplaced: true);
303+
await checkOpenNotification(tester, accounts[1], eg.streamMessage(),
304+
expectHomePageReplaced: true);
305+
await checkOpenNotification(tester, accounts[0], eg.streamMessage(),
306+
expectHomePageReplaced: true);
275307
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
276308

277309
testWidgets('wait for app to become ready', (tester) async {
@@ -341,7 +373,8 @@ void main() {
341373
await prepare(tester);
342374
check(testBinding.globalStore).lastVisitedAccount.equals(eg.selfAccount);
343375

344-
await checkOpenNotification(tester, eg.otherAccount, eg.streamMessage());
376+
await checkOpenNotification(tester, eg.otherAccount, eg.streamMessage(),
377+
expectHomePageReplaced: true);
345378
check(testBinding.globalStore).lastVisitedAccount.equals(eg.otherAccount);
346379
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
347380

@@ -367,6 +400,21 @@ void main() {
367400
check(testBinding.globalStore).lastVisitedAccount.equals(accountB);
368401
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
369402
});
403+
404+
testWidgets('replaces HomePage with related account', (tester) async {
405+
addTearDown(testBinding.reset);
406+
await testBinding.globalStore.add(
407+
eg.selfAccount, eg.initialSnapshot(realmUsers: [eg.selfUser]));
408+
await testBinding.globalStore.add(
409+
eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]));
410+
411+
await prepare(tester);
412+
check(testBinding.globalStore).lastVisitedAccount.equals(eg.otherAccount);
413+
414+
await checkOpenNotification(tester, eg.selfAccount, eg.streamMessage(),
415+
expectHomePageReplaced: true);
416+
check(testBinding.globalStore).lastVisitedAccount.equals(eg.selfAccount);
417+
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
370418
});
371419

372420
group('NotificationOpenPayload', () {

0 commit comments

Comments
 (0)