Skip to content

Experimental feature flags; general bool global settings #1421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 24, 2025
Merged
8 changes: 8 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
12 changes: 12 additions & 0 deletions lib/generated/l10n/zulip_localizations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_ar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_en.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_ja.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_nb.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_pl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_ru.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 => 'Не удалось открыть оповещения';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_sk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
26 changes: 26 additions & 0 deletions lib/log.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@

import 'package:flutter/foundation.dart';

/// Whether [debugLog] should do anything.
///
/// This has an effect only in a debug build.
Expand Down Expand Up @@ -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.
Expand Down
56 changes: 54 additions & 2 deletions lib/model/database.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,44 @@ class GlobalSettings extends Table {

Column<String> get browserPreference => textEnum<BrowserPreference>()
.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<String> 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<bool> get value => boolean()();

@override
Set<Column<Object>>? get primaryKey => {name};
}

/// The table of [Account] records in the app's database.
Expand Down Expand Up @@ -68,7 +106,7 @@ class UriConverter extends TypeConverter<Uri, String> {
@override Uri fromSql(String fromDb) => Uri.parse(fromDb);
}

@DriftDatabase(tables: [GlobalSettings, Accounts])
@DriftDatabase(tables: [GlobalSettings, BoolGlobalSettings, Accounts])
class AppDatabase extends _$AppDatabase {
AppDatabase(super.e);

Expand All @@ -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;
Expand Down Expand Up @@ -133,6 +171,9 @@ class AppDatabase extends _$AppDatabase {
RawValuesInsertable({}));
}
},
from5To6: (m, schema) async {
await m.createTable(schema.boolGlobalSettings);
},
);

Future<void> _createLatestSchema(Migrator m) async {
Expand Down Expand Up @@ -173,6 +214,17 @@ class AppDatabase extends _$AppDatabase {
return await (select(globalSettings)..limit(1)).getSingle();
}

Future<Map<BoolGlobalSetting, bool>> getBoolGlobalSettings() async {
final result = <BoolGlobalSetting, bool>{};
final rows = await select(boolGlobalSettings).get();
for (final row in rows) {
final setting = BoolGlobalSetting.byName(row.name);
if (setting == null) continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be beneficial to have tests verify that this doesn't throw when there are rows with unknown setting names.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah, good catch. In this version no tests fail if I cut this line and say setting! on the next one.

result[setting] = row.value;
}
return result;
}

Future<int> createAccount(AccountsCompanion values) async {
try {
return await into(accounts).insert(values);
Expand Down
Loading