Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Add stable unstable feature for jump to date before v1.6 is fully supported on a homeserver #10398

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
controller: new ServerSupportUnstableFeatureController(
"feature_exploring_public_spaces",
defaultWatchManager,
["org.matrix.msc3827.stable"],
[["org.matrix.msc3827.stable"]],
undefined,
_td("Requires your server to support the stable version of MSC3827"),
),
Expand Down Expand Up @@ -372,7 +372,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
controller: new ServerSupportUnstableFeatureController(
"feature_jump_to_date",
defaultWatchManager,
["org.matrix.msc3030"],
[["org.matrix.msc3030"], ["org.matrix.msc3030.stable"]],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add stable unstable version (org.matrix.msc3030.stable) for jump to date before v1.6 is fully supported on a homeserver.

org.matrix.msc3030.stable wasn't included in MSC3030 but it seems like pattern that should have been included like MSC2285 as an example.

Are we okay with moving forward with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"v1.6",
_td("Requires your server to support MSC3030"),
),
Expand All @@ -388,7 +388,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
controller: new ServerSupportUnstableFeatureController(
"sendReadReceipts",
defaultWatchManager,
["org.matrix.msc2285.stable"],
[["org.matrix.msc2285.stable"]],
"v1.4",
_td("Your server doesn't support disabling sending read receipts."),
true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,21 @@ import SettingsStore from "../SettingsStore";
* When a setting gets disabled or enabled from this controller it notifies the given WatchManager
*/
export default class ServerSupportUnstableFeatureController extends MatrixClientBackedController {
// Starts off as `undefined` so when we first compare the `newDisabledValue`, it sees
// it as a change and updates the watchers.
private enabled: boolean | undefined;

/**
* Construct a new ServerSupportUnstableFeatureController.
*
* @param unstableFeatureGroups - If any one of the feature groups is satisfied,
* then the setting is considered enabled. A feature group is satisfied if all of
* the features in the group are supported (all features in a group are required).
*/
Comment on lines +36 to +39
Copy link
Contributor Author

@MadLittleMods MadLittleMods Mar 17, 2023

Choose a reason for hiding this comment

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

As suggested by @t3chguy in #10398 (comment),

Guess it should be extended to string[][] where its an OR of AND groups to offer more flexibility

Refactoring ServerSupportUnstableFeatureController to support multiple feature groups where any one of them will enable the setting. All features in a feature group are required.

This way having either org.matrix.msc3030 or org.matrix.msc3030.stable will enable the jump to date feature flag with a config of [["org.matrix.msc3030"], ["org.matrix.msc3030.stable"]]

If you want to AND features together like before, just add them to the same group: [["foo", "bar"]]

public constructor(
private readonly settingName: string,
private readonly watchers: WatchManager,
private readonly unstableFeatures: string[],
private readonly unstableFeatureGroups: string[][],
private readonly stableVersion?: string,
private readonly disabledMessage?: string,
private readonly forcedValue: any = false,
Expand All @@ -43,29 +52,43 @@ export default class ServerSupportUnstableFeatureController extends MatrixClient
return !this.enabled;
}

public set disabled(v: boolean) {
if (!v === this.enabled) return;
this.enabled = !v;
public set disabled(newDisabledValue: boolean) {
if (!newDisabledValue === this.enabled) return;
this.enabled = !newDisabledValue;
const level = SettingsStore.firstSupportedLevel(this.settingName);
if (!level) return;
const settingValue = SettingsStore.getValue(this.settingName, null);
this.watchers.notifyUpdate(this.settingName, null, level, settingValue);
}

protected async initMatrixClient(oldClient: MatrixClient, newClient: MatrixClient): Promise<void> {
this.disabled = true;
let supported = true;

// Check for stable version support first
if (this.stableVersion && (await this.client.isVersionSupported(this.stableVersion))) {
this.disabled = false;
return;
}
for (const feature of this.unstableFeatures) {
supported = await this.client.doesServerSupportUnstableFeature(feature);
if (!supported) break;

// Otherwise, only one of the unstable feature groups needs to be satisfied in
// order for this setting overall to be enabled
let isEnabled = false;
for (const featureGroup of this.unstableFeatureGroups) {
const featureSupportList = await Promise.all(
featureGroup.map(async (feature) => {
const isFeatureSupported = await this.client.doesServerSupportUnstableFeature(feature);
return isFeatureSupported;
}),
);

// Every feature in a feature group is required in order
// for this setting overall to be enabled.
const isFeatureGroupSatisfied = featureSupportList.every((isFeatureSupported) => isFeatureSupported);
if (isFeatureGroupSatisfied) {
isEnabled = true;
break;
}
}

this.disabled = !supported;
this.disabled = !isEnabled;
}

public getValueOverride(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe("ServerSupportUnstableFeatureController", () => {
const controller = new ServerSupportUnstableFeatureController(
setting,
watchers,
["feature"],
[["feature"]],
undefined,
undefined,
"other_value",
Expand All @@ -74,7 +74,7 @@ describe("ServerSupportUnstableFeatureController", () => {
const controller = new ServerSupportUnstableFeatureController(
setting,
watchers,
["feature"],
[["feature"]],
"other_value",
);
await prepareSetting(cli, controller);
Expand All @@ -84,38 +84,65 @@ describe("ServerSupportUnstableFeatureController", () => {
});

describe("settingDisabled()", () => {
it("returns true if there is no matrix client", () => {
const controller = new ServerSupportUnstableFeatureController(setting, watchers, ["org.matrix.msc3030"]);
it("considered disabled if there is no matrix client", () => {
const controller = new ServerSupportUnstableFeatureController(setting, watchers, [["org.matrix.msc3030"]]);
expect(controller.settingDisabled).toEqual(true);
});

it("returns true if not all required features are supported", async () => {
it("considered disabled if not all required features in the only feature group are supported", async () => {
const cli = stubClient();
cli.doesServerSupportUnstableFeature = jest.fn(async (featureName) => {
return featureName === "org.matrix.msc3827.stable";
});

const controller = new ServerSupportUnstableFeatureController(setting, watchers, [
"org.matrix.msc3827.stable",
"org.matrix.msc3030",
["org.matrix.msc3827.stable", "org.matrix.msc3030"],
]);
await prepareSetting(cli, controller);

expect(controller.settingDisabled).toEqual(true);
});

it("returns false if all required features are supported", async () => {
it("considered enabled if all required features in the only feature group are supported", async () => {
const cli = stubClient();
cli.doesServerSupportUnstableFeature = jest.fn(async (featureName) => {
return featureName === "org.matrix.msc3827.stable" || featureName === "org.matrix.msc3030";
});
const controller = new ServerSupportUnstableFeatureController(setting, watchers, [
"org.matrix.msc3827.stable",
"org.matrix.msc3030",
["org.matrix.msc3827.stable", "org.matrix.msc3030"],
]);
await prepareSetting(cli, controller);

expect(controller.settingDisabled).toEqual(false);
});

it("considered enabled if all required features in one of the feature groups are supported", async () => {
const cli = stubClient();
cli.doesServerSupportUnstableFeature = jest.fn(async (featureName) => {
return featureName === "org.matrix.msc3827.stable" || featureName === "org.matrix.msc3030";
});
const controller = new ServerSupportUnstableFeatureController(setting, watchers, [
["foo-unsupported", "bar-unsupported"],
["org.matrix.msc3827.stable", "org.matrix.msc3030"],
]);
await prepareSetting(cli, controller);

expect(controller.settingDisabled).toEqual(false);
});

it("considered disabled if not all required features in one of the feature groups are supported", async () => {
const cli = stubClient();
cli.doesServerSupportUnstableFeature = jest.fn(async (featureName) => {
return featureName === "org.matrix.msc3827.stable";
});

const controller = new ServerSupportUnstableFeatureController(setting, watchers, [
["foo-unsupported", "bar-unsupported"],
["org.matrix.msc3827.stable", "org.matrix.msc3030"],
]);
await prepareSetting(cli, controller);

expect(controller.settingDisabled).toEqual(true);
});
});
});