From aa9f3e9b754cd7acd920bcc459247ae200d49158 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Mar 2025 16:03:29 -0700 Subject: [PATCH 1/9] settings [nfc]: Reorder GlobalSettingsStore members to group by setting As we add more settings, this will keep things better clustered logically together. --- lib/model/settings.dart | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/model/settings.dart b/lib/model/settings.dart index 44cc5262f3..0ebc18bc92 100644 --- a/lib/model/settings.dart +++ b/lib/model/settings.dart @@ -60,12 +60,23 @@ class GlobalSettingsStore extends ChangeNotifier { /// A cache of the [GlobalSettingsData] singleton in the underlying data store. GlobalSettingsData _data; + Future _update(GlobalSettingsCompanion data) async { + await _backend.doUpdateGlobalSettings(data); + _data = _data.copyWithCompanion(data); + notifyListeners(); + } + /// The user's choice of [ThemeSetting]; /// null means the device-level choice of theme. /// /// See also [setThemeSetting]. ThemeSetting? get themeSetting => _data.themeSetting; + /// Set [themeSetting], persistently for future runs of the app. + Future setThemeSetting(ThemeSetting? value) async { + await _update(GlobalSettingsCompanion(themeSetting: Value(value))); + } + /// The user's choice of [BrowserPreference]; /// null means use our default choice. /// @@ -74,6 +85,11 @@ class GlobalSettingsStore extends ChangeNotifier { /// See also [setBrowserPreference]. BrowserPreference? get browserPreference => _data.browserPreference; + /// Set [browserPreference], persistently for future runs of the app. + Future setBrowserPreference(BrowserPreference? value) async { + await _update(GlobalSettingsCompanion(browserPreference: Value(value))); + } + /// The value of [BrowserPreference] to use: /// the user's choice [browserPreference] if any, else our default. /// @@ -113,20 +129,4 @@ class GlobalSettingsStore extends ChangeNotifier { return UrlLaunchMode.externalApplication; } } - - Future _update(GlobalSettingsCompanion data) async { - await _backend.doUpdateGlobalSettings(data); - _data = _data.copyWithCompanion(data); - notifyListeners(); - } - - /// Set [themeSetting], persistently for future runs of the app. - Future setThemeSetting(ThemeSetting? value) async { - await _update(GlobalSettingsCompanion(themeSetting: Value(value))); - } - - /// Set [browserPreference], persistently for future runs of the app. - Future setBrowserPreference(BrowserPreference? value) async { - await _update(GlobalSettingsCompanion(browserPreference: Value(value))); - } } From 8a1c562742181d9a39e128ffdcd9fad8352affd7 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Mar 2025 14:17:57 -0700 Subject: [PATCH 2/9] log: Introduce profilePrint; use it for timing initial fetch --- lib/log.dart | 26 ++++++++++++++++++++++++++ lib/model/store.dart | 5 +++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/log.dart b/lib/log.dart index 60bcfb637b..5aa5348931 100644 --- a/lib/log.dart +++ b/lib/log.dart @@ -1,4 +1,6 @@ +import 'package:flutter/foundation.dart'; + /// Whether [debugLog] should do anything. /// /// This has an effect only in a debug build. @@ -31,6 +33,30 @@ bool debugLog(String message) { return true; } +/// Print a piece of profiling data. +/// +/// This should be called only in profile mode: +/// * In debug mode, any profiling results will be misleading. +/// * In release mode, we should avoid doing the computation to even produce +/// the [message] argument. +/// +/// As a reminder of that, this function will throw in debug mode. +/// +/// Example usage: +/// ```dart +/// final stopwatch = Stopwatch()..start(); +/// final data = await someSlowOperation(); +/// if (kProfileMode) { +/// final t = stopwatch.elapsed; +/// profilePrint("some-operation time: ${t.inMilliseconds}ms"); +/// } +/// ``` +void profilePrint(String message) { + assert(kProfileMode, 'Use profilePrint only within `if (kProfileMode)`.'); + if (kReleaseMode) return; + print(message); // ignore: avoid_print +} + // This should only be used for error reporting functions that allow the error // to be cancelled programmatically. The implementation is expected to handle // `null` for the `message` parameter and promptly dismiss the reported errors. diff --git a/lib/model/store.dart b/lib/model/store.dart index 3ca945157a..878dffba45 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -996,8 +996,9 @@ class UpdateMachine { final stopwatch = Stopwatch()..start(); final initialSnapshot = await _registerQueueWithRetry(connection, stopAndThrowIfNoAccount: stopAndThrowIfNoAccount); - final t = (stopwatch..stop()).elapsed; - assert(debugLog("initial fetch time: ${t.inMilliseconds}ms")); + if (kProfileMode) { + profilePrint("initial fetch time: ${stopwatch.elapsed.inMilliseconds}ms"); + } if (initialSnapshot.zulipVersion != account.zulipVersion || initialSnapshot.zulipMergeBase != account.zulipMergeBase From 2e054bf126a31d4ed2f6ec032bd9e8d0da42c845 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Mar 2025 13:40:29 -0700 Subject: [PATCH 3/9] db: Add profile-mode timings of load at startup As we add more features using the database and more queries we make from it here at the app's startup, this will help us check that we aren't unduly delaying startup. Results on my Pixel 8, with 2 account records: db load time 91.9ms total: 1.5ms init, 85.4ms settings, 5.0ms accounts db load time 87.8ms total: 1.7ms init, 78.1ms settings, 8.0ms accounts db load time 93.3ms total: 1.2ms init, 83.7ms settings, 8.5ms accounts db load time 78.1ms total: 1.4ms init, 71.8ms settings, 4.8ms accounts db load time 86.9ms total: 1.2ms init, 77.8ms settings, 7.9ms accounts --- lib/model/store.dart | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lib/model/store.dart b/lib/model/store.dart index 878dffba45..3df88ea608 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -882,9 +882,27 @@ class LiveGlobalStore extends GlobalStore { // We keep the API simple and synchronous for the bulk of the app's code // by doing this loading up front before constructing a [GlobalStore]. static Future load() async { + // Loading this data takes roughly 80-100ms (measured on a Pixel 8). + // That's only a small increment on the time spent loading server data, + // so we don't worry about optimizing it further. + // In a future where we keep server data locally between runs (#477) -- + // which will also mean having much more data to load from the database -- + // we'd invest in this area more. For example we'd try doing these + // in parallel, or deferring some to be concurrent with loading server data. + final stopwatch = Stopwatch()..start(); final db = AppDatabase(NativeDatabase.createInBackground(await _dbFile())); + final t1 = stopwatch.elapsed; final globalSettings = await db.getGlobalSettings(); + final t2 = stopwatch.elapsed; final accounts = await db.select(db.accounts).get(); + final t3 = stopwatch.elapsed; + if (kProfileMode) { + String format(Duration d) => + "${(d.inMicroseconds / 1000.0).toStringAsFixed(1)}ms"; + profilePrint("db load time ${format(t3)} total: ${format(t1)} init, " + "${format(t2 - t1)} settings, ${format(t3 - t2)} accounts"); + } + return LiveGlobalStore._( backend: LiveGlobalStoreBackend._(db: db), globalSettings: globalSettings, From 8f5df972d3926c82db59d37a67f3662ba43d08fe Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Mar 2025 15:56:24 -0700 Subject: [PATCH 4/9] settings test [nfc]: Use globalSettings as variable This aligns these tests more closely with how we expect most app code to interact with these settings (using GlobalStoreWidget.settingsOf). --- test/model/settings_test.dart | 30 +++++++++++++++--------------- test/model/store_test.dart | 14 +++++++------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/test/model/settings_test.dart b/test/model/settings_test.dart index 66f109158c..247777ba44 100644 --- a/test/model/settings_test.dart +++ b/test/model/settings_test.dart @@ -15,39 +15,39 @@ void main() { group('getUrlLaunchMode', () { testAndroidIos('globalSettings.browserPreference is null; use our per-platform defaults for HTTP links', () { - final globalStore = eg.globalStore(globalSettings: GlobalSettingsData( - browserPreference: null)); - check(globalStore).settings.getUrlLaunchMode(httpLink).equals( + final globalSettings = eg.globalStore(globalSettings: GlobalSettingsData( + browserPreference: null)).settings; + check(globalSettings).getUrlLaunchMode(httpLink).equals( defaultTargetPlatform == TargetPlatform.android ? UrlLaunchMode.inAppBrowserView : UrlLaunchMode.externalApplication); }); testAndroidIos('globalSettings.browserPreference is null; use our per-platform defaults for non-HTTP links', () { - final globalStore = eg.globalStore(globalSettings: GlobalSettingsData( - browserPreference: null)); - check(globalStore).settings.getUrlLaunchMode(nonHttpLink).equals( + final globalSettings = eg.globalStore(globalSettings: GlobalSettingsData( + browserPreference: null)).settings; + check(globalSettings).getUrlLaunchMode(nonHttpLink).equals( defaultTargetPlatform == TargetPlatform.android ? UrlLaunchMode.platformDefault : UrlLaunchMode.externalApplication); }); testAndroidIos('globalSettings.browserPreference is inApp; follow the user preference for http links', () { - final globalStore = eg.globalStore(globalSettings: GlobalSettingsData( - browserPreference: BrowserPreference.inApp)); - check(globalStore).settings.getUrlLaunchMode(httpLink).equals( + final globalSettings = eg.globalStore(globalSettings: GlobalSettingsData( + browserPreference: BrowserPreference.inApp)).settings; + check(globalSettings).getUrlLaunchMode(httpLink).equals( UrlLaunchMode.inAppBrowserView); }); testAndroidIos('globalSettings.browserPreference is inApp; use platform default for non-http links', () { - final globalStore = eg.globalStore(globalSettings: GlobalSettingsData( - browserPreference: BrowserPreference.inApp)); - check(globalStore).settings.getUrlLaunchMode(nonHttpLink).equals( + final globalSettings = eg.globalStore(globalSettings: GlobalSettingsData( + browserPreference: BrowserPreference.inApp)).settings; + check(globalSettings).getUrlLaunchMode(nonHttpLink).equals( UrlLaunchMode.platformDefault); }); testAndroidIos('globalSettings.browserPreference is external; follow the user preference', () { - final globalStore = eg.globalStore(globalSettings: GlobalSettingsData( - browserPreference: BrowserPreference.external)); - check(globalStore).settings.getUrlLaunchMode(httpLink).equals( + final globalSettings = eg.globalStore(globalSettings: GlobalSettingsData( + browserPreference: BrowserPreference.external)).settings; + check(globalSettings).getUrlLaunchMode(httpLink).equals( UrlLaunchMode.externalApplication); }); }); diff --git a/test/model/store_test.dart b/test/model/store_test.dart index a00194c8f8..e7d4a42864 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -33,20 +33,20 @@ void main() { group('GlobalStore.updateGlobalSettings', () { test('smoke', () async { - final globalStore = eg.globalStore(); - check(globalStore).settings.themeSetting.equals(null); + final globalSettings = eg.globalStore().settings; + check(globalSettings).themeSetting.equals(null); - await globalStore.settings.setThemeSetting(ThemeSetting.dark); - check(globalStore).settings.themeSetting.equals(ThemeSetting.dark); + await globalSettings.setThemeSetting(ThemeSetting.dark); + check(globalSettings).themeSetting.equals(ThemeSetting.dark); }); test('should notify listeners', () async { int notifyCount = 0; - final globalStore = eg.globalStore(); - globalStore.settings.addListener(() => notifyCount++); + final globalSettings = eg.globalStore().settings; + globalSettings.addListener(() => notifyCount++); check(notifyCount).equals(0); - await globalStore.settings.setThemeSetting(ThemeSetting.light); + await globalSettings.setThemeSetting(ThemeSetting.light); check(notifyCount).equals(1); }); From 9456d44aaa06ae5cf39cb9be01db794098ec16d6 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Mar 2025 16:00:39 -0700 Subject: [PATCH 5/9] settings test [nfc]: Move tests from store_test; rename This probably should have happened as part of 1cf31a44c and 9201ae42f, oh well. --- test/model/settings_test.dart | 22 ++++++++++++++++++++++ test/model/store_test.dart | 23 ----------------------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/test/model/settings_test.dart b/test/model/settings_test.dart index 247777ba44..3dae1c4be2 100644 --- a/test/model/settings_test.dart +++ b/test/model/settings_test.dart @@ -51,4 +51,26 @@ void main() { UrlLaunchMode.externalApplication); }); }); + + group('setThemeSetting', () { + test('smoke', () async { + final globalSettings = eg.globalStore().settings; + check(globalSettings).themeSetting.equals(null); + + await globalSettings.setThemeSetting(ThemeSetting.dark); + check(globalSettings).themeSetting.equals(ThemeSetting.dark); + }); + + test('should notify listeners', () async { + int notifyCount = 0; + final globalSettings = eg.globalStore().settings; + globalSettings.addListener(() => notifyCount++); + check(notifyCount).equals(0); + + await globalSettings.setThemeSetting(ThemeSetting.light); + check(notifyCount).equals(1); + }); + + // TODO integration tests with sqlite + }); } diff --git a/test/model/store_test.dart b/test/model/store_test.dart index e7d4a42864..4c83cd5da0 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -15,7 +15,6 @@ import 'package:zulip/api/route/messages.dart'; import 'package:zulip/api/route/realm.dart'; import 'package:zulip/log.dart'; import 'package:zulip/model/actions.dart'; -import 'package:zulip/model/settings.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/notifications/receive.dart'; @@ -31,28 +30,6 @@ import 'test_store.dart'; void main() { TestZulipBinding.ensureInitialized(); - group('GlobalStore.updateGlobalSettings', () { - test('smoke', () async { - final globalSettings = eg.globalStore().settings; - check(globalSettings).themeSetting.equals(null); - - await globalSettings.setThemeSetting(ThemeSetting.dark); - check(globalSettings).themeSetting.equals(ThemeSetting.dark); - }); - - test('should notify listeners', () async { - int notifyCount = 0; - final globalSettings = eg.globalStore().settings; - globalSettings.addListener(() => notifyCount++); - check(notifyCount).equals(0); - - await globalSettings.setThemeSetting(ThemeSetting.light); - check(notifyCount).equals(1); - }); - - // TODO integration tests with sqlite - }); - final account1 = eg.selfAccount.copyWith(id: 1); final account2 = eg.otherAccount.copyWith(id: 2); From 8f4155d20a31379af2229a53205044814b1d422e Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Mar 2025 16:35:58 -0700 Subject: [PATCH 6/9] db test [nfc]: Rename database local as "db" This is the same abbreviation we use in implementation code. Especially when the name occurs repeatedly in a line, this can make the code usefully more concise. --- test/model/database_test.dart | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/test/model/database_test.dart b/test/model/database_test.dart index 3d7e8f170d..7ab9669824 100644 --- a/test/model/database_test.dart +++ b/test/model/database_test.dart @@ -16,36 +16,34 @@ import 'store_checks.dart'; void main() { group('non-migration tests', () { - late AppDatabase database; + late AppDatabase db; setUp(() { - database = AppDatabase(NativeDatabase.memory()); + db = AppDatabase(NativeDatabase.memory()); }); tearDown(() async { - await database.close(); + await db.close(); }); test('initialize GlobalSettings with defaults', () async { - check(await database.getGlobalSettings()).themeSetting.isNull(); + check(await db.getGlobalSettings()).themeSetting.isNull(); }); test('does not crash if multiple global settings rows', () async { - await database.into(database.globalSettings) + await db.into(db.globalSettings) .insert(const GlobalSettingsCompanion(themeSetting: Value(ThemeSetting.dark))); - check(await database.select(database.globalSettings).get()).length.equals(2); - check(await database.getGlobalSettings()).themeSetting.isNull(); + check(await db.select(db.globalSettings).get()).length.equals(2); + check(await db.getGlobalSettings()).themeSetting.isNull(); }); test('GlobalSettings updates work', () async { - check(await database.getGlobalSettings()) - .themeSetting.isNull(); + check(await db.getGlobalSettings()).themeSetting.isNull(); // As in doUpdateGlobalSettings. - await database.update(database.globalSettings) + await db.update(db.globalSettings) .write(GlobalSettingsCompanion(themeSetting: Value(ThemeSetting.dark))); - check(await database.getGlobalSettings()) - .themeSetting.equals(ThemeSetting.dark); + check(await db.getGlobalSettings()).themeSetting.equals(ThemeSetting.dark); }); test('create account', () async { @@ -58,8 +56,8 @@ void main() { zulipMergeBase: const Value('6.0'), zulipFeatureLevel: 42, ); - final accountId = await database.createAccount(accountData); - final account = await (database.select(database.accounts) + final accountId = await db.createAccount(accountData); + final account = await (db.select(db.accounts) ..where((a) => a.id.equals(accountId))) .watchSingle() .first; @@ -89,8 +87,8 @@ void main() { zulipMergeBase: const Value('6.0'), zulipFeatureLevel: 42, ); - await database.createAccount(accountData); - await check(database.createAccount(accountDataWithSameUserId)) + await db.createAccount(accountData); + await check(db.createAccount(accountDataWithSameUserId)) .throws(); }); @@ -113,8 +111,8 @@ void main() { zulipMergeBase: const Value('6.0'), zulipFeatureLevel: 42, ); - await database.createAccount(accountData); - await check(database.createAccount(accountDataWithSameEmail)) + await db.createAccount(accountData); + await check(db.createAccount(accountDataWithSameEmail)) .throws(); }); }); From 5cfd4ae48347c7b9c8377f6b8e82355bf72c9056 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sun, 16 Mar 2025 21:17:57 -0700 Subject: [PATCH 7/9] settings: Make general bool global settings, so as to add without migrations This new table to read looks to add something like 5-8ms to the app's startup time. Not nothing; but not a big cost compared to the time we spend loading server data right afterward. Specifically, results on my Pixel 8 (compare to those in the previous commit): db load time 117.9ms total: 15.2ms init, 89.2ms settings, 5.1ms bool-settings, 8.3ms accounts db load time 90.9ms total: 1.3ms init, 78.5ms settings, 8.3ms bool-settings, 2.8ms accounts db load time 87.2ms total: 1.4ms init, 71.8ms settings, 5.9ms bool-settings, 8.1ms accounts db load time 85.7ms total: 1.2ms init, 72.8ms settings, 4.9ms bool-settings, 6.8ms accounts db load time 91.3ms total: 1.4ms init, 80.0ms settings, 7.5ms bool-settings, 2.5ms accounts db load time 83.8ms total: 1.1ms init, 70.0ms settings, 5.0ms bool-settings, 7.7ms accounts --- lib/model/database.dart | 56 +- lib/model/database.g.dart | 403 +++++++++++ lib/model/schema_versions.g.dart | 89 +++ lib/model/settings.dart | 91 ++- lib/model/store.dart | 32 +- test/example_data.dart | 8 +- test/model/database_test.dart | 67 ++ test/model/schemas/drift_schema_v6.json | 1 + test/model/schemas/schema.dart | 5 +- test/model/schemas/schema_v6.dart | 890 ++++++++++++++++++++++++ test/model/settings_test.dart | 44 ++ test/model/store_checks.dart | 1 + test/model/test_store.dart | 16 +- 13 files changed, 1692 insertions(+), 11 deletions(-) create mode 100644 test/model/schemas/drift_schema_v6.json create mode 100644 test/model/schemas/schema_v6.dart diff --git a/lib/model/database.dart b/lib/model/database.dart index ed6cba5f74..57910e7a50 100644 --- a/lib/model/database.dart +++ b/lib/model/database.dart @@ -23,6 +23,44 @@ class GlobalSettings extends Table { Column get browserPreference => textEnum() .nullable()(); + + // If adding a new column to this table, consider whether [BoolGlobalSettings] + // can do the job instead (by adding a value to the [BoolGlobalSetting] enum). + // That way is more convenient, when it works, because + // it avoids a migration and therefore several added copies of our schema + // in the Drift generated files. +} + +/// The table of the user's bool-valued, account-independent settings. +/// +/// These apply across all the user's accounts on this client +/// (i.e. on this install of the app on this device). +/// +/// Each row is a [BoolGlobalSettingRow], +/// referring to a possible setting from [BoolGlobalSetting]. +/// For settings in [BoolGlobalSetting] without a row in this table, +/// the setting's value is that of [BoolGlobalSetting.default_]. +@DataClassName('BoolGlobalSettingRow') +class BoolGlobalSettings extends Table { + /// The setting's name, a possible name from [BoolGlobalSetting]. + /// + /// The table may have rows where [name] is not the name of any + /// enum value in [BoolGlobalSetting]. + /// This happens if the app has previously run at a future or modified + /// version which had additional values in that enum, + /// and the user set one of those additional settings. + /// The app ignores any such unknown rows. + Column get name => text()(); + + /// The user's chosen value for the setting. + /// + /// This is non-nullable; if the user wants to revert to + /// following the app's default for the setting, + /// that can be expressed by deleting the row. + Column get value => boolean()(); + + @override + Set>? get primaryKey => {name}; } /// The table of [Account] records in the app's database. @@ -68,7 +106,7 @@ class UriConverter extends TypeConverter { @override Uri fromSql(String fromDb) => Uri.parse(fromDb); } -@DriftDatabase(tables: [GlobalSettings, Accounts]) +@DriftDatabase(tables: [GlobalSettings, BoolGlobalSettings, Accounts]) class AppDatabase extends _$AppDatabase { AppDatabase(super.e); @@ -81,7 +119,7 @@ class AppDatabase extends _$AppDatabase { // information on using the build_runner. // * Write a migration in `_migrationSteps` below. // * Write tests. - static const int latestSchemaVersion = 5; // See note. + static const int latestSchemaVersion = 6; // See note. @override int get schemaVersion => latestSchemaVersion; @@ -133,6 +171,9 @@ class AppDatabase extends _$AppDatabase { RawValuesInsertable({})); } }, + from5To6: (m, schema) async { + await m.createTable(schema.boolGlobalSettings); + }, ); Future _createLatestSchema(Migrator m) async { @@ -173,6 +214,17 @@ class AppDatabase extends _$AppDatabase { return await (select(globalSettings)..limit(1)).getSingle(); } + Future> getBoolGlobalSettings() async { + final result = {}; + final rows = await select(boolGlobalSettings).get(); + for (final row in rows) { + final setting = BoolGlobalSetting.byName(row.name); + if (setting == null) continue; + result[setting] = row.value; + } + return result; + } + Future createAccount(AccountsCompanion values) async { try { return await into(accounts).insert(values); diff --git a/lib/model/database.g.dart b/lib/model/database.g.dart index 9c0946af5b..99752bdd62 100644 --- a/lib/model/database.g.dart +++ b/lib/model/database.g.dart @@ -259,6 +259,235 @@ class GlobalSettingsCompanion extends UpdateCompanion { } } +class $BoolGlobalSettingsTable extends BoolGlobalSettings + with TableInfo<$BoolGlobalSettingsTable, BoolGlobalSettingRow> { + @override + final GeneratedDatabase attachedDatabase; + final String? _alias; + $BoolGlobalSettingsTable(this.attachedDatabase, [this._alias]); + static const VerificationMeta _nameMeta = const VerificationMeta('name'); + @override + late final GeneratedColumn name = GeneratedColumn( + 'name', + aliasedName, + false, + type: DriftSqlType.string, + requiredDuringInsert: true, + ); + static const VerificationMeta _valueMeta = const VerificationMeta('value'); + @override + late final GeneratedColumn value = GeneratedColumn( + 'value', + aliasedName, + false, + type: DriftSqlType.bool, + requiredDuringInsert: true, + defaultConstraints: GeneratedColumn.constraintIsAlways( + 'CHECK ("value" IN (0, 1))', + ), + ); + @override + List get $columns => [name, value]; + @override + String get aliasedName => _alias ?? actualTableName; + @override + String get actualTableName => $name; + static const String $name = 'bool_global_settings'; + @override + VerificationContext validateIntegrity( + Insertable instance, { + bool isInserting = false, + }) { + final context = VerificationContext(); + final data = instance.toColumns(true); + if (data.containsKey('name')) { + context.handle( + _nameMeta, + name.isAcceptableOrUnknown(data['name']!, _nameMeta), + ); + } else if (isInserting) { + context.missing(_nameMeta); + } + if (data.containsKey('value')) { + context.handle( + _valueMeta, + value.isAcceptableOrUnknown(data['value']!, _valueMeta), + ); + } else if (isInserting) { + context.missing(_valueMeta); + } + return context; + } + + @override + Set get $primaryKey => {name}; + @override + BoolGlobalSettingRow map(Map data, {String? tablePrefix}) { + final effectivePrefix = tablePrefix != null ? '$tablePrefix.' : ''; + return BoolGlobalSettingRow( + name: + attachedDatabase.typeMapping.read( + DriftSqlType.string, + data['${effectivePrefix}name'], + )!, + value: + attachedDatabase.typeMapping.read( + DriftSqlType.bool, + data['${effectivePrefix}value'], + )!, + ); + } + + @override + $BoolGlobalSettingsTable createAlias(String alias) { + return $BoolGlobalSettingsTable(attachedDatabase, alias); + } +} + +class BoolGlobalSettingRow extends DataClass + implements Insertable { + /// The setting's name, a possible name from [BoolGlobalSetting]. + /// + /// The table may have rows where [name] is not the name of any + /// enum value in [BoolGlobalSetting]. + /// This happens if the app has previously run at a future or modified + /// version which had additional values in that enum, + /// and the user set one of those additional settings. + /// The app ignores any such unknown rows. + final String name; + + /// The user's chosen value for the setting. + /// + /// This is non-nullable; if the user wants to revert to + /// following the app's default for the setting, + /// that can be expressed by deleting the row. + final bool value; + const BoolGlobalSettingRow({required this.name, required this.value}); + @override + Map toColumns(bool nullToAbsent) { + final map = {}; + map['name'] = Variable(name); + map['value'] = Variable(value); + return map; + } + + BoolGlobalSettingsCompanion toCompanion(bool nullToAbsent) { + return BoolGlobalSettingsCompanion(name: Value(name), value: Value(value)); + } + + factory BoolGlobalSettingRow.fromJson( + Map json, { + ValueSerializer? serializer, + }) { + serializer ??= driftRuntimeOptions.defaultSerializer; + return BoolGlobalSettingRow( + name: serializer.fromJson(json['name']), + value: serializer.fromJson(json['value']), + ); + } + @override + Map toJson({ValueSerializer? serializer}) { + serializer ??= driftRuntimeOptions.defaultSerializer; + return { + 'name': serializer.toJson(name), + 'value': serializer.toJson(value), + }; + } + + BoolGlobalSettingRow copyWith({String? name, bool? value}) => + BoolGlobalSettingRow(name: name ?? this.name, value: value ?? this.value); + BoolGlobalSettingRow copyWithCompanion(BoolGlobalSettingsCompanion data) { + return BoolGlobalSettingRow( + name: data.name.present ? data.name.value : this.name, + value: data.value.present ? data.value.value : this.value, + ); + } + + @override + String toString() { + return (StringBuffer('BoolGlobalSettingRow(') + ..write('name: $name, ') + ..write('value: $value') + ..write(')')) + .toString(); + } + + @override + int get hashCode => Object.hash(name, value); + @override + bool operator ==(Object other) => + identical(this, other) || + (other is BoolGlobalSettingRow && + other.name == this.name && + other.value == this.value); +} + +class BoolGlobalSettingsCompanion + extends UpdateCompanion { + final Value name; + final Value value; + final Value rowid; + const BoolGlobalSettingsCompanion({ + this.name = const Value.absent(), + this.value = const Value.absent(), + this.rowid = const Value.absent(), + }); + BoolGlobalSettingsCompanion.insert({ + required String name, + required bool value, + this.rowid = const Value.absent(), + }) : name = Value(name), + value = Value(value); + static Insertable custom({ + Expression? name, + Expression? value, + Expression? rowid, + }) { + return RawValuesInsertable({ + if (name != null) 'name': name, + if (value != null) 'value': value, + if (rowid != null) 'rowid': rowid, + }); + } + + BoolGlobalSettingsCompanion copyWith({ + Value? name, + Value? value, + Value? rowid, + }) { + return BoolGlobalSettingsCompanion( + name: name ?? this.name, + value: value ?? this.value, + rowid: rowid ?? this.rowid, + ); + } + + @override + Map toColumns(bool nullToAbsent) { + final map = {}; + if (name.present) { + map['name'] = Variable(name.value); + } + if (value.present) { + map['value'] = Variable(value.value); + } + if (rowid.present) { + map['rowid'] = Variable(rowid.value); + } + return map; + } + + @override + String toString() { + return (StringBuffer('BoolGlobalSettingsCompanion(') + ..write('name: $name, ') + ..write('value: $value, ') + ..write('rowid: $rowid') + ..write(')')) + .toString(); + } +} + class $AccountsTable extends Accounts with TableInfo<$AccountsTable, Account> { @override final GeneratedDatabase attachedDatabase; @@ -862,6 +1091,8 @@ abstract class _$AppDatabase extends GeneratedDatabase { _$AppDatabase(QueryExecutor e) : super(e); $AppDatabaseManager get managers => $AppDatabaseManager(this); late final $GlobalSettingsTable globalSettings = $GlobalSettingsTable(this); + late final $BoolGlobalSettingsTable boolGlobalSettings = + $BoolGlobalSettingsTable(this); late final $AccountsTable accounts = $AccountsTable(this); @override Iterable> get allTables => @@ -869,6 +1100,7 @@ abstract class _$AppDatabase extends GeneratedDatabase { @override List get allSchemaEntities => [ globalSettings, + boolGlobalSettings, accounts, ]; } @@ -1043,6 +1275,175 @@ typedef $$GlobalSettingsTableProcessedTableManager = GlobalSettingsData, PrefetchHooks Function() >; +typedef $$BoolGlobalSettingsTableCreateCompanionBuilder = + BoolGlobalSettingsCompanion Function({ + required String name, + required bool value, + Value rowid, + }); +typedef $$BoolGlobalSettingsTableUpdateCompanionBuilder = + BoolGlobalSettingsCompanion Function({ + Value name, + Value value, + Value rowid, + }); + +class $$BoolGlobalSettingsTableFilterComposer + extends Composer<_$AppDatabase, $BoolGlobalSettingsTable> { + $$BoolGlobalSettingsTableFilterComposer({ + required super.$db, + required super.$table, + super.joinBuilder, + super.$addJoinBuilderToRootComposer, + super.$removeJoinBuilderFromRootComposer, + }); + ColumnFilters get name => $composableBuilder( + column: $table.name, + builder: (column) => ColumnFilters(column), + ); + + ColumnFilters get value => $composableBuilder( + column: $table.value, + builder: (column) => ColumnFilters(column), + ); +} + +class $$BoolGlobalSettingsTableOrderingComposer + extends Composer<_$AppDatabase, $BoolGlobalSettingsTable> { + $$BoolGlobalSettingsTableOrderingComposer({ + required super.$db, + required super.$table, + super.joinBuilder, + super.$addJoinBuilderToRootComposer, + super.$removeJoinBuilderFromRootComposer, + }); + ColumnOrderings get name => $composableBuilder( + column: $table.name, + builder: (column) => ColumnOrderings(column), + ); + + ColumnOrderings get value => $composableBuilder( + column: $table.value, + builder: (column) => ColumnOrderings(column), + ); +} + +class $$BoolGlobalSettingsTableAnnotationComposer + extends Composer<_$AppDatabase, $BoolGlobalSettingsTable> { + $$BoolGlobalSettingsTableAnnotationComposer({ + required super.$db, + required super.$table, + super.joinBuilder, + super.$addJoinBuilderToRootComposer, + super.$removeJoinBuilderFromRootComposer, + }); + GeneratedColumn get name => + $composableBuilder(column: $table.name, builder: (column) => column); + + GeneratedColumn get value => + $composableBuilder(column: $table.value, builder: (column) => column); +} + +class $$BoolGlobalSettingsTableTableManager + extends + RootTableManager< + _$AppDatabase, + $BoolGlobalSettingsTable, + BoolGlobalSettingRow, + $$BoolGlobalSettingsTableFilterComposer, + $$BoolGlobalSettingsTableOrderingComposer, + $$BoolGlobalSettingsTableAnnotationComposer, + $$BoolGlobalSettingsTableCreateCompanionBuilder, + $$BoolGlobalSettingsTableUpdateCompanionBuilder, + ( + BoolGlobalSettingRow, + BaseReferences< + _$AppDatabase, + $BoolGlobalSettingsTable, + BoolGlobalSettingRow + >, + ), + BoolGlobalSettingRow, + PrefetchHooks Function() + > { + $$BoolGlobalSettingsTableTableManager( + _$AppDatabase db, + $BoolGlobalSettingsTable table, + ) : super( + TableManagerState( + db: db, + table: table, + createFilteringComposer: + () => $$BoolGlobalSettingsTableFilterComposer( + $db: db, + $table: table, + ), + createOrderingComposer: + () => $$BoolGlobalSettingsTableOrderingComposer( + $db: db, + $table: table, + ), + createComputedFieldComposer: + () => $$BoolGlobalSettingsTableAnnotationComposer( + $db: db, + $table: table, + ), + updateCompanionCallback: + ({ + Value name = const Value.absent(), + Value value = const Value.absent(), + Value rowid = const Value.absent(), + }) => BoolGlobalSettingsCompanion( + name: name, + value: value, + rowid: rowid, + ), + createCompanionCallback: + ({ + required String name, + required bool value, + Value rowid = const Value.absent(), + }) => BoolGlobalSettingsCompanion.insert( + name: name, + value: value, + rowid: rowid, + ), + withReferenceMapper: + (p0) => + p0 + .map( + (e) => ( + e.readTable(table), + BaseReferences(db, table, e), + ), + ) + .toList(), + prefetchHooksCallback: null, + ), + ); +} + +typedef $$BoolGlobalSettingsTableProcessedTableManager = + ProcessedTableManager< + _$AppDatabase, + $BoolGlobalSettingsTable, + BoolGlobalSettingRow, + $$BoolGlobalSettingsTableFilterComposer, + $$BoolGlobalSettingsTableOrderingComposer, + $$BoolGlobalSettingsTableAnnotationComposer, + $$BoolGlobalSettingsTableCreateCompanionBuilder, + $$BoolGlobalSettingsTableUpdateCompanionBuilder, + ( + BoolGlobalSettingRow, + BaseReferences< + _$AppDatabase, + $BoolGlobalSettingsTable, + BoolGlobalSettingRow + >, + ), + BoolGlobalSettingRow, + PrefetchHooks Function() + >; typedef $$AccountsTableCreateCompanionBuilder = AccountsCompanion Function({ Value id, @@ -1329,6 +1730,8 @@ class $AppDatabaseManager { $AppDatabaseManager(this._db); $$GlobalSettingsTableTableManager get globalSettings => $$GlobalSettingsTableTableManager(_db, _db.globalSettings); + $$BoolGlobalSettingsTableTableManager get boolGlobalSettings => + $$BoolGlobalSettingsTableTableManager(_db, _db.boolGlobalSettings); $$AccountsTableTableManager get accounts => $$AccountsTableTableManager(_db, _db.accounts); } diff --git a/lib/model/schema_versions.g.dart b/lib/model/schema_versions.g.dart index 82ffcb7f00..9bfa74f627 100644 --- a/lib/model/schema_versions.g.dart +++ b/lib/model/schema_versions.g.dart @@ -286,11 +286,93 @@ final class Schema5 extends i0.VersionedSchema { ); } +final class Schema6 extends i0.VersionedSchema { + Schema6({required super.database}) : super(version: 6); + @override + late final List entities = [ + globalSettings, + boolGlobalSettings, + accounts, + ]; + late final Shape2 globalSettings = Shape2( + source: i0.VersionedTable( + entityName: 'global_settings', + withoutRowId: false, + isStrict: false, + tableConstraints: [], + columns: [_column_9, _column_10], + attachedDatabase: database, + ), + alias: null, + ); + late final Shape3 boolGlobalSettings = Shape3( + source: i0.VersionedTable( + entityName: 'bool_global_settings', + withoutRowId: false, + isStrict: false, + tableConstraints: ['PRIMARY KEY(name)'], + columns: [_column_11, _column_12], + attachedDatabase: database, + ), + alias: null, + ); + late final Shape0 accounts = Shape0( + source: i0.VersionedTable( + entityName: 'accounts', + withoutRowId: false, + isStrict: false, + tableConstraints: [ + 'UNIQUE(realm_url, user_id)', + 'UNIQUE(realm_url, email)', + ], + columns: [ + _column_0, + _column_1, + _column_2, + _column_3, + _column_4, + _column_5, + _column_6, + _column_7, + _column_8, + ], + attachedDatabase: database, + ), + alias: null, + ); +} + +class Shape3 extends i0.VersionedTable { + Shape3({required super.source, required super.alias}) : super.aliased(); + i1.GeneratedColumn get name => + columnsByName['name']! as i1.GeneratedColumn; + i1.GeneratedColumn get value => + columnsByName['value']! as i1.GeneratedColumn; +} + +i1.GeneratedColumn _column_11(String aliasedName) => + i1.GeneratedColumn( + 'name', + aliasedName, + false, + type: i1.DriftSqlType.string, + ); +i1.GeneratedColumn _column_12(String aliasedName) => + i1.GeneratedColumn( + 'value', + aliasedName, + false, + type: i1.DriftSqlType.bool, + defaultConstraints: i1.GeneratedColumn.constraintIsAlways( + 'CHECK ("value" IN (0, 1))', + ), + ); i0.MigrationStepWithVersion migrationSteps({ required Future Function(i1.Migrator m, Schema2 schema) from1To2, required Future Function(i1.Migrator m, Schema3 schema) from2To3, required Future Function(i1.Migrator m, Schema4 schema) from3To4, required Future Function(i1.Migrator m, Schema5 schema) from4To5, + required Future Function(i1.Migrator m, Schema6 schema) from5To6, }) { return (currentVersion, database) async { switch (currentVersion) { @@ -314,6 +396,11 @@ i0.MigrationStepWithVersion migrationSteps({ final migrator = i1.Migrator(database, schema); await from4To5(migrator, schema); return 5; + case 5: + final schema = Schema6(database: database); + final migrator = i1.Migrator(database, schema); + await from5To6(migrator, schema); + return 6; default: throw ArgumentError.value('Unknown migration from $currentVersion'); } @@ -325,11 +412,13 @@ i1.OnUpgrade stepByStep({ required Future Function(i1.Migrator m, Schema3 schema) from2To3, required Future Function(i1.Migrator m, Schema4 schema) from3To4, required Future Function(i1.Migrator m, Schema5 schema) from4To5, + required Future Function(i1.Migrator m, Schema6 schema) from5To6, }) => i0.VersionedSchema.stepByStepHelper( step: migrationSteps( from1To2: from1To2, from2To3: from2To3, from3To4: from3To4, from4To5: from4To5, + from5To6: from5To6, ), ); diff --git a/lib/model/settings.dart b/lib/model/settings.dart index 0ebc18bc92..45edda115d 100644 --- a/lib/model/settings.dart +++ b/lib/model/settings.dart @@ -45,6 +45,67 @@ enum BrowserPreference { external, } +/// A general category of account-independent setting the user might set. +/// +/// Different kinds of settings call for different treatment in the UI, +/// and different expected lifecycles as the app evolves. +enum GlobalSettingType { + /// Describes a non-setting which exists to avoid an empty enum. + /// + /// A Dart enum must have at least one value. + /// But in steady state we expect to have no experimental feature flags. + /// To allow [BoolGlobalSetting] to continue to exist in that situation + /// (so that it stands ready to accept a future feature flag), + /// we give it a placeholder value which isn't a real setting. + placeholder, + ; +} + +/// A bool-valued, account-independent setting the user might set. +/// +/// These are recorded in the table [BoolGlobalSettings]. +/// To read the value of one of these settings, use [GlobalSettingsStore.getBool]; +/// to set the value, use [GlobalSettingsStore.setBool]. +/// +/// To introduce a new setting, add a value to this enum. +/// Avoid re-using any old names found in the "former settings" list. +/// +/// To remove a setting, comment it out and move to the "former settings" list. +/// Tracking the names of settings that formerly existed is important because +/// they may still appear in users' databases, which means that if we were to +/// accidentally reuse one for an unrelated new setting then users would +/// unwittingly get those values applied to the new setting, +/// which could cause very confusing buggy behavior. +/// +/// (If the list of former settings gets long, we could do a migration to clear +/// them from existing installs, and then drop the list. We don't do that +/// eagerly each time, to avoid creating a new schema version each time we +/// finish an experimental feature.) +enum BoolGlobalSetting { + /// A non-setting to ensure this enum has at least one value. + placeholderIgnore(GlobalSettingType.placeholder, false), + + // Former settings which might exist in the database, + // whose names should therefore not be reused: + // (this list is empty so far) + ; + + const BoolGlobalSetting(this.type, this.default_); + + /// The general category of setting that this setting belongs to. + final GlobalSettingType type; + + /// The value the setting effectively has if the user hasn't chosen a value. + final bool default_; + + static BoolGlobalSetting? byName(String name) => _byName[name]; + + static final Map _byName = { + for (final v in values) + v.name: v, + }; +} + /// Store for the user's account-independent settings. /// /// From UI code, use [GlobalStoreWidget.settingsOf] to get hold of @@ -53,7 +114,8 @@ class GlobalSettingsStore extends ChangeNotifier { GlobalSettingsStore({ required GlobalStoreBackend backend, required GlobalSettingsData data, - }) : _backend = backend, _data = data; + required Map boolData, + }) : _backend = backend, _data = data, _boolData = boolData; final GlobalStoreBackend _backend; @@ -129,4 +191,31 @@ class GlobalSettingsStore extends ChangeNotifier { return UrlLaunchMode.externalApplication; } } + + /// The user's choice of the given bool-valued setting, or our default for it. + /// + /// See also [setBool]. + bool getBool(BoolGlobalSetting setting) { + return _boolData[setting] ?? setting.default_; + } + + /// A cache of the [BoolGlobalSettings] table in the underlying data store. + final Map _boolData; + + /// Set or unset the given bool-valued setting, + /// persistently for future runs of the app. + /// + /// A value of null means the setting will revert to following + /// the app's default. + /// + /// See also [getBool]. + Future setBool(BoolGlobalSetting setting, bool? value) async { + await _backend.doSetBoolGlobalSetting(setting, value); + if (value == null) { + _boolData.remove(setting); + } else { + _boolData[setting] = value; + } + notifyListeners(); + } } diff --git a/lib/model/store.dart b/lib/model/store.dart index 3df88ea608..c3aaa501db 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -49,6 +49,11 @@ abstract class GlobalStoreBackend { /// This should only be called from [GlobalSettingsStore]. Future doUpdateGlobalSettings(GlobalSettingsCompanion data); + /// Set or unset the given bool-valued setting in the underlying data store. + /// + /// This should only be called from [GlobalSettingsStore]. + Future doSetBoolGlobalSetting(BoolGlobalSetting setting, bool? value); + // TODO move here the similar methods for accounts; // perhaps the rest of the GlobalStore abstract methods, too. } @@ -73,9 +78,11 @@ abstract class GlobalStore extends ChangeNotifier { GlobalStore({ required GlobalStoreBackend backend, required GlobalSettingsData globalSettings, + required Map boolGlobalSettings, required Iterable accounts, }) - : settings = GlobalSettingsStore(backend: backend, data: globalSettings), + : settings = GlobalSettingsStore(backend: backend, + data: globalSettings, boolData: boolGlobalSettings), _accounts = Map.fromEntries(accounts.map((a) => MapEntry(a.id, a))); /// The store for the user's account-independent settings. @@ -853,6 +860,18 @@ class LiveGlobalStoreBackend implements GlobalStoreBackend { final rowsAffected = await _db.update(_db.globalSettings).write(data); assert(rowsAffected == 1); } + + @override + Future doSetBoolGlobalSetting(BoolGlobalSetting setting, bool? value) async { + if (value == null) { + final query = _db.delete(_db.boolGlobalSettings) + ..where((r) => r.name.equals(setting.name)); + await query.go(); + } else { + await _db.into(_db.boolGlobalSettings).insertOnConflictUpdate( + BoolGlobalSettingRow(name: setting.name, value: value)); + } + } } /// A [GlobalStore] that uses a live server and live, persistent local database. @@ -866,6 +885,7 @@ class LiveGlobalStore extends GlobalStore { LiveGlobalStore._({ required LiveGlobalStoreBackend backend, required super.globalSettings, + required super.boolGlobalSettings, required super.accounts, }) : _backend = backend, super(backend: backend); @@ -894,18 +914,22 @@ class LiveGlobalStore extends GlobalStore { final t1 = stopwatch.elapsed; final globalSettings = await db.getGlobalSettings(); final t2 = stopwatch.elapsed; - final accounts = await db.select(db.accounts).get(); + final boolGlobalSettings = await db.getBoolGlobalSettings(); final t3 = stopwatch.elapsed; + final accounts = await db.select(db.accounts).get(); + final t4 = stopwatch.elapsed; if (kProfileMode) { String format(Duration d) => "${(d.inMicroseconds / 1000.0).toStringAsFixed(1)}ms"; - profilePrint("db load time ${format(t3)} total: ${format(t1)} init, " - "${format(t2 - t1)} settings, ${format(t3 - t2)} accounts"); + profilePrint("db load time ${format(t4)} total: ${format(t1)} init, " + "${format(t2 - t1)} settings, ${format(t3 - t2)} bool-settings, " + "${format(t4 - t3)} accounts"); } return LiveGlobalStore._( backend: LiveGlobalStoreBackend._(db: db), globalSettings: globalSettings, + boolGlobalSettings: boolGlobalSettings, accounts: accounts); } diff --git a/test/example_data.dart b/test/example_data.dart index c2ffcae660..d44849bc6a 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -11,6 +11,7 @@ import 'package:zulip/api/route/realm.dart'; import 'package:zulip/api/route/channels.dart'; import 'package:zulip/model/database.dart'; import 'package:zulip/model/narrow.dart'; +import 'package:zulip/model/settings.dart'; import 'package:zulip/model/store.dart'; import 'model/test_store.dart'; @@ -875,9 +876,14 @@ ChannelUpdateEvent channelUpdateEvent( TestGlobalStore globalStore({ GlobalSettingsData? globalSettings, + Map? boolGlobalSettings, List accounts = const [], }) { - return TestGlobalStore(globalSettings: globalSettings, accounts: accounts); + return TestGlobalStore( + globalSettings: globalSettings, + boolGlobalSettings: boolGlobalSettings, + accounts: accounts, + ); } const _globalStore = globalStore; diff --git a/test/model/database_test.dart b/test/model/database_test.dart index 7ab9669824..e6e2b729be 100644 --- a/test/model/database_test.dart +++ b/test/model/database_test.dart @@ -46,6 +46,73 @@ void main() { check(await db.getGlobalSettings()).themeSetting.equals(ThemeSetting.dark); }); + test('BoolGlobalSettings get ignores unknown names', () async { + await db.into(db.boolGlobalSettings) + .insert(BoolGlobalSettingRow(name: 'nonsense', value: true)); + check(await db.getBoolGlobalSettings()).isEmpty(); + + final setting = BoolGlobalSetting.placeholderIgnore; + await db.into(db.boolGlobalSettings) + .insert(BoolGlobalSettingRow(name: setting.name, value: true)); + check(await db.getBoolGlobalSettings()) + .deepEquals({setting: true}); + }); + + test('BoolGlobalSettings insert, then get', () async { + check(await db.getBoolGlobalSettings()).isEmpty(); + + // As in doSetBoolGlobalSetting for `value` non-null. + final setting = BoolGlobalSetting.placeholderIgnore; + await db.into(db.boolGlobalSettings).insertOnConflictUpdate( + BoolGlobalSettingRow(name: setting.name, value: true)); + check(await db.getBoolGlobalSettings()) + .deepEquals({setting: true}); + check(await db.select(db.boolGlobalSettings).get()).length.equals(1); + }); + + test('BoolGlobalSettings delete, then get', () async { + final setting = BoolGlobalSetting.placeholderIgnore; + await db.into(db.boolGlobalSettings).insertOnConflictUpdate( + BoolGlobalSettingRow(name: setting.name, value: true)); + check(await db.getBoolGlobalSettings()) + .deepEquals({setting: true}); + + // As in doSetBoolGlobalSetting for `value` null. + final query = db.delete(db.boolGlobalSettings) + ..where((r) => r.name.equals(setting.name)); + await query.go(); + check(await db.getBoolGlobalSettings()).isEmpty(); + check(await db.select(db.boolGlobalSettings).get()).isEmpty(); + }); + + test('BoolGlobalSettings insert replaces', () async { + final setting = BoolGlobalSetting.placeholderIgnore; + await db.into(db.boolGlobalSettings).insertOnConflictUpdate( + BoolGlobalSettingRow(name: setting.name, value: true)); + check(await db.getBoolGlobalSettings()) + .deepEquals({setting: true}); + + // As in doSetBoolGlobalSetting for `value` non-null. + await db.into(db.boolGlobalSettings).insertOnConflictUpdate( + BoolGlobalSettingRow(name: setting.name, value: false)); + check(await db.getBoolGlobalSettings()) + .deepEquals({setting: false}); + check(await db.select(db.boolGlobalSettings).get()).length.equals(1); + }); + + test('BoolGlobalSettings delete is idempotent', () async { + check(await db.getBoolGlobalSettings()).isEmpty(); + + // As in doSetBoolGlobalSetting for `value` null. + final setting = BoolGlobalSetting.placeholderIgnore; + final query = db.delete(db.boolGlobalSettings) + ..where((r) => r.name.equals(setting.name)); + await query.go(); + // (No error occurred, even though there was nothing to delete.) + check(await db.getBoolGlobalSettings()).isEmpty(); + check(await db.select(db.boolGlobalSettings).get()).isEmpty(); + }); + test('create account', () async { final accountData = AccountsCompanion.insert( realmUrl: Uri.parse('https://chat.example/'), diff --git a/test/model/schemas/drift_schema_v6.json b/test/model/schemas/drift_schema_v6.json new file mode 100644 index 0000000000..807c36c934 --- /dev/null +++ b/test/model/schemas/drift_schema_v6.json @@ -0,0 +1 @@ +{"_meta":{"description":"This file contains a serialized version of schema entities for drift.","version":"1.2.0"},"options":{"store_date_time_values_as_text":false},"entities":[{"id":0,"references":[],"type":"table","data":{"name":"global_settings","was_declared_in_moor":false,"columns":[{"name":"theme_setting","getter_name":"themeSetting","moor_type":"string","nullable":true,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[],"type_converter":{"dart_expr":"const EnumNameConverter(ThemeSetting.values)","dart_type_name":"ThemeSetting"}},{"name":"browser_preference","getter_name":"browserPreference","moor_type":"string","nullable":true,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[],"type_converter":{"dart_expr":"const EnumNameConverter(BrowserPreference.values)","dart_type_name":"BrowserPreference"}}],"is_virtual":false,"without_rowid":false,"constraints":[]}},{"id":1,"references":[],"type":"table","data":{"name":"bool_global_settings","was_declared_in_moor":false,"columns":[{"name":"name","getter_name":"name","moor_type":"string","nullable":false,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[]},{"name":"value","getter_name":"value","moor_type":"bool","nullable":false,"customConstraints":null,"defaultConstraints":"CHECK (\"value\" IN (0, 1))","dialectAwareDefaultConstraints":{"sqlite":"CHECK (\"value\" IN (0, 1))"},"default_dart":null,"default_client_dart":null,"dsl_features":[]}],"is_virtual":false,"without_rowid":false,"constraints":[],"explicit_pk":["name"]}},{"id":2,"references":[],"type":"table","data":{"name":"accounts","was_declared_in_moor":false,"columns":[{"name":"id","getter_name":"id","moor_type":"int","nullable":false,"customConstraints":null,"defaultConstraints":"PRIMARY KEY AUTOINCREMENT","dialectAwareDefaultConstraints":{"sqlite":"PRIMARY KEY AUTOINCREMENT"},"default_dart":null,"default_client_dart":null,"dsl_features":["auto-increment"]},{"name":"realm_url","getter_name":"realmUrl","moor_type":"string","nullable":false,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[],"type_converter":{"dart_expr":"const UriConverter()","dart_type_name":"Uri"}},{"name":"user_id","getter_name":"userId","moor_type":"int","nullable":false,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[]},{"name":"email","getter_name":"email","moor_type":"string","nullable":false,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[]},{"name":"api_key","getter_name":"apiKey","moor_type":"string","nullable":false,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[]},{"name":"zulip_version","getter_name":"zulipVersion","moor_type":"string","nullable":false,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[]},{"name":"zulip_merge_base","getter_name":"zulipMergeBase","moor_type":"string","nullable":true,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[]},{"name":"zulip_feature_level","getter_name":"zulipFeatureLevel","moor_type":"int","nullable":false,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[]},{"name":"acked_push_token","getter_name":"ackedPushToken","moor_type":"string","nullable":true,"customConstraints":null,"default_dart":null,"default_client_dart":null,"dsl_features":[]}],"is_virtual":false,"without_rowid":false,"constraints":[],"unique_keys":[["realm_url","user_id"],["realm_url","email"]]}}]} \ No newline at end of file diff --git a/test/model/schemas/schema.dart b/test/model/schemas/schema.dart index c42542afb3..d59002bf56 100644 --- a/test/model/schemas/schema.dart +++ b/test/model/schemas/schema.dart @@ -8,6 +8,7 @@ import 'schema_v2.dart' as v2; import 'schema_v3.dart' as v3; import 'schema_v4.dart' as v4; import 'schema_v5.dart' as v5; +import 'schema_v6.dart' as v6; class GeneratedHelper implements SchemaInstantiationHelper { @override @@ -23,10 +24,12 @@ class GeneratedHelper implements SchemaInstantiationHelper { return v4.DatabaseAtV4(db); case 5: return v5.DatabaseAtV5(db); + case 6: + return v6.DatabaseAtV6(db); default: throw MissingSchemaException(version, versions); } } - static const versions = const [1, 2, 3, 4, 5]; + static const versions = const [1, 2, 3, 4, 5, 6]; } diff --git a/test/model/schemas/schema_v6.dart b/test/model/schemas/schema_v6.dart new file mode 100644 index 0000000000..17ff55be21 --- /dev/null +++ b/test/model/schemas/schema_v6.dart @@ -0,0 +1,890 @@ +// dart format width=80 +// GENERATED CODE, DO NOT EDIT BY HAND. +// ignore_for_file: type=lint +import 'package:drift/drift.dart'; + +class GlobalSettings extends Table + with TableInfo { + @override + final GeneratedDatabase attachedDatabase; + final String? _alias; + GlobalSettings(this.attachedDatabase, [this._alias]); + late final GeneratedColumn themeSetting = GeneratedColumn( + 'theme_setting', + aliasedName, + true, + type: DriftSqlType.string, + requiredDuringInsert: false, + ); + late final GeneratedColumn browserPreference = + GeneratedColumn( + 'browser_preference', + aliasedName, + true, + type: DriftSqlType.string, + requiredDuringInsert: false, + ); + @override + List get $columns => [themeSetting, browserPreference]; + @override + String get aliasedName => _alias ?? actualTableName; + @override + String get actualTableName => $name; + static const String $name = 'global_settings'; + @override + Set get $primaryKey => const {}; + @override + GlobalSettingsData map(Map data, {String? tablePrefix}) { + final effectivePrefix = tablePrefix != null ? '$tablePrefix.' : ''; + return GlobalSettingsData( + themeSetting: attachedDatabase.typeMapping.read( + DriftSqlType.string, + data['${effectivePrefix}theme_setting'], + ), + browserPreference: attachedDatabase.typeMapping.read( + DriftSqlType.string, + data['${effectivePrefix}browser_preference'], + ), + ); + } + + @override + GlobalSettings createAlias(String alias) { + return GlobalSettings(attachedDatabase, alias); + } +} + +class GlobalSettingsData extends DataClass + implements Insertable { + final String? themeSetting; + final String? browserPreference; + const GlobalSettingsData({this.themeSetting, this.browserPreference}); + @override + Map toColumns(bool nullToAbsent) { + final map = {}; + if (!nullToAbsent || themeSetting != null) { + map['theme_setting'] = Variable(themeSetting); + } + if (!nullToAbsent || browserPreference != null) { + map['browser_preference'] = Variable(browserPreference); + } + return map; + } + + GlobalSettingsCompanion toCompanion(bool nullToAbsent) { + return GlobalSettingsCompanion( + themeSetting: + themeSetting == null && nullToAbsent + ? const Value.absent() + : Value(themeSetting), + browserPreference: + browserPreference == null && nullToAbsent + ? const Value.absent() + : Value(browserPreference), + ); + } + + factory GlobalSettingsData.fromJson( + Map json, { + ValueSerializer? serializer, + }) { + serializer ??= driftRuntimeOptions.defaultSerializer; + return GlobalSettingsData( + themeSetting: serializer.fromJson(json['themeSetting']), + browserPreference: serializer.fromJson( + json['browserPreference'], + ), + ); + } + @override + Map toJson({ValueSerializer? serializer}) { + serializer ??= driftRuntimeOptions.defaultSerializer; + return { + 'themeSetting': serializer.toJson(themeSetting), + 'browserPreference': serializer.toJson(browserPreference), + }; + } + + GlobalSettingsData copyWith({ + Value themeSetting = const Value.absent(), + Value browserPreference = const Value.absent(), + }) => GlobalSettingsData( + themeSetting: themeSetting.present ? themeSetting.value : this.themeSetting, + browserPreference: + browserPreference.present + ? browserPreference.value + : this.browserPreference, + ); + GlobalSettingsData copyWithCompanion(GlobalSettingsCompanion data) { + return GlobalSettingsData( + themeSetting: + data.themeSetting.present + ? data.themeSetting.value + : this.themeSetting, + browserPreference: + data.browserPreference.present + ? data.browserPreference.value + : this.browserPreference, + ); + } + + @override + String toString() { + return (StringBuffer('GlobalSettingsData(') + ..write('themeSetting: $themeSetting, ') + ..write('browserPreference: $browserPreference') + ..write(')')) + .toString(); + } + + @override + int get hashCode => Object.hash(themeSetting, browserPreference); + @override + bool operator ==(Object other) => + identical(this, other) || + (other is GlobalSettingsData && + other.themeSetting == this.themeSetting && + other.browserPreference == this.browserPreference); +} + +class GlobalSettingsCompanion extends UpdateCompanion { + final Value themeSetting; + final Value browserPreference; + final Value rowid; + const GlobalSettingsCompanion({ + this.themeSetting = const Value.absent(), + this.browserPreference = const Value.absent(), + this.rowid = const Value.absent(), + }); + GlobalSettingsCompanion.insert({ + this.themeSetting = const Value.absent(), + this.browserPreference = const Value.absent(), + this.rowid = const Value.absent(), + }); + static Insertable custom({ + Expression? themeSetting, + Expression? browserPreference, + Expression? rowid, + }) { + return RawValuesInsertable({ + if (themeSetting != null) 'theme_setting': themeSetting, + if (browserPreference != null) 'browser_preference': browserPreference, + if (rowid != null) 'rowid': rowid, + }); + } + + GlobalSettingsCompanion copyWith({ + Value? themeSetting, + Value? browserPreference, + Value? rowid, + }) { + return GlobalSettingsCompanion( + themeSetting: themeSetting ?? this.themeSetting, + browserPreference: browserPreference ?? this.browserPreference, + rowid: rowid ?? this.rowid, + ); + } + + @override + Map toColumns(bool nullToAbsent) { + final map = {}; + if (themeSetting.present) { + map['theme_setting'] = Variable(themeSetting.value); + } + if (browserPreference.present) { + map['browser_preference'] = Variable(browserPreference.value); + } + if (rowid.present) { + map['rowid'] = Variable(rowid.value); + } + return map; + } + + @override + String toString() { + return (StringBuffer('GlobalSettingsCompanion(') + ..write('themeSetting: $themeSetting, ') + ..write('browserPreference: $browserPreference, ') + ..write('rowid: $rowid') + ..write(')')) + .toString(); + } +} + +class BoolGlobalSettings extends Table + with TableInfo { + @override + final GeneratedDatabase attachedDatabase; + final String? _alias; + BoolGlobalSettings(this.attachedDatabase, [this._alias]); + late final GeneratedColumn name = GeneratedColumn( + 'name', + aliasedName, + false, + type: DriftSqlType.string, + requiredDuringInsert: true, + ); + late final GeneratedColumn value = GeneratedColumn( + 'value', + aliasedName, + false, + type: DriftSqlType.bool, + requiredDuringInsert: true, + defaultConstraints: GeneratedColumn.constraintIsAlways( + 'CHECK ("value" IN (0, 1))', + ), + ); + @override + List get $columns => [name, value]; + @override + String get aliasedName => _alias ?? actualTableName; + @override + String get actualTableName => $name; + static const String $name = 'bool_global_settings'; + @override + Set get $primaryKey => {name}; + @override + BoolGlobalSettingsData map(Map data, {String? tablePrefix}) { + final effectivePrefix = tablePrefix != null ? '$tablePrefix.' : ''; + return BoolGlobalSettingsData( + name: + attachedDatabase.typeMapping.read( + DriftSqlType.string, + data['${effectivePrefix}name'], + )!, + value: + attachedDatabase.typeMapping.read( + DriftSqlType.bool, + data['${effectivePrefix}value'], + )!, + ); + } + + @override + BoolGlobalSettings createAlias(String alias) { + return BoolGlobalSettings(attachedDatabase, alias); + } +} + +class BoolGlobalSettingsData extends DataClass + implements Insertable { + final String name; + final bool value; + const BoolGlobalSettingsData({required this.name, required this.value}); + @override + Map toColumns(bool nullToAbsent) { + final map = {}; + map['name'] = Variable(name); + map['value'] = Variable(value); + return map; + } + + BoolGlobalSettingsCompanion toCompanion(bool nullToAbsent) { + return BoolGlobalSettingsCompanion(name: Value(name), value: Value(value)); + } + + factory BoolGlobalSettingsData.fromJson( + Map json, { + ValueSerializer? serializer, + }) { + serializer ??= driftRuntimeOptions.defaultSerializer; + return BoolGlobalSettingsData( + name: serializer.fromJson(json['name']), + value: serializer.fromJson(json['value']), + ); + } + @override + Map toJson({ValueSerializer? serializer}) { + serializer ??= driftRuntimeOptions.defaultSerializer; + return { + 'name': serializer.toJson(name), + 'value': serializer.toJson(value), + }; + } + + BoolGlobalSettingsData copyWith({String? name, bool? value}) => + BoolGlobalSettingsData( + name: name ?? this.name, + value: value ?? this.value, + ); + BoolGlobalSettingsData copyWithCompanion(BoolGlobalSettingsCompanion data) { + return BoolGlobalSettingsData( + name: data.name.present ? data.name.value : this.name, + value: data.value.present ? data.value.value : this.value, + ); + } + + @override + String toString() { + return (StringBuffer('BoolGlobalSettingsData(') + ..write('name: $name, ') + ..write('value: $value') + ..write(')')) + .toString(); + } + + @override + int get hashCode => Object.hash(name, value); + @override + bool operator ==(Object other) => + identical(this, other) || + (other is BoolGlobalSettingsData && + other.name == this.name && + other.value == this.value); +} + +class BoolGlobalSettingsCompanion + extends UpdateCompanion { + final Value name; + final Value value; + final Value rowid; + const BoolGlobalSettingsCompanion({ + this.name = const Value.absent(), + this.value = const Value.absent(), + this.rowid = const Value.absent(), + }); + BoolGlobalSettingsCompanion.insert({ + required String name, + required bool value, + this.rowid = const Value.absent(), + }) : name = Value(name), + value = Value(value); + static Insertable custom({ + Expression? name, + Expression? value, + Expression? rowid, + }) { + return RawValuesInsertable({ + if (name != null) 'name': name, + if (value != null) 'value': value, + if (rowid != null) 'rowid': rowid, + }); + } + + BoolGlobalSettingsCompanion copyWith({ + Value? name, + Value? value, + Value? rowid, + }) { + return BoolGlobalSettingsCompanion( + name: name ?? this.name, + value: value ?? this.value, + rowid: rowid ?? this.rowid, + ); + } + + @override + Map toColumns(bool nullToAbsent) { + final map = {}; + if (name.present) { + map['name'] = Variable(name.value); + } + if (value.present) { + map['value'] = Variable(value.value); + } + if (rowid.present) { + map['rowid'] = Variable(rowid.value); + } + return map; + } + + @override + String toString() { + return (StringBuffer('BoolGlobalSettingsCompanion(') + ..write('name: $name, ') + ..write('value: $value, ') + ..write('rowid: $rowid') + ..write(')')) + .toString(); + } +} + +class Accounts extends Table with TableInfo { + @override + final GeneratedDatabase attachedDatabase; + final String? _alias; + Accounts(this.attachedDatabase, [this._alias]); + late final GeneratedColumn id = GeneratedColumn( + 'id', + aliasedName, + false, + hasAutoIncrement: true, + type: DriftSqlType.int, + requiredDuringInsert: false, + defaultConstraints: GeneratedColumn.constraintIsAlways( + 'PRIMARY KEY AUTOINCREMENT', + ), + ); + late final GeneratedColumn realmUrl = GeneratedColumn( + 'realm_url', + aliasedName, + false, + type: DriftSqlType.string, + requiredDuringInsert: true, + ); + late final GeneratedColumn userId = GeneratedColumn( + 'user_id', + aliasedName, + false, + type: DriftSqlType.int, + requiredDuringInsert: true, + ); + late final GeneratedColumn email = GeneratedColumn( + 'email', + aliasedName, + false, + type: DriftSqlType.string, + requiredDuringInsert: true, + ); + late final GeneratedColumn apiKey = GeneratedColumn( + 'api_key', + aliasedName, + false, + type: DriftSqlType.string, + requiredDuringInsert: true, + ); + late final GeneratedColumn zulipVersion = GeneratedColumn( + 'zulip_version', + aliasedName, + false, + type: DriftSqlType.string, + requiredDuringInsert: true, + ); + late final GeneratedColumn zulipMergeBase = GeneratedColumn( + 'zulip_merge_base', + aliasedName, + true, + type: DriftSqlType.string, + requiredDuringInsert: false, + ); + late final GeneratedColumn zulipFeatureLevel = GeneratedColumn( + 'zulip_feature_level', + aliasedName, + false, + type: DriftSqlType.int, + requiredDuringInsert: true, + ); + late final GeneratedColumn ackedPushToken = GeneratedColumn( + 'acked_push_token', + aliasedName, + true, + type: DriftSqlType.string, + requiredDuringInsert: false, + ); + @override + List get $columns => [ + id, + realmUrl, + userId, + email, + apiKey, + zulipVersion, + zulipMergeBase, + zulipFeatureLevel, + ackedPushToken, + ]; + @override + String get aliasedName => _alias ?? actualTableName; + @override + String get actualTableName => $name; + static const String $name = 'accounts'; + @override + Set get $primaryKey => {id}; + @override + List> get uniqueKeys => [ + {realmUrl, userId}, + {realmUrl, email}, + ]; + @override + AccountsData map(Map data, {String? tablePrefix}) { + final effectivePrefix = tablePrefix != null ? '$tablePrefix.' : ''; + return AccountsData( + id: + attachedDatabase.typeMapping.read( + DriftSqlType.int, + data['${effectivePrefix}id'], + )!, + realmUrl: + attachedDatabase.typeMapping.read( + DriftSqlType.string, + data['${effectivePrefix}realm_url'], + )!, + userId: + attachedDatabase.typeMapping.read( + DriftSqlType.int, + data['${effectivePrefix}user_id'], + )!, + email: + attachedDatabase.typeMapping.read( + DriftSqlType.string, + data['${effectivePrefix}email'], + )!, + apiKey: + attachedDatabase.typeMapping.read( + DriftSqlType.string, + data['${effectivePrefix}api_key'], + )!, + zulipVersion: + attachedDatabase.typeMapping.read( + DriftSqlType.string, + data['${effectivePrefix}zulip_version'], + )!, + zulipMergeBase: attachedDatabase.typeMapping.read( + DriftSqlType.string, + data['${effectivePrefix}zulip_merge_base'], + ), + zulipFeatureLevel: + attachedDatabase.typeMapping.read( + DriftSqlType.int, + data['${effectivePrefix}zulip_feature_level'], + )!, + ackedPushToken: attachedDatabase.typeMapping.read( + DriftSqlType.string, + data['${effectivePrefix}acked_push_token'], + ), + ); + } + + @override + Accounts createAlias(String alias) { + return Accounts(attachedDatabase, alias); + } +} + +class AccountsData extends DataClass implements Insertable { + final int id; + final String realmUrl; + final int userId; + final String email; + final String apiKey; + final String zulipVersion; + final String? zulipMergeBase; + final int zulipFeatureLevel; + final String? ackedPushToken; + const AccountsData({ + required this.id, + required this.realmUrl, + required this.userId, + required this.email, + required this.apiKey, + required this.zulipVersion, + this.zulipMergeBase, + required this.zulipFeatureLevel, + this.ackedPushToken, + }); + @override + Map toColumns(bool nullToAbsent) { + final map = {}; + map['id'] = Variable(id); + map['realm_url'] = Variable(realmUrl); + map['user_id'] = Variable(userId); + map['email'] = Variable(email); + map['api_key'] = Variable(apiKey); + map['zulip_version'] = Variable(zulipVersion); + if (!nullToAbsent || zulipMergeBase != null) { + map['zulip_merge_base'] = Variable(zulipMergeBase); + } + map['zulip_feature_level'] = Variable(zulipFeatureLevel); + if (!nullToAbsent || ackedPushToken != null) { + map['acked_push_token'] = Variable(ackedPushToken); + } + return map; + } + + AccountsCompanion toCompanion(bool nullToAbsent) { + return AccountsCompanion( + id: Value(id), + realmUrl: Value(realmUrl), + userId: Value(userId), + email: Value(email), + apiKey: Value(apiKey), + zulipVersion: Value(zulipVersion), + zulipMergeBase: + zulipMergeBase == null && nullToAbsent + ? const Value.absent() + : Value(zulipMergeBase), + zulipFeatureLevel: Value(zulipFeatureLevel), + ackedPushToken: + ackedPushToken == null && nullToAbsent + ? const Value.absent() + : Value(ackedPushToken), + ); + } + + factory AccountsData.fromJson( + Map json, { + ValueSerializer? serializer, + }) { + serializer ??= driftRuntimeOptions.defaultSerializer; + return AccountsData( + id: serializer.fromJson(json['id']), + realmUrl: serializer.fromJson(json['realmUrl']), + userId: serializer.fromJson(json['userId']), + email: serializer.fromJson(json['email']), + apiKey: serializer.fromJson(json['apiKey']), + zulipVersion: serializer.fromJson(json['zulipVersion']), + zulipMergeBase: serializer.fromJson(json['zulipMergeBase']), + zulipFeatureLevel: serializer.fromJson(json['zulipFeatureLevel']), + ackedPushToken: serializer.fromJson(json['ackedPushToken']), + ); + } + @override + Map toJson({ValueSerializer? serializer}) { + serializer ??= driftRuntimeOptions.defaultSerializer; + return { + 'id': serializer.toJson(id), + 'realmUrl': serializer.toJson(realmUrl), + 'userId': serializer.toJson(userId), + 'email': serializer.toJson(email), + 'apiKey': serializer.toJson(apiKey), + 'zulipVersion': serializer.toJson(zulipVersion), + 'zulipMergeBase': serializer.toJson(zulipMergeBase), + 'zulipFeatureLevel': serializer.toJson(zulipFeatureLevel), + 'ackedPushToken': serializer.toJson(ackedPushToken), + }; + } + + AccountsData copyWith({ + int? id, + String? realmUrl, + int? userId, + String? email, + String? apiKey, + String? zulipVersion, + Value zulipMergeBase = const Value.absent(), + int? zulipFeatureLevel, + Value ackedPushToken = const Value.absent(), + }) => AccountsData( + id: id ?? this.id, + realmUrl: realmUrl ?? this.realmUrl, + userId: userId ?? this.userId, + email: email ?? this.email, + apiKey: apiKey ?? this.apiKey, + zulipVersion: zulipVersion ?? this.zulipVersion, + zulipMergeBase: + zulipMergeBase.present ? zulipMergeBase.value : this.zulipMergeBase, + zulipFeatureLevel: zulipFeatureLevel ?? this.zulipFeatureLevel, + ackedPushToken: + ackedPushToken.present ? ackedPushToken.value : this.ackedPushToken, + ); + AccountsData copyWithCompanion(AccountsCompanion data) { + return AccountsData( + id: data.id.present ? data.id.value : this.id, + realmUrl: data.realmUrl.present ? data.realmUrl.value : this.realmUrl, + userId: data.userId.present ? data.userId.value : this.userId, + email: data.email.present ? data.email.value : this.email, + apiKey: data.apiKey.present ? data.apiKey.value : this.apiKey, + zulipVersion: + data.zulipVersion.present + ? data.zulipVersion.value + : this.zulipVersion, + zulipMergeBase: + data.zulipMergeBase.present + ? data.zulipMergeBase.value + : this.zulipMergeBase, + zulipFeatureLevel: + data.zulipFeatureLevel.present + ? data.zulipFeatureLevel.value + : this.zulipFeatureLevel, + ackedPushToken: + data.ackedPushToken.present + ? data.ackedPushToken.value + : this.ackedPushToken, + ); + } + + @override + String toString() { + return (StringBuffer('AccountsData(') + ..write('id: $id, ') + ..write('realmUrl: $realmUrl, ') + ..write('userId: $userId, ') + ..write('email: $email, ') + ..write('apiKey: $apiKey, ') + ..write('zulipVersion: $zulipVersion, ') + ..write('zulipMergeBase: $zulipMergeBase, ') + ..write('zulipFeatureLevel: $zulipFeatureLevel, ') + ..write('ackedPushToken: $ackedPushToken') + ..write(')')) + .toString(); + } + + @override + int get hashCode => Object.hash( + id, + realmUrl, + userId, + email, + apiKey, + zulipVersion, + zulipMergeBase, + zulipFeatureLevel, + ackedPushToken, + ); + @override + bool operator ==(Object other) => + identical(this, other) || + (other is AccountsData && + other.id == this.id && + other.realmUrl == this.realmUrl && + other.userId == this.userId && + other.email == this.email && + other.apiKey == this.apiKey && + other.zulipVersion == this.zulipVersion && + other.zulipMergeBase == this.zulipMergeBase && + other.zulipFeatureLevel == this.zulipFeatureLevel && + other.ackedPushToken == this.ackedPushToken); +} + +class AccountsCompanion extends UpdateCompanion { + final Value id; + final Value realmUrl; + final Value userId; + final Value email; + final Value apiKey; + final Value zulipVersion; + final Value zulipMergeBase; + final Value zulipFeatureLevel; + final Value ackedPushToken; + const AccountsCompanion({ + this.id = const Value.absent(), + this.realmUrl = const Value.absent(), + this.userId = const Value.absent(), + this.email = const Value.absent(), + this.apiKey = const Value.absent(), + this.zulipVersion = const Value.absent(), + this.zulipMergeBase = const Value.absent(), + this.zulipFeatureLevel = const Value.absent(), + this.ackedPushToken = const Value.absent(), + }); + AccountsCompanion.insert({ + this.id = const Value.absent(), + required String realmUrl, + required int userId, + required String email, + required String apiKey, + required String zulipVersion, + this.zulipMergeBase = const Value.absent(), + required int zulipFeatureLevel, + this.ackedPushToken = const Value.absent(), + }) : realmUrl = Value(realmUrl), + userId = Value(userId), + email = Value(email), + apiKey = Value(apiKey), + zulipVersion = Value(zulipVersion), + zulipFeatureLevel = Value(zulipFeatureLevel); + static Insertable custom({ + Expression? id, + Expression? realmUrl, + Expression? userId, + Expression? email, + Expression? apiKey, + Expression? zulipVersion, + Expression? zulipMergeBase, + Expression? zulipFeatureLevel, + Expression? ackedPushToken, + }) { + return RawValuesInsertable({ + if (id != null) 'id': id, + if (realmUrl != null) 'realm_url': realmUrl, + if (userId != null) 'user_id': userId, + if (email != null) 'email': email, + if (apiKey != null) 'api_key': apiKey, + if (zulipVersion != null) 'zulip_version': zulipVersion, + if (zulipMergeBase != null) 'zulip_merge_base': zulipMergeBase, + if (zulipFeatureLevel != null) 'zulip_feature_level': zulipFeatureLevel, + if (ackedPushToken != null) 'acked_push_token': ackedPushToken, + }); + } + + AccountsCompanion copyWith({ + Value? id, + Value? realmUrl, + Value? userId, + Value? email, + Value? apiKey, + Value? zulipVersion, + Value? zulipMergeBase, + Value? zulipFeatureLevel, + Value? ackedPushToken, + }) { + return AccountsCompanion( + id: id ?? this.id, + realmUrl: realmUrl ?? this.realmUrl, + userId: userId ?? this.userId, + email: email ?? this.email, + apiKey: apiKey ?? this.apiKey, + zulipVersion: zulipVersion ?? this.zulipVersion, + zulipMergeBase: zulipMergeBase ?? this.zulipMergeBase, + zulipFeatureLevel: zulipFeatureLevel ?? this.zulipFeatureLevel, + ackedPushToken: ackedPushToken ?? this.ackedPushToken, + ); + } + + @override + Map toColumns(bool nullToAbsent) { + final map = {}; + if (id.present) { + map['id'] = Variable(id.value); + } + if (realmUrl.present) { + map['realm_url'] = Variable(realmUrl.value); + } + if (userId.present) { + map['user_id'] = Variable(userId.value); + } + if (email.present) { + map['email'] = Variable(email.value); + } + if (apiKey.present) { + map['api_key'] = Variable(apiKey.value); + } + if (zulipVersion.present) { + map['zulip_version'] = Variable(zulipVersion.value); + } + if (zulipMergeBase.present) { + map['zulip_merge_base'] = Variable(zulipMergeBase.value); + } + if (zulipFeatureLevel.present) { + map['zulip_feature_level'] = Variable(zulipFeatureLevel.value); + } + if (ackedPushToken.present) { + map['acked_push_token'] = Variable(ackedPushToken.value); + } + return map; + } + + @override + String toString() { + return (StringBuffer('AccountsCompanion(') + ..write('id: $id, ') + ..write('realmUrl: $realmUrl, ') + ..write('userId: $userId, ') + ..write('email: $email, ') + ..write('apiKey: $apiKey, ') + ..write('zulipVersion: $zulipVersion, ') + ..write('zulipMergeBase: $zulipMergeBase, ') + ..write('zulipFeatureLevel: $zulipFeatureLevel, ') + ..write('ackedPushToken: $ackedPushToken') + ..write(')')) + .toString(); + } +} + +class DatabaseAtV6 extends GeneratedDatabase { + DatabaseAtV6(QueryExecutor e) : super(e); + late final GlobalSettings globalSettings = GlobalSettings(this); + late final BoolGlobalSettings boolGlobalSettings = BoolGlobalSettings(this); + late final Accounts accounts = Accounts(this); + @override + Iterable> get allTables => + allSchemaEntities.whereType>(); + @override + List get allSchemaEntities => [ + globalSettings, + boolGlobalSettings, + accounts, + ]; + @override + int get schemaVersion => 6; +} diff --git a/test/model/settings_test.dart b/test/model/settings_test.dart index 3dae1c4be2..6e728c5237 100644 --- a/test/model/settings_test.dart +++ b/test/model/settings_test.dart @@ -73,4 +73,48 @@ void main() { // TODO integration tests with sqlite }); + + group('getBool/setBool', () { + test('get from default', () { + final globalSettings = eg.globalStore(boolGlobalSettings: {}).settings; + check(globalSettings).getBool(BoolGlobalSetting.placeholderIgnore) + .isFalse(); + assert(!BoolGlobalSetting.placeholderIgnore.default_); + }); + + test('get from initial load', () { + final globalSettings = eg.globalStore(boolGlobalSettings: { + BoolGlobalSetting.placeholderIgnore: true, + }).settings; + check(globalSettings).getBool(BoolGlobalSetting.placeholderIgnore) + .isTrue(); + }); + + test('set, get', () async { + final globalSettings = eg.globalStore(boolGlobalSettings: {}).settings; + check(globalSettings).getBool(BoolGlobalSetting.placeholderIgnore) + .isFalse(); + + await globalSettings.setBool(BoolGlobalSetting.placeholderIgnore, true); + check(globalSettings).getBool(BoolGlobalSetting.placeholderIgnore) + .isTrue(); + + await globalSettings.setBool(BoolGlobalSetting.placeholderIgnore, false); + check(globalSettings).getBool(BoolGlobalSetting.placeholderIgnore) + .isFalse(); + }); + + test('set to null -> revert to default', () async { + final globalSettings = eg.globalStore(boolGlobalSettings: { + BoolGlobalSetting.placeholderIgnore: true, + }).settings; + check(globalSettings).getBool(BoolGlobalSetting.placeholderIgnore) + .isTrue(); + + await globalSettings.setBool(BoolGlobalSetting.placeholderIgnore, null); + check(globalSettings).getBool(BoolGlobalSetting.placeholderIgnore) + .isFalse(); + assert(!BoolGlobalSetting.placeholderIgnore.default_); + }); + }); } diff --git a/test/model/store_checks.dart b/test/model/store_checks.dart index 170be3ca47..5e7f5fbbfe 100644 --- a/test/model/store_checks.dart +++ b/test/model/store_checks.dart @@ -32,6 +32,7 @@ extension GlobalSettingsStoreChecks on Subject { Subject get browserPreference => has((x) => x.browserPreference, 'browserPreference'); Subject get effectiveBrowserPreference => has((x) => x.effectiveBrowserPreference, 'effectiveBrowserPreference'); Subject getUrlLaunchMode(Uri url) => has((x) => x.getUrlLaunchMode(url), 'getUrlLaunchMode'); + Subject getBool(BoolGlobalSetting setting) => has((x) => x.getBool(setting), 'getBool(${setting.name}'); } extension GlobalStoreChecks on Subject { diff --git a/test/model/test_store.dart b/test/model/test_store.dart index b819125091..6f13845cf5 100644 --- a/test/model/test_store.dart +++ b/test/model/test_store.dart @@ -4,6 +4,7 @@ import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/events.dart'; import 'package:zulip/api/route/realm.dart'; import 'package:zulip/model/database.dart'; +import 'package:zulip/model/settings.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/notifications/receive.dart'; import 'package:zulip/widgets/store.dart'; @@ -67,6 +68,11 @@ class _TestGlobalStoreBackend implements GlobalStoreBackend { Future doUpdateGlobalSettings(GlobalSettingsCompanion data) async { // Nothing to do. } + + @override + Future doSetBoolGlobalSetting(BoolGlobalSetting setting, bool? value) async { + // Nothing to do. + } } mixin _DatabaseMixin on GlobalStore { @@ -139,9 +145,12 @@ mixin _DatabaseMixin on GlobalStore { class TestGlobalStore extends GlobalStore with _ApiConnectionsMixin, _DatabaseMixin { TestGlobalStore({ GlobalSettingsData? globalSettings, + Map? boolGlobalSettings, required super.accounts, }) : super(backend: _TestGlobalStoreBackend(), - globalSettings: globalSettings ?? GlobalSettingsData()); + globalSettings: globalSettings ?? GlobalSettingsData(), + boolGlobalSettings: boolGlobalSettings ?? {}, + ); final Map _initialSnapshots = {}; @@ -204,9 +213,12 @@ class TestGlobalStore extends GlobalStore with _ApiConnectionsMixin, _DatabaseMi class UpdateMachineTestGlobalStore extends GlobalStore with _ApiConnectionsMixin, _DatabaseMixin { UpdateMachineTestGlobalStore({ GlobalSettingsData? globalSettings, + Map? boolGlobalSettings, required super.accounts, }) : super(backend: _TestGlobalStoreBackend(), - globalSettings: globalSettings ?? GlobalSettingsData()); + globalSettings: globalSettings ?? GlobalSettingsData(), + boolGlobalSettings: boolGlobalSettings ?? {}, + ); // [doLoadPerAccount] depends on the cache to prepare the API responses. // Calling [clearCachedApiConnections] is permitted, though. From cb3fa1df47ac4db59266569427cb3e819237689c Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sun, 16 Mar 2025 21:17:57 -0700 Subject: [PATCH 8/9] settings: Add the concept of experimental feature flags Fixes #1409. --- assets/l10n/app_en.arb | 8 +++++ lib/generated/l10n/zulip_localizations.dart | 12 +++++++ .../l10n/zulip_localizations_ar.dart | 6 ++++ .../l10n/zulip_localizations_en.dart | 6 ++++ .../l10n/zulip_localizations_ja.dart | 6 ++++ .../l10n/zulip_localizations_nb.dart | 6 ++++ .../l10n/zulip_localizations_pl.dart | 6 ++++ .../l10n/zulip_localizations_ru.dart | 6 ++++ .../l10n/zulip_localizations_sk.dart | 6 ++++ lib/model/settings.dart | 29 ++++++++++++++++ lib/widgets/settings.dart | 33 +++++++++++++++++++ test/widgets/settings_test.dart | 7 ++++ 12 files changed, 131 insertions(+) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index 1973f5fab7..70417e356e 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -823,6 +823,14 @@ "@pollWidgetOptionsMissing": { "description": "Text to display for a poll when it has no options" }, + "experimentalFeatureSettingsPageTitle": "Experimental features", + "@experimentalFeatureSettingsPageTitle": { + "description": "Title of settings page for experimental, in-development features" + }, + "experimentalFeatureSettingsWarning": "These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.", + "@experimentalFeatureSettingsWarning": { + "description": "Warning text on settings page for experimental, in-development features" + }, "errorNotificationOpenTitle": "Failed to open notification", "@errorNotificationOpenTitle": { "description": "Error title when notification opening fails" diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index d4bdd2db47..3203569966 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -1203,6 +1203,18 @@ abstract class ZulipLocalizations { /// **'This poll has no options yet.'** String get pollWidgetOptionsMissing; + /// Title of settings page for experimental, in-development features + /// + /// In en, this message translates to: + /// **'Experimental features'** + String get experimentalFeatureSettingsPageTitle; + + /// Warning text on settings page for experimental, in-development features + /// + /// In en, this message translates to: + /// **'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.'** + String get experimentalFeatureSettingsWarning; + /// Error title when notification opening fails /// /// In en, this message translates to: diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index 5d76c0bfae..20ad3cbe24 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -643,6 +643,12 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get pollWidgetOptionsMissing => 'This poll has no options yet.'; + @override + String get experimentalFeatureSettingsPageTitle => 'Experimental features'; + + @override + String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.'; + @override String get errorNotificationOpenTitle => 'Failed to open notification'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index 17df08bfd5..a88981fc26 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -643,6 +643,12 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get pollWidgetOptionsMissing => 'This poll has no options yet.'; + @override + String get experimentalFeatureSettingsPageTitle => 'Experimental features'; + + @override + String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.'; + @override String get errorNotificationOpenTitle => 'Failed to open notification'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index 9baef2dbae..be6eee8870 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -643,6 +643,12 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get pollWidgetOptionsMissing => 'This poll has no options yet.'; + @override + String get experimentalFeatureSettingsPageTitle => 'Experimental features'; + + @override + String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.'; + @override String get errorNotificationOpenTitle => 'Failed to open notification'; diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index fb3ca10833..8e51c7a19b 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -643,6 +643,12 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get pollWidgetOptionsMissing => 'This poll has no options yet.'; + @override + String get experimentalFeatureSettingsPageTitle => 'Experimental features'; + + @override + String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.'; + @override String get errorNotificationOpenTitle => 'Failed to open notification'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 15af01b7fe..0e4009a462 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -643,6 +643,12 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get pollWidgetOptionsMissing => 'Ta sonda nie ma opcji do wyboru.'; + @override + String get experimentalFeatureSettingsPageTitle => 'Experimental features'; + + @override + String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.'; + @override String get errorNotificationOpenTitle => 'Otwieranie powiadomienia bez powodzenia'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 946fe8c231..f3ec9d623c 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -643,6 +643,12 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get pollWidgetOptionsMissing => 'В опросе пока нет вариантов ответа.'; + @override + String get experimentalFeatureSettingsPageTitle => 'Experimental features'; + + @override + String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.'; + @override String get errorNotificationOpenTitle => 'Не удалось открыть оповещения'; diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index 90d71704f9..6d22409eb5 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -643,6 +643,12 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get pollWidgetOptionsMissing => 'This poll has no options yet.'; + @override + String get experimentalFeatureSettingsPageTitle => 'Experimental features'; + + @override + String get experimentalFeatureSettingsWarning => 'These options enable features which are still under development and not ready. They may not work, and may cause issues in other areas of the app.\n\nThe purpose of these settings is for experimentation by people working on developing Zulip.'; + @override String get errorNotificationOpenTitle => 'Nepodarilo sa otvoriť oznámenie'; diff --git a/lib/model/settings.dart b/lib/model/settings.dart index 45edda115d..dc1bdc524a 100644 --- a/lib/model/settings.dart +++ b/lib/model/settings.dart @@ -58,6 +58,27 @@ enum GlobalSettingType { /// (so that it stands ready to accept a future feature flag), /// we give it a placeholder value which isn't a real setting. placeholder, + + /// Describes a setting which enables an in-progress feature of the app. + /// + /// Sometimes when building a complex feature it's useful to merge PRs that + /// make partial progress, and then to have the feature's logic gated behind + /// a setting that serves as a "feature flag". + /// This enables those working on the feature to enable the flag in order to + /// see the current incomplete behavior, while for everyone else it remains + /// disabled and so (barring bugs in the use of the flag itself) has no effect. + /// + /// These settings are primarily meant for people developing Zulip to use, + /// and so appear in an out-of-the-way part of the settings UI. + /// + /// Settings of this kind are costly to the health of the codebase if + /// allowed to accumulate. Most features don't need one, even features that + /// take two or three PRs to implement. See discussion at: + /// https://github.com/zulip/zulip-flutter/issues/1409#issuecomment-2725793787 + /// When a feature flag is introduced, take care to drive the project to + /// completion, either by merge or removal, so that the flag can be retired + /// within a period of a few weeks or months. + experimentalFeatureFlag, ; } @@ -83,6 +104,10 @@ enum GlobalSettingType { /// finish an experimental feature.) enum BoolGlobalSetting { /// A non-setting to ensure this enum has at least one value. + /// + /// Leave this in place even when there are experimental feature flags too. + /// That way when we remove those, this is already here. + /// (Having one stable value in this enum is also handy for tests.) placeholderIgnore(GlobalSettingType.placeholder, false), // Former settings which might exist in the database, @@ -117,6 +142,10 @@ class GlobalSettingsStore extends ChangeNotifier { required Map boolData, }) : _backend = backend, _data = data, _boolData = boolData; + static final List experimentalFeatureFlags = + BoolGlobalSetting.values.where((setting) => + setting.type == GlobalSettingType.experimentalFeatureFlag).toList(); + final GlobalStoreBackend _backend; /// A cache of the [GlobalSettingsData] singleton in the underlying data store. diff --git a/lib/widgets/settings.dart b/lib/widgets/settings.dart index 3d70f6b39f..9e7581c539 100644 --- a/lib/widgets/settings.dart +++ b/lib/widgets/settings.dart @@ -23,6 +23,11 @@ class SettingsPage extends StatelessWidget { body: Column(children: [ const _ThemeSetting(), const _BrowserPreferenceSetting(), + if (GlobalSettingsStore.experimentalFeatureFlags.isNotEmpty) + ListTile( + title: Text(zulipLocalizations.experimentalFeatureSettingsPageTitle), + onTap: () => Navigator.push(context, + ExperimentalFeaturesPage.buildRoute())) ])); } } @@ -76,3 +81,31 @@ class _BrowserPreferenceSetting extends StatelessWidget { onChanged: (newValue) => _handleChange(context, newValue)); } } + +class ExperimentalFeaturesPage extends StatelessWidget { + const ExperimentalFeaturesPage({super.key}); + + static WidgetRoute buildRoute() { + return MaterialWidgetRoute(page: const ExperimentalFeaturesPage()); + } + + @override + Widget build(BuildContext context) { + final zulipLocalizations = ZulipLocalizations.of(context); + final globalSettings = GlobalStoreWidget.settingsOf(context); + final flags = GlobalSettingsStore.experimentalFeatureFlags; + assert(flags.isNotEmpty); + return Scaffold( + appBar: AppBar( + title: Text(zulipLocalizations.experimentalFeatureSettingsPageTitle)), + body: Column(children: [ + ListTile( + title: Text(zulipLocalizations.experimentalFeatureSettingsWarning)), + for (final flag in flags) + SwitchListTile.adaptive( + title: Text(flag.name), // no i18n; these are developer-facing settings + value: globalSettings.getBool(flag), + onChanged: (value) => globalSettings.setBool(flag, value)), + ])); + } +} diff --git a/test/widgets/settings_test.dart b/test/widgets/settings_test.dart index ced9c3f853..46df165ecc 100644 --- a/test/widgets/settings_test.dart +++ b/test/widgets/settings_test.dart @@ -126,4 +126,11 @@ void main() { ? BrowserPreference.inApp : BrowserPreference.external); }, variant: TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); }); + + // TODO maybe test GlobalSettingType.experimentalFeatureFlag settings + // Or maybe not; after all, it's a developer-facing feature, so + // should be low risk. + // (The main ingredient in writing such tests would be to wire up + // [GlobalSettingsStore.experimentalFeatureFlags] so that tests can + // control making it empty, or non-empty, at will.) } From 8cf9c3780a2d292ced57f6509cec40b7e48fee5d Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sun, 16 Mar 2025 22:18:21 -0700 Subject: [PATCH 9/9] content [nfc]: Dedupe a pair of ContentTheme.of lookups --- lib/widgets/content.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 3d2f3aa2b8..c949602824 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -830,10 +830,11 @@ class MathBlock extends StatelessWidget { @override Widget build(BuildContext context) { + final contentTheme = ContentTheme.of(context); return _CodeBlockContainer( - borderColor: ContentTheme.of(context).colorMathBlockBorder, + borderColor: contentTheme.colorMathBlockBorder, child: Text.rich(TextSpan( - style: ContentTheme.of(context).codeBlockTextStyles.plain, + style: contentTheme.codeBlockTextStyles.plain, children: [TextSpan(text: node.texSource)]))); } }