Skip to content

Commit 2fa5348

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 513cde2 commit 2fa5348

File tree

12 files changed

+243
-57
lines changed

12 files changed

+243
-57
lines changed

‎lib/model/database.dart‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,22 @@ class AppDatabase extends _$AppDatabase {
238238
},
239239
from9To10: (m, schema) async {
240240
await m.createTable(schema.intGlobalSettings);
241+
242+
// To provide a smooth experience for users the first time they install a
243+
// new version of the app with the support of last visited account, we
244+
// declare the first account, if available, as the last visited one.
245+
// That way, user is brought straight to the first account instead of the
246+
// choose account page, as they would expect.
247+
final firstAccount = await (m.database.select(schema.accounts)
248+
..limit(1)).getSingleOrNull();
249+
if (firstAccount == null) return;
250+
251+
final firstAccountId = firstAccount.read<int>('id');
252+
await m.database.into(schema.intGlobalSettings).insert(
253+
RawValuesInsertable({
254+
'name': Variable(IntGlobalSetting.lastVisitedAccountId.name),
255+
'value': Variable(firstAccountId),
256+
}));
241257
},
242258
);
243259

‎lib/model/settings.dart‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,15 @@ enum BoolGlobalSetting {
224224
/// eagerly each time, to avoid creating a new schema version each time we
225225
/// finish an experimental feature.)
226226
enum IntGlobalSetting {
227-
/// A non-setting to ensure this enum has at least one value.
227+
/// A pseudo-setting recording the id of the account the user has visited most
228+
/// recently, from the list of all the available accounts on the device.
228229
///
229-
/// We'll remove this once we have an actual setting (in the next commits).
230-
/// (Having one stable value in this enum is also handy for tests.)
231-
placeholderIgnore,
230+
/// In some cases, this may point to an account that doesn't actually exist on
231+
/// the device, for example, when the last visited account is logged out and
232+
/// another account is not visited during the same running session. For cases
233+
/// like these, it's the responsibility of the code that reads this value to
234+
/// check for the availability of the account that corresponds to this id.
235+
lastVisitedAccountId,
232236

233237
// Former settings which might exist in the database,
234238
// whose names should therefore not be reused:

‎lib/model/store.dart‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,13 @@ abstract class GlobalStore extends ChangeNotifier {
269269

270270
Account? getAccount(int id) => _accounts[id];
271271

272+
Account? get lastVisitedAccount =>
273+
_accounts[settings.getInt(IntGlobalSetting.lastVisitedAccountId)];
274+
275+
Future<void> setLastVisitedAccount(int accountId) {
276+
return settings.setInt(IntGlobalSetting.lastVisitedAccountId, accountId);
277+
}
278+
272279
/// Add an account to the store, returning its assigned account ID.
273280
Future<int> insertAccount(AccountsCompanion data) async {
274281
final account = await doInsertAccount(data);

‎lib/widgets/app.dart‎

Lines changed: 19 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

@@ -254,6 +255,7 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
254255
if (widget.navigatorObservers != null)
255256
...widget.navigatorObservers!,
256257
_PreventEmptyStack(),
258+
_UpdateLastVisitedAccount(GlobalStoreWidget.of(context)),
257259
],
258260
builder: (BuildContext context, Widget? child) {
259261
if (!ZulipApp.ready.value) {
@@ -305,6 +307,19 @@ class _PreventEmptyStack extends NavigatorObserver {
305307
}
306308
}
307309

310+
class _UpdateLastVisitedAccount extends NavigatorObserver {
311+
_UpdateLastVisitedAccount(this.globalStore);
312+
313+
final GlobalStore globalStore;
314+
315+
@override
316+
void didChangeTop(Route<void> topRoute, _) {
317+
if (topRoute case AccountPageRouteMixin(:var accountId)) {
318+
globalStore.setLastVisitedAccount(accountId);
319+
}
320+
}
321+
}
322+
308323
class ChooseAccountPage extends StatelessWidget {
309324
const ChooseAccountPage({super.key});
310325

‎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/database_test.dart‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import 'schemas/schema_v2.dart' as v2;
1313
import 'schemas/schema_v3.dart' as v3;
1414
import 'schemas/schema_v4.dart' as v4;
1515
import 'schemas/schema_v5.dart' as v5;
16+
import 'schemas/schema_v9.dart' as v9;
17+
import 'schemas/schema_v10.dart' as v10;
1618
import 'store_checks.dart';
1719

1820
void main() {
@@ -122,7 +124,7 @@ void main() {
122124
.insert(IntGlobalSettingRow(name: 'nonsense', value: 1));
123125
check(await db.getIntGlobalSettings()).isEmpty();
124126

125-
final setting = IntGlobalSetting.placeholderIgnore;
127+
final setting = IntGlobalSetting.lastVisitedAccountId;
126128
await db.into(db.intGlobalSettings)
127129
.insert(IntGlobalSettingRow(name: setting.name, value: 1));
128130
check(await db.getIntGlobalSettings())
@@ -133,7 +135,7 @@ void main() {
133135
check(await db.getIntGlobalSettings()).isEmpty();
134136

135137
// As in doSetIntGlobalSetting for `value` non-null.
136-
final setting = IntGlobalSetting.placeholderIgnore;
138+
final setting = IntGlobalSetting.lastVisitedAccountId;
137139
await db.into(db.intGlobalSettings).insertOnConflictUpdate(
138140
IntGlobalSettingRow(name: setting.name, value: 1));
139141
check(await db.getIntGlobalSettings())
@@ -142,7 +144,7 @@ void main() {
142144
});
143145

144146
test('delete, then get', () async {
145-
final setting = IntGlobalSetting.placeholderIgnore;
147+
final setting = IntGlobalSetting.lastVisitedAccountId;
146148
await db.into(db.intGlobalSettings).insertOnConflictUpdate(
147149
IntGlobalSettingRow(name: setting.name, value: 1));
148150
check(await db.getIntGlobalSettings()).deepEquals({setting: 1});
@@ -156,7 +158,7 @@ void main() {
156158
});
157159

158160
test('insert replaces', () async {
159-
final setting = IntGlobalSetting.placeholderIgnore;
161+
final setting = IntGlobalSetting.lastVisitedAccountId;
160162
await db.into(db.intGlobalSettings).insertOnConflictUpdate(
161163
IntGlobalSettingRow(name: setting.name, value: 1));
162164
check(await db.getIntGlobalSettings())
@@ -174,7 +176,7 @@ void main() {
174176
check(await db.getIntGlobalSettings()).isEmpty();
175177

176178
// As in doSetIntGlobalSetting for `value` null.
177-
final setting = IntGlobalSetting.placeholderIgnore;
179+
final setting = IntGlobalSetting.lastVisitedAccountId;
178180
final query = db.delete(db.intGlobalSettings)
179181
..where((r) => r.name.equals(setting.name));
180182
await query.go();
@@ -400,6 +402,40 @@ void main() {
400402
});
401403

402404
// TODO(#1593) test upgrade to v9: legacyUpgradeState set to noLegacy
405+
406+
test('upgrade to v10: first account id is set as the last visited account id', () async {
407+
final schema = await verifier.schemaAt(9);
408+
final before = v9.DatabaseAtV9(schema.newConnection());
409+
await before.into(before.accounts).insert(v9.AccountsCompanion.insert(
410+
realmUrl: 'https://chat.example/',
411+
userId: 1,
412+
email: '[email protected]',
413+
apiKey: '1234',
414+
zulipVersion: '10.0',
415+
zulipMergeBase: const Value('10.0'),
416+
zulipFeatureLevel: 370,
417+
));
418+
await before.into(before.accounts).insert(v9.AccountsCompanion.insert(
419+
realmUrl: 'https://example.com/',
420+
userId: 2,
421+
email: '[email protected]',
422+
apiKey: '4321',
423+
zulipVersion: '11.0',
424+
zulipMergeBase: const Value('11.0'),
425+
zulipFeatureLevel: 420,
426+
));
427+
await before.close();
428+
429+
final db = AppDatabase(schema.newConnection());
430+
await verifier.migrateAndValidate(db, 10);
431+
await db.close();
432+
433+
final after = v10.DatabaseAtV10(schema.newConnection());
434+
final intGlobalSettings = await after.select(after.intGlobalSettings).getSingle();
435+
check(intGlobalSettings.name).equals(IntGlobalSetting.lastVisitedAccountId.name);
436+
check(intGlobalSettings.value).equals(1);
437+
await after.close();
438+
});
403439
});
404440
}
405441

‎test/model/settings_test.dart‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -191,35 +191,35 @@ void main() {
191191
group('getInt/setInt', () {
192192
test('get from initial load', () {
193193
final globalSettings = eg.globalStore(intGlobalSettings: {
194-
IntGlobalSetting.placeholderIgnore: 1,
194+
IntGlobalSetting.lastVisitedAccountId: 1,
195195
}).settings;
196-
check(globalSettings).getInt(IntGlobalSetting.placeholderIgnore)
196+
check(globalSettings).getInt(IntGlobalSetting.lastVisitedAccountId)
197197
.equals(1);
198198
});
199199

200200
test('set, get', () async {
201201
final globalSettings = eg.globalStore(intGlobalSettings: {}).settings;
202-
check(globalSettings).getInt(IntGlobalSetting.placeholderIgnore)
202+
check(globalSettings).getInt(IntGlobalSetting.lastVisitedAccountId)
203203
.isNull();
204204

205-
await globalSettings.setInt(IntGlobalSetting.placeholderIgnore, 1);
206-
check(globalSettings).getInt(IntGlobalSetting.placeholderIgnore)
205+
await globalSettings.setInt(IntGlobalSetting.lastVisitedAccountId, 1);
206+
check(globalSettings).getInt(IntGlobalSetting.lastVisitedAccountId)
207207
.equals(1);
208208

209-
await globalSettings.setInt(IntGlobalSetting.placeholderIgnore, 100);
210-
check(globalSettings).getInt(IntGlobalSetting.placeholderIgnore)
209+
await globalSettings.setInt(IntGlobalSetting.lastVisitedAccountId, 100);
210+
check(globalSettings).getInt(IntGlobalSetting.lastVisitedAccountId)
211211
.equals(100);
212212
});
213213

214214
test('set to null -> get returns null', () async {
215215
final globalSettings = eg.globalStore(intGlobalSettings: {
216-
IntGlobalSetting.placeholderIgnore: 1,
216+
IntGlobalSetting.lastVisitedAccountId: 1,
217217
}).settings;
218-
check(globalSettings).getInt(IntGlobalSetting.placeholderIgnore)
218+
check(globalSettings).getInt(IntGlobalSetting.lastVisitedAccountId)
219219
.equals(1);
220220

221-
await globalSettings.setInt(IntGlobalSetting.placeholderIgnore, null);
222-
check(globalSettings).getInt(IntGlobalSetting.placeholderIgnore)
221+
await globalSettings.setInt(IntGlobalSetting.lastVisitedAccountId, null);
222+
check(globalSettings).getInt(IntGlobalSetting.lastVisitedAccountId)
223223
.isNull();
224224
});
225225

@@ -231,45 +231,45 @@ void main() {
231231
final globalSettings = eg.globalStore(intGlobalSettings: {}).settings;
232232
globalSettings.addListener(() => notifiedCount++);
233233

234-
await globalSettings.setInt(IntGlobalSetting.placeholderIgnore, null);
234+
await globalSettings.setInt(IntGlobalSetting.lastVisitedAccountId, null);
235235
check(notifiedCount).equals(0);
236236
});
237237

238238
test('10 to 10 -> no update', () async {
239239
final globalSettings = eg.globalStore(intGlobalSettings: {
240-
IntGlobalSetting.placeholderIgnore: 10,
240+
IntGlobalSetting.lastVisitedAccountId: 10,
241241
}).settings;
242242
globalSettings.addListener(() => notifiedCount++);
243243

244-
await globalSettings.setInt(IntGlobalSetting.placeholderIgnore, 10);
244+
await globalSettings.setInt(IntGlobalSetting.lastVisitedAccountId, 10);
245245
check(notifiedCount).equals(0);
246246
});
247247

248248
test('null to 10 -> does the update', () async {
249249
final globalSettings = eg.globalStore(intGlobalSettings: {}).settings;
250250
globalSettings.addListener(() => notifiedCount++);
251251

252-
await globalSettings.setInt(IntGlobalSetting.placeholderIgnore, 10);
252+
await globalSettings.setInt(IntGlobalSetting.lastVisitedAccountId, 10);
253253
check(notifiedCount).equals(1);
254254
});
255255

256256
test('10 to null -> does the update', () async {
257257
final globalSettings = eg.globalStore(intGlobalSettings: {
258-
IntGlobalSetting.placeholderIgnore: 10,
258+
IntGlobalSetting.lastVisitedAccountId: 10,
259259
}).settings;
260260
globalSettings.addListener(() => notifiedCount++);
261261

262-
await globalSettings.setInt(IntGlobalSetting.placeholderIgnore, null);
262+
await globalSettings.setInt(IntGlobalSetting.lastVisitedAccountId, null);
263263
check(notifiedCount).equals(1);
264264
});
265265

266266
test('10 to 100 -> does the update', () async {
267267
final globalSettings = eg.globalStore(intGlobalSettings: {
268-
IntGlobalSetting.placeholderIgnore: 10,
268+
IntGlobalSetting.lastVisitedAccountId: 10,
269269
}).settings;
270270
globalSettings.addListener(() => notifiedCount++);
271271

272-
await globalSettings.setInt(IntGlobalSetting.placeholderIgnore, 100);
272+
await globalSettings.setInt(IntGlobalSetting.lastVisitedAccountId, 100);
273273
check(notifiedCount).equals(1);
274274
});
275275
});

‎test/model/store_checks.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ extension GlobalStoreChecks on Subject<GlobalStore> {
4545
Subject<Iterable<int>> get accountIds => has((x) => x.accountIds, 'accountIds');
4646
Subject<Iterable<({ int accountId, Account account })>> get accountEntries => has((x) => x.accountEntries, 'accountEntries');
4747
Subject<Account?> getAccount(int id) => has((x) => x.getAccount(id), 'getAccount($id)');
48+
Subject<Account?> get lastVisitedAccount => has((x) => x.lastVisitedAccount, 'lastVisitedAccount');
4849
}
4950

5051
extension PerAccountStoreChecks on Subject<PerAccountStore> {

‎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;

0 commit comments

Comments
 (0)